From 3ca5eec002c54d7623ab85b66254ec2f31851cc9 Mon Sep 17 00:00:00 2001 From: Adrien Destugues Date: Sat, 3 Jul 2021 11:04:34 +0200 Subject: [PATCH] libnetservices: fix handling of HEAD requests and 204 responses We were incorrectly reporting a B_IO_ERROR for these requests because we could not read the content after the headers. There is no content in these cases. Add an unit test for both HEAD and 204 status, checking that there is no content and the headers are correct. Fixes #16885. Change-Id: I98fefc5c604253bb2545b50395b7af9f8834def0 Reviewed-on: https://review.haiku-os.org/c/haiku/+/4142 Tested-by: Commit checker robot Reviewed-by: Niels Sascha Reedijk --- .../network/libnetservices/HttpRequest.cpp | 22 ++++- .../libnetservices/UrlProtocolListener.cpp | 13 +-- src/tests/kits/net/service/HttpTest.cpp | 92 +++++++++++++++++++ src/tests/kits/net/service/HttpTest.h | 2 + src/tests/kits/net/service/testserver.py | 13 ++- 5 files changed, 126 insertions(+), 16 deletions(-) diff --git a/src/kits/network/libnetservices/HttpRequest.cpp b/src/kits/network/libnetservices/HttpRequest.cpp index c25ef941b4..4326850bdc 100644 --- a/src/kits/network/libnetservices/HttpRequest.cpp +++ b/src/kits/network/libnetservices/HttpRequest.cpp @@ -684,8 +684,7 @@ BHttpRequest::_MakeRequest() // example in the case of a chunked transfer, we can't know // - If the request method is "HEAD" which explicitly asks the // server to not send any data (only the headers) - if (bytesTotal > 0 && bytesReceived != bytesTotal - && fRequestMethod != B_HTTP_HEAD) { + if (bytesTotal > 0 && bytesReceived != bytesTotal) { readError = B_IO_ERROR; break; } @@ -752,6 +751,14 @@ BHttpRequest::_MakeRequest() bytesTotal = atoll(fHeaders.HeaderAt(index).Value()); else bytesTotal = -1; + + if (fRequestMethod == B_HTTP_HEAD + || fResult.StatusCode() == 204) { + // In the case of a HEAD request or if the server replies + // 204 ("no content"), we don't expect to receive anything + // more, and the socket will be closed. + receiveEnd = true; + } } } @@ -968,8 +975,7 @@ BHttpRequest::_MakeRequest() // example in the case of a chunked transfer, we can't know // - If the request method is "HEAD" which explicitly asks the // server to not send any data (only the headers) - if (bytesTotal > 0 && bytesReceived != bytesTotal - && fRequestMethod != B_HTTP_HEAD) { + if (bytesTotal > 0 && bytesReceived != bytesTotal) { readError = B_IO_ERROR; break; } @@ -1047,6 +1053,14 @@ BHttpRequest::_MakeRequest() bytesTotal = atoll(fHeaders.HeaderAt(index).Value()); else bytesTotal = -1; + + if (fRequestMethod == B_HTTP_HEAD + || fResult.StatusCode() == 204) { + // In the case of a HEAD request or if the server replies + // 204 ("no content"), we don't expect to receive anything + // more, and the socket will be closed. + receiveEnd = true; + } } } diff --git a/src/kits/network/libnetservices/UrlProtocolListener.cpp b/src/kits/network/libnetservices/UrlProtocolListener.cpp index f2fad72a66..0a79e6aa21 100644 --- a/src/kits/network/libnetservices/UrlProtocolListener.cpp +++ b/src/kits/network/libnetservices/UrlProtocolListener.cpp @@ -8,9 +8,6 @@ */ #include -#ifdef DEBUG -#include -#endif #include #include @@ -111,24 +108,24 @@ BUrlProtocolListener::DebugMessage(BUrlRequest* caller, #ifdef DEBUG switch (type) { case B_URL_PROTOCOL_DEBUG_TEXT: - cout << " "; + fprintf(stderr, " "); break; case B_URL_PROTOCOL_DEBUG_ERROR: - cout << "!!!"; + fprintf(stderr, "!!!"); break; case B_URL_PROTOCOL_DEBUG_TRANSFER_IN: case B_URL_PROTOCOL_DEBUG_HEADER_IN: - cout << "<--"; + fprintf(stderr, "<--"); break; case B_URL_PROTOCOL_DEBUG_TRANSFER_OUT: case B_URL_PROTOCOL_DEBUG_HEADER_OUT: - cout << "-->"; + fprintf(stderr, "-->"); break; } - cout << " " << caller->Protocol() << ": " << text << endl; + fprintf(stderr, " %s: %s\n", caller->Protocol().String(), text); #endif } diff --git a/src/tests/kits/net/service/HttpTest.cpp b/src/tests/kits/net/service/HttpTest.cpp index 22080ed014..2dc17e90f3 100644 --- a/src/tests/kits/net/service/HttpTest.cpp +++ b/src/tests/kits/net/service/HttpTest.cpp @@ -168,6 +168,8 @@ template void AddCommonTests(BThreadedTestCaller& testCaller) { testCaller.addThread("GetTest", &T::GetTest); + testCaller.addThread("HeadTest", &T::HeadTest); + testCaller.addThread("NoContentTest", &T::NoContentTest); testCaller.addThread("UploadTest", &T::UploadTest); testCaller.addThread("BasicAuthTest", &T::AuthBasicTest); testCaller.addThread("DigestAuthTest", &T::AuthDigestTest); @@ -206,6 +208,96 @@ HttpTest::GetTest() } +void +HttpTest::HeadTest() +{ + BUrl testUrl(fTestServer.BaseUrl(), "/"); + BUrlContext* context = new BUrlContext(); + context->AcquireReference(); + + std::string expectedResponseBody(""); + HttpHeaderMap expectedResponseHeaders; + expectedResponseHeaders["Content-Encoding"] = "gzip"; + expectedResponseHeaders["Content-Length"] = "144"; + expectedResponseHeaders["Content-Type"] = "text/plain"; + expectedResponseHeaders["Date"] = "Sun, 09 Feb 2020 19:32:42 GMT"; + expectedResponseHeaders["Server"] = "Test HTTP Server for Haiku"; + + TestListener listener(expectedResponseBody, expectedResponseHeaders); + + ObjectDeleter requestDeleter( + BUrlProtocolRoster::MakeRequest(testUrl, &listener, &listener, + context)); + BHttpRequest* request = dynamic_cast(requestDeleter.Get()); + CPPUNIT_ASSERT(request != NULL); + + request->SetAutoReferrer(false); + request->SetMethod("HEAD"); + + CPPUNIT_ASSERT(request->Run()); + while (request->IsRunning()) + snooze(1000); + + CPPUNIT_ASSERT_EQUAL(B_OK, request->Status()); + + const BHttpResult& result + = dynamic_cast(request->Result()); + CPPUNIT_ASSERT_EQUAL(200, result.StatusCode()); + CPPUNIT_ASSERT_EQUAL(BString("OK"), result.StatusText()); + + CPPUNIT_ASSERT_EQUAL(144, result.Length()); + + listener.Verify(); + + CPPUNIT_ASSERT(!context->GetCookieJar().GetIterator().HasNext()); + // This page should not set cookies + + context->ReleaseReference(); +} + + +void +HttpTest::NoContentTest() +{ + BUrl testUrl(fTestServer.BaseUrl(), "/204"); + BUrlContext* context = new BUrlContext(); + context->AcquireReference(); + + std::string expectedResponseBody(""); + HttpHeaderMap expectedResponseHeaders; + expectedResponseHeaders["Date"] = "Sun, 09 Feb 2020 19:32:42 GMT"; + expectedResponseHeaders["Server"] = "Test HTTP Server for Haiku"; + + TestListener listener(expectedResponseBody, expectedResponseHeaders); + + ObjectDeleter requestDeleter( + BUrlProtocolRoster::MakeRequest(testUrl, &listener, &listener, + context)); + BHttpRequest* request = dynamic_cast(requestDeleter.Get()); + CPPUNIT_ASSERT(request != NULL); + + request->SetAutoReferrer(false); + + CPPUNIT_ASSERT(request->Run()); + while (request->IsRunning()) + snooze(1000); + + CPPUNIT_ASSERT_EQUAL(B_OK, request->Status()); + + const BHttpResult& result + = dynamic_cast(request->Result()); + CPPUNIT_ASSERT_EQUAL(204, result.StatusCode()); + CPPUNIT_ASSERT_EQUAL(BString("No Content"), result.StatusText()); + + listener.Verify(); + + CPPUNIT_ASSERT(!context->GetCookieJar().GetIterator().HasNext()); + // This page should not set cookies + + context->ReleaseReference(); +} + + void HttpTest::ProxyTest() { diff --git a/src/tests/kits/net/service/HttpTest.h b/src/tests/kits/net/service/HttpTest.h index 3e853e11a9..79c61fd0dc 100644 --- a/src/tests/kits/net/service/HttpTest.h +++ b/src/tests/kits/net/service/HttpTest.h @@ -25,6 +25,8 @@ public: virtual void setUp(); void GetTest(); + void HeadTest(); + void NoContentTest(); void UploadTest(); void AuthBasicTest(); void AuthDigestTest(); diff --git a/src/tests/kits/net/service/testserver.py b/src/tests/kits/net/service/testserver.py index ba87e0fcd5..74143a0ae2 100644 --- a/src/tests/kits/net/service/testserver.py +++ b/src/tests/kits/net/service/testserver.py @@ -68,10 +68,15 @@ class RequestHandler(http.server.BaseHTTPRequestHandler): self.send_response(status_code) if status_code >= 300 and status_code < 400: self.send_header('Location', '/') - self.send_header('Content-Type', 'text/plain') - self.send_header('Content-Length', str(len(response_body))) - if encoding: - self.send_header('Content-Encoding', encoding) + + if status_code == 204: + write_response = False + else: + self.send_header('Content-Type', 'text/plain') + self.send_header('Content-Length', str(len(response_body))) + if encoding: + self.send_header('Content-Encoding', encoding) + for header_name, header_value in extra_headers: self.send_header(header_name, header_value) self.end_headers()