HttpRequest: Don't trigger listener on auto redirect

When the user enable auto-redirection, what they meant is that they do
not want to handle redirections, thus we should take total control of this
step and hide it from the user. In fact, a majority of in-tree users
write code to disable their listener when a redirection happen.

This should also allow us to simplify BUrlResult, which has been turned
into a BArchivable for the sole reason of "preserving" headers when
auto redirect is enabled when used with BUrlDispatchingListener, which
has been shown to have little (if any) practical usage.

Change-Id: I9b10b81de0a13edbaec25f6b48ed7a4335ea691a
Reviewed-on: https://review.haiku-os.org/c/haiku/+/3081
Reviewed-by: Adrien Destugues <[email protected]>

src/kits/network/libnetservices/HttpRequest.cpp

@@ -590,6 +590,7 @@ BHttpRequest::_MakeRequest()
 
 
 	// Receive loop
+	bool disableListener = false;
 	bool receiveEnd = false;
 	bool parseEnd = false;
 	bool readByChunks = false;
@@ -640,8 +641,16 @@ BHttpRequest::_MakeRequest()
 		if (fRequestStatus < kRequestStatusReceived) {
 			_ParseStatus();
 
+#ifndef LIBNETAPI_DEPRECATED
+			// Deprecated behavior is to not disable the listener on redirect
+			if (fOptFollowLocation
+					&& IsRedirectionStatusCode(fResult.StatusCode()))
+				disableListener = true;
+#endif
+
 			//! ProtocolHook:ResponseStarted
-			if (fRequestStatus >= kRequestStatusReceived && fListener != NULL)
+			if (fRequestStatus >= kRequestStatusReceived && fListener != NULL
+					&& !disableListener)
 				fListener->ResponseStarted(this);
 		}
 
@@ -662,7 +671,7 @@ BHttpRequest::_MakeRequest()
 				}
 
 				//! ProtocolHook:HeadersReceived
-				if (fListener != NULL)
+				if (fListener != NULL && !disableListener)
 					fListener->HeadersReceived(this, fResult);
 
 
@@ -768,7 +777,7 @@ BHttpRequest::_MakeRequest()
 			if (bytesRead >= 0) {
 				bytesReceived += bytesRead;
 
-				if (fListener != NULL) {
+				if (fListener != NULL && !disableListener) {
 					if (decompress) {
 						readError = decompressingStream->WriteExactly(
 							inputTempBuffer, bytesRead);
@@ -791,7 +800,7 @@ BHttpRequest::_MakeRequest()
 				if (bytesTotal >= 0 && bytesReceived >= bytesTotal)
 					receiveEnd = true;
 
-				if (decompress && receiveEnd) {
+				if (decompress && receiveEnd && !disableListener) {
 					readError = decompressingStream->Flush();
 
 					if (readError == B_BUFFER_OVERFLOW)
@@ -803,9 +812,11 @@ BHttpRequest::_MakeRequest()
 					ssize_t size = decompressorStorage.Size();
 					BStackOrHeapArray<char, 4096> buffer(size);
 					size = decompressorStorage.Read(buffer, size);
-					_NotifyDataReceived(buffer, bytesUnpacked, size,
+					if (fListener != NULL) {
-						bytesReceived, bytesTotal);
+						_NotifyDataReceived(buffer, bytesUnpacked, size,
-					bytesUnpacked += size;
+							bytesReceived, bytesTotal);
+						bytesUnpacked += size;
+					}
 				}
 			}
 		}

src/tests/kits/net/service/HttpTest.cpp

@@ -172,6 +172,7 @@ void AddCommonTests(BThreadedTestCaller<T>& testCaller)
 	testCaller.addThread("UploadTest", &T::UploadTest);
 	testCaller.addThread("BasicAuthTest", &T::AuthBasicTest);
 	testCaller.addThread("DigestAuthTest", &T::AuthDigestTest);
+	testCaller.addThread("AutoRedirectTest", &T::AutoRedirectTest);
 }
 
 }
@@ -202,53 +203,7 @@ HttpTest::setUp()
 void
 HttpTest::GetTest()
 {
-	BUrl testUrl(fTestServer.BaseUrl(), "/");
+	_GetTest("/");
-	BUrlContext* context = new BUrlContext();
-	context->AcquireReference();
-
-	std::string expectedResponseBody(
-		"Path: /\r\n"
-		"\r\n"
-		"Headers:\r\n"
-		"--------\r\n"
-		"Host: 127.0.0.1:PORT\r\n"
-		"Accept: */*\r\n"
-		"Accept-Encoding: gzip\r\n"
-		"Connection: close\r\n"
-		"User-Agent: Services Kit (Haiku)\r\n");
-	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<BUrlRequest> requestDeleter(
-		BUrlProtocolRoster::MakeRequest(testUrl, &listener, context));
-	BHttpRequest* request = dynamic_cast<BHttpRequest*>(requestDeleter.Get());
-	CPPUNIT_ASSERT(request != NULL);
-
-	CPPUNIT_ASSERT(request->Run());
-	while (request->IsRunning())
-		snooze(1000);
-
-	CPPUNIT_ASSERT_EQUAL(B_OK, request->Status());
-
-	const BHttpResult& result
-		= dynamic_cast<const BHttpResult&>(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();
 }
 
 
@@ -507,6 +462,13 @@ HttpTest::AuthDigestTest()
 }
 
 
+void
+HttpTest::AutoRedirectTest()
+{
+	_GetTest("/302");
+}
+
+
 /* static */ void
 HttpTest::AddTests(BTestSuite& parent)
 {
@@ -542,6 +504,61 @@ HttpTest::AddTests(BTestSuite& parent)
 }
 
 
+void
+HttpTest::_GetTest(const BString& path)
+{
+	BUrl testUrl(fTestServer.BaseUrl(), path);
+	BUrlContext* context = new BUrlContext();
+	context->AcquireReference();
+
+	std::string expectedResponseBody(
+		"Path: /\r\n"
+		"\r\n"
+		"Headers:\r\n"
+		"--------\r\n"
+		"Host: 127.0.0.1:PORT\r\n"
+		"Accept: */*\r\n"
+		"Accept-Encoding: gzip\r\n"
+		"Connection: close\r\n"
+		"User-Agent: Services Kit (Haiku)\r\n");
+	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<BUrlRequest> requestDeleter(
+		BUrlProtocolRoster::MakeRequest(testUrl, &listener, context));
+	BHttpRequest* request = dynamic_cast<BHttpRequest*>(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<const BHttpResult&>(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();
+}
+
+
 // # pragma mark - HTTPS
 
 

src/tests/kits/net/service/HttpTest.h

@@ -29,9 +29,12 @@ public:
 			void		AuthBasicTest();
 			void		AuthDigestTest();
 			void		ProxyTest();
+			void		AutoRedirectTest();
 
 	static	void		AddTests(BTestSuite& suite);
 
+private:
+			void		_GetTest(const BString& path);
 private:
 			TestServer	fTestServer;
 };

src/tests/kits/net/service/testserver.py

@@ -64,8 +64,10 @@ class RequestHandler(http.server.BaseHTTPRequestHandler):
 
         encoding, response_body = self._build_response_body()
 
-        self.send_response(
+        status_code = extract_desired_status_code_from_path(self.path, 200)
-            extract_desired_status_code_from_path(self.path, 200))
+        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: