diff --git a/README.md b/README.md index a7d0434..2568047 100644 --- a/README.md +++ b/README.md @@ -42,9 +42,21 @@ or `"nsurl:NSURLErrorTimedOut"`). The `` portion is the platform's human message — it may be OS-localized on Apple platforms and includes request specifics like host, port, and path where the platform provides them. -Platform support: the Apple (`NSURLSession`) and Linux (`libcurl`) backends populate -these accessors today; the Windows and Android backends currently always report -empty/zero (contributions welcome — the plumbing in `UrlRequest_Base.h` is shared). +Platform support: the Apple (`NSURLSession`), Linux (`libcurl`), and Android +(`URLConnection`) backends populate these accessors. Android reports Java exceptions +as `java:JavaException(0)` and other read failures as `urllib:ResponseReadFailed(0)`. +The Windows backend currently reports empty/zero. + +Android publishes the HTTP status only after the entire response body has been read. +A read exception or premature end of a known-length body clears the partial response +and reports status 0, rather than reporting the already received HTTP status as success. +Completed HTTP error responses are read from `getErrorStream()` and retain their status. + +The Apple backend also checks fixed-length, unencoded HTTP bodies before publishing +the response: `NSURLSession` can report a truncated HTTP error response without an +`NSError`. Premature EOF reports `urllib:ResponseReadFailed(0)`. This check excludes +bodyless statuses and encoded/chunked bodies, whose decoded size is not the wire +`Content-Length`. ## Contributing diff --git a/Source/UrlRequest_Android.cpp b/Source/UrlRequest_Android.cpp index 8f97d95..2f6a17c 100644 --- a/Source/UrlRequest_Android.cpp +++ b/Source/UrlRequest_Android.cpp @@ -4,6 +4,7 @@ #include #include #include +#include using namespace android::global; using namespace android::net; @@ -15,6 +16,29 @@ namespace UrlLib { namespace { + void ThrowIfFaulted(JNIEnv* env) + { + if (auto error = env->ExceptionOccurred()) + { + env->ExceptionClear(); + auto release = gsl::finally([&] { env->DeleteLocalRef(error); }); + throw Throwable{error}; + } + } + + jobject GetResponseStream(URLConnection& connection, int statusCode) + { + auto env = GetEnvForCurrentThread(); + // HttpURLConnection throws for HTTP error responses via getInputStream; + // their bodies are still successful transfers, exposed by getErrorStream. + auto method = env->GetMethodID(connection.GetClass(), + statusCode >= 400 ? "getErrorStream" : "getInputStream", "()Ljava/io/InputStream;"); + ThrowIfFaulted(env); + auto stream = env->CallObjectMethod(connection, method); + ThrowIfFaulted(env); + return stream; + } + template void LoadAsset(AAssetManager* assetManager, const char* url, T& data) { AAsset* asset = AAssetManager_open(assetManager, url, AASSET_MODE_UNKNOWN); @@ -121,13 +145,10 @@ namespace UrlLib } connection.Connect(); + int statusCode = static_cast(UrlStatusCode::Ok); if (connection.GetClass().IsAssignableFrom(HttpURLConnection::Class())) { - m_statusCode = static_cast(((HttpURLConnection)connection).GetResponseCode()); - } - else - { - m_statusCode = UrlStatusCode::Ok; + statusCode = ((HttpURLConnection)connection).GetResponseCode(); } for (int n = 0;; ++n) @@ -145,20 +166,38 @@ namespace UrlLib m_headers.insert({lowerCaseKey, value}); } - int contentLength = connection.GetContentLength(); - if (contentLength < 0) - { - contentLength = 0; - } - - InputStream inputStream{connection.GetInputStream()}; - ByteArrayOutputStream byteArrayOutputStream{contentLength}; + const int contentLength = connection.GetContentLength(); + auto env = GetEnvForCurrentThread(); + auto stream = GetResponseStream(connection, statusCode); + auto releaseStream = gsl::finally([&] { env->DeleteLocalRef(stream); }); + ByteArrayOutputStream byteArrayOutputStream{std::max(contentLength, 0)}; + ThrowIfFaulted(env); ByteArray byteArray{4096}; - int bytesRead{}; - while ((bytesRead = inputStream.Read(byteArray)) != -1) + ThrowIfFaulted(env); + size_t totalBytesRead{}; + if (stream) { - byteArrayOutputStream.Write(byteArray, 0, bytesRead); + InputStream inputStream{stream}; + while (true) + { + const int bytesRead = inputStream.Read(byteArray); + // The pinned InputStream wrapper leaves read exceptions pending. + // Check before making another JNI call or interpreting its result. + ThrowIfFaulted(env); + if (bytesRead == -1) + { + break; + } + byteArrayOutputStream.Write(byteArray, 0, bytesRead); + ThrowIfFaulted(env); + totalBytesRead += static_cast(bytesRead); + } + } + if (statusCode != 204 && statusCode != 304 && + contentLength >= 0 && totalBytesRead != static_cast(contentLength)) + { + throw std::runtime_error{"Response body ended before Content-Length bytes were received"}; } switch (m_responseType) @@ -166,12 +205,16 @@ namespace UrlLib case UrlResponseType::String: { // TODO: use the charset from the content type? - m_responseString = byteArrayOutputStream.ToString("UTF-8"); + auto text = byteArrayOutputStream.ToString("UTF-8"); + ThrowIfFaulted(env); + m_responseString = text; break; } case UrlResponseType::Buffer: { - m_responseBuffer = byteArrayOutputStream.ToByteArray(); + auto bytes = byteArrayOutputStream.ToByteArray(); + ThrowIfFaulted(env); + m_responseBuffer = bytes; break; } default: @@ -182,11 +225,21 @@ namespace UrlLib // Must happen after getting the content to get the redirected URL. m_responseUrl = connection.GetURL().ToString(); + ThrowIfFaulted(env); + m_statusCode = static_cast(statusCode); } } - catch (const Throwable&) + catch (const Throwable& error) + { + ResetForOpen(); + m_responseBuffer.clear(); + SetError("java", "JavaException", 0, error.what()); + } + catch (const std::exception& error) { - // Catch Java exceptions, but retain the default status code of 0 to indicate a client side error. + ResetForOpen(); + m_responseBuffer.clear(); + SetError("urllib", "ResponseReadFailed", 0, error.what()); } }); } diff --git a/Source/UrlRequest_Apple.mm b/Source/UrlRequest_Apple.mm index f6e8ea6..ac9528e 100644 --- a/Source/UrlRequest_Apple.mm +++ b/Source/UrlRequest_Apple.mm @@ -185,16 +185,35 @@ void Open(UrlMethod method, const std::string& url) return; } - if ([response class] == [NSHTTPURLResponse class]) + if ([response isKindOfClass:[NSHTTPURLResponse class]]) { NSHTTPURLResponse* httpResponse{(NSHTTPURLResponse*)response}; - m_statusCode = static_cast(httpResponse.statusCode); for (id key in httpResponse.allHeaderFields) { id value = [httpResponse.allHeaderFields objectForKey:key]; m_headers.insert({ToLower([key UTF8String]), [value UTF8String]}); } + + // NSURLSession can complete truncated HTTP error bodies without an NSError. + // NSData is decoded, so only compare an identity, fixed-length body with + // Content-Length; encoded/chunked responses must use the transport's checks. + const auto encoding = m_headers.find("content-encoding"); + const bool identity = encoding == m_headers.end() || encoding->second.empty() || + ToLower(encoding->second.c_str()) == "identity"; + if (httpResponse.statusCode != 204 && httpResponse.statusCode != 304 && + identity && m_headers.find("transfer-encoding") == m_headers.end() && + response.expectedContentLength >= 0 && + data.length < static_cast(response.expectedContentLength)) + { + m_headers.clear(); + SetError("urllib", "ResponseReadFailed", 0, + "Response body ended before Content-Length bytes were received"); + taskCompletionSource.complete(); + return; + } + + m_statusCode = static_cast(httpResponse.statusCode); } else { diff --git a/Tests/UrlRequestErrorReporting.cpp b/Tests/UrlRequestErrorReporting.cpp index 812116e..4baf35d 100644 --- a/Tests/UrlRequestErrorReporting.cpp +++ b/Tests/UrlRequestErrorReporting.cpp @@ -202,15 +202,11 @@ namespace uint16_t m_port; }; - // A loopback TCP server that accepts connections but never responds, so an HTTP request to it - // hangs until it is aborted. A background thread accept()s connections and holds them open - // until teardown. Used to verify that UrlRequest::Abort() interrupts an in-flight request - // rather than waiting for the transport's own timeout. Non-movable: the accept thread captures - // `this`. - class HangingServer + // Without a response, hold connections open until teardown to test cancellation. + class LoopbackServer { public: - HangingServer() + explicit LoopbackServer(std::string response = {}) { if (!EnsureSocketsInitialized()) { @@ -238,7 +234,7 @@ namespace m_listener = listener; m_port = ntohs(address.sin_port); - m_acceptThread = std::thread{[this]() { + m_acceptThread = std::thread{[this, response = std::move(response)]() { for (;;) { NativeSocket connection = ::accept(m_listener, nullptr, nullptr); @@ -246,17 +242,60 @@ namespace { break; // listener closed during teardown } - m_accepted.push_back(connection); // hold open, never respond + m_accepted.push_back(connection); + if (!response.empty()) + { +#if defined(_WIN32) + DWORD timeout{2000}; + setsockopt(connection, SOL_SOCKET, SO_RCVTIMEO, reinterpret_cast(&timeout), sizeof(timeout)); + setsockopt(connection, SOL_SOCKET, SO_SNDTIMEO, reinterpret_cast(&timeout), sizeof(timeout)); +#else + timeval timeout{2, 0}; + setsockopt(connection, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout)); + setsockopt(connection, SOL_SOCKET, SO_SNDTIMEO, &timeout, sizeof(timeout)); +#endif +#if defined(__APPLE__) + int noSignal{1}; + setsockopt(connection, SOL_SOCKET, SO_NOSIGPIPE, &noSignal, sizeof(noSignal)); +#endif + std::string headers; + char buffer[1024]; + while (headers.find("\r\n\r\n") == std::string::npos && headers.size() < 8192) + { + const auto count = recv(connection, buffer, sizeof(buffer), 0); + if (count <= 0) + { + break; + } + headers.append(buffer, static_cast(count)); + } + size_t offset{}; + while (offset < response.size()) + { +#if defined(MSG_NOSIGNAL) + constexpr int flags = MSG_NOSIGNAL; +#else + constexpr int flags = 0; +#endif + const auto count = send(connection, response.data() + offset, static_cast(response.size() - offset), flags); + if (count <= 0) + { + break; + } + offset += static_cast(count); + } + ShutdownSocket(connection); + } } }}; } - HangingServer(const HangingServer&) = delete; - HangingServer& operator=(const HangingServer&) = delete; - HangingServer(HangingServer&&) = delete; - HangingServer& operator=(HangingServer&&) = delete; + LoopbackServer(const LoopbackServer&) = delete; + LoopbackServer& operator=(const LoopbackServer&) = delete; + LoopbackServer(LoopbackServer&&) = delete; + LoopbackServer& operator=(LoopbackServer&&) = delete; - ~HangingServer() + ~LoopbackServer() { if (m_listener != InvalidSocket) { @@ -346,6 +385,91 @@ TEST(UrlRequestErrorReporting, SuccessfulLocalFileReportsNoError) EXPECT_EQ(request.ErrorCode(), 0); } +#if !defined(_WIN32) +// The WinRT backend currently returns non-2xx responses without reading their bodies. +TEST(UrlRequestErrorReporting, CompletedAndTruncatedHttpBodies) +{ + for (const auto responseType : {UrlLib::UrlResponseType::String, UrlLib::UrlResponseType::Buffer}) + { + for (const int status : {200, 404}) + { + for (const bool truncated : {false, true}) + { + SCOPED_TRACE(std::to_string(status) + (truncated ? " truncated" : " complete") + + (responseType == UrlLib::UrlResponseType::String ? " text" : " buffer")); + LoopbackServer server{"HTTP/1.1 " + std::to_string(status) + + (status == 404 ? " Not Found\r\n" : " OK\r\n") + + "Content-Length: " + (truncated ? "100" : "5") + + "\r\nContent-Type: text/plain\r\nConnection: close\r\n\r\nhello"}; + ASSERT_TRUE(server.Valid()); + UrlLib::UrlRequest request{}; + request.Open(UrlLib::UrlMethod::Get, server.Url()); + request.ResponseType(responseType); + ASSERT_TRUE(SendAndWait(request)); + EXPECT_EQ(static_cast(request.StatusCode()), truncated ? 0 : status); + if (truncated) + { + EXPECT_FALSE(request.ErrorString().empty()); +#if defined(__APPLE__) + EXPECT_TRUE(request.ResponseString().empty()); + EXPECT_TRUE(request.ResponseBuffer().empty()); + EXPECT_TRUE(request.GetAllResponseHeaders().empty()); +#endif + continue; + } + EXPECT_TRUE(request.ErrorString().empty()); + if (responseType == UrlLib::UrlResponseType::String) + { + EXPECT_EQ(request.ResponseString(), "hello"); + } + else + { + const auto bytes = request.ResponseBuffer(); + EXPECT_EQ(std::string(reinterpret_cast(bytes.data()), bytes.size()), "hello"); + } + } + } + } +} +#endif + +#if defined(__APPLE__) +TEST(UrlRequestErrorReporting, BodyLengthChecksPreserveEncodedAndBodylessResponses) +{ + // gzip("hello"): its 25 wire bytes must not be compared with the 5 decoded bytes. + const std::string gzip{"\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x0a\xcb\x48\xcd\xc9\xc9\x07\x00\x86\xa6\x10\x36\x05\x00\x00\x00", 25}; + struct TestCase + { + int status; + std::string headers; + std::string body; + std::string expected; + }; + const TestCase cases[]{ + {200, "Content-Length: 25\r\nContent-Encoding: gzip\r\n", gzip, "hello"}, + {404, "Content-Length: 25\r\nContent-Encoding: gzip\r\n", gzip, "hello"}, + {200, "Transfer-Encoding: chunked\r\n", "5\r\nhello\r\n0\r\n\r\n", "hello"}, + {404, "Transfer-Encoding: chunked\r\n", "5\r\nhello\r\n0\r\n\r\n", "hello"}, + {204, "", "", ""}, + {304, "Content-Length: 100\r\n", "", ""}, + {404, "Content-Length: 0\r\n", "", ""}, + }; + for (const auto& test : cases) + { + SCOPED_TRACE(std::to_string(test.status) + " " + test.headers); + LoopbackServer server{"HTTP/1.1 " + std::to_string(test.status) + + " Response\r\n" + test.headers + "Connection: close\r\n\r\n" + test.body}; + ASSERT_TRUE(server.Valid()); + UrlLib::UrlRequest request{}; + request.Open(UrlLib::UrlMethod::Get, server.Url()); + ASSERT_TRUE(SendAndWait(request)); + EXPECT_EQ(static_cast(request.StatusCode()), test.status) << request.ErrorString(); + EXPECT_EQ(request.ResponseString(), test.expected); + EXPECT_TRUE(request.ErrorString().empty()); + } +} +#endif + TEST(UrlRequestErrorReporting, MissingLocalFileReportsError) { SKIP_WITHOUT_TRANSPORT_ERROR_DETAIL(); @@ -465,7 +589,7 @@ TEST(UrlRequestErrorReporting, AbortInterruptsInFlightRequest) // symbol assertions below are gated to the backends that do. SKIP_WITHOUT_TRANSPORT_ERROR_DETAIL(); - HangingServer server{}; + LoopbackServer server{}; ASSERT_TRUE(server.Valid()); UrlLib::UrlRequest request{};