BUrlRequest: fix various issues

* Remove unused headers interface from BUrlProtocol
* Change confusing function names
* Style fixes and whitespace cleanup

headers/os/net/UrlProtocol.h

@@ -58,8 +58,6 @@ public:
 			BUrlContext*			Context() const;
 			BUrlProtocolListener*	Listener() const;
 			const BString&			Protocol() const;
-	// TODO: Does not belong here.
-			BHttpHeaders&			Headers() { return fRequestHeaders; }
 
 	// URL protocol informations
 			bool					IsRunning() const;
@@ -82,8 +80,6 @@ protected:
 
 protected:
 			BUrl					fUrl;
-			BHttpHeaders			fRequestHeaders;
-				// TODO: Does not belong here.
 			BUrlResult*				fResult;
 			BUrlContext*			fContext;
 			BUrlProtocolListener*	fListener;

headers/os/net/UrlRequest.h

@@ -22,10 +22,10 @@ enum {
 class BUrlRequest {
 public:
 									BUrlRequest(const BUrl& url,
-										BUrlProtocolListener* listener);
+										BUrlProtocolListener* listener = NULL,
-									BUrlRequest(const BUrl& url);
+										BUrlContext* context = NULL);
 									BUrlRequest(const BUrlRequest& other);
-	virtual							~BUrlRequest() { };
+	virtual							~BUrlRequest();
 
 	// Request parameters modification
 			status_t				SetUrl(const BUrl& url);
@@ -40,15 +40,13 @@ public:
 			const BUrl&				Url();
 
 	// Request control
-			status_t				Identify();
+	virtual	status_t				Start();
-	virtual	status_t				Perform();
-		// TODO: Rename to Run() perhaps? "Perform" is used for FBC stuff.
 	virtual status_t				Pause();
 	virtual status_t				Resume();
 	virtual status_t				Abort();
 
 	// Request informations
-	virtual	bool					InitCheck() const;
+	virtual	status_t				InitCheck() const;
 			bool					IsRunning() const;
 			status_t				Status() const;
 
@@ -62,7 +60,10 @@ protected:
 			BUrlResult				fResult;
 			BUrlContext*			fContext;
 			BUrl					fUrl;
-			bool					fReady;
+			status_t				fInitStatus;
+
+private:
+			status_t				_SetupProtocol();
 };
 
 #endif // _B_URL_REQUEST_H_

src/kits/network/libnetapi/UrlProtocolHttp.cpp

@@ -32,8 +32,8 @@ static const char* kHttpProtocolThreadStrStatus[
 
 
 BUrlProtocolHttp::BUrlProtocolHttp(BUrl& url, bool ssl,
-		const char *protocolName, BUrlProtocolListener* listener, 
+	const char* protocolName, BUrlProtocolListener* listener,
-		BUrlContext* context, BUrlResult* result)
+	BUrlContext* context, BUrlResult* result)
 	:
 	BUrlProtocol(url, listener, context, result, "BUrlProtocol.HTTP", protocolName),
 	fSSL(ssl),
@@ -45,7 +45,6 @@ BUrlProtocolHttp::BUrlProtocolHttp(BUrl& url, bool ssl,
 		fSocket = new BSecureSocket();
 	else
 		fSocket = new BSocket();
-
 }
 
 
@@ -320,8 +319,7 @@ BUrlProtocolHttp::_ProtocolLoop()
 
 	_EmitDebug(B_URL_PROTOCOL_DEBUG_TEXT,
 		"%ld headers and %ld bytes of data remaining",
-		fHeaders.CountHeaders(),
+		fHeaders.CountHeaders(), fInputBuffer.Size());
-		fInputBuffer.Size());
 
 	if (fResult->StatusCode() == 404)
 		return B_PROT_HTTP_NOT_FOUND;
@@ -339,10 +337,7 @@ BUrlProtocolHttp::_ResolveHostName()
 	if (fUrl.HasPort())
 		fRemoteAddr = BNetworkAddress(fUrl.Host(), fUrl.Port());
 	else {
-		if (fSSL)
+		fRemoteAddr = BNetworkAddress(fUrl.Host(), fSSL ? 443 : 80);
-			fRemoteAddr = BNetworkAddress(fUrl.Host(), 443);
-		else
-			fRemoteAddr = BNetworkAddress(fUrl.Host(), 80);
 	}
 
 	if (fRemoteAddr.InitCheck() != B_OK)
@@ -350,11 +345,10 @@ BUrlProtocolHttp::_ResolveHostName()
 
 	//! ProtocolHook:HostnameResolved
 	if (fListener != NULL)
-		fListener->HostnameResolved(this, 
+		fListener->HostnameResolved(this, fRemoteAddr.ToString().String());
-					const_cast<const char*>(fRemoteAddr.ToString().String()));
 
 	_EmitDebug(B_URL_PROTOCOL_DEBUG_TEXT, "Hostname resolved to: %s",
-			   fRemoteAddr.ToString().String());
+		fRemoteAddr.ToString().String());
 
 	return true;
 }
@@ -582,7 +576,7 @@ BUrlProtocolHttp::_MakeRequest()
 	if (readError)
 		return B_PROT_READ_FAILED;
 
-	return fQuit?B_PROT_ABORTED:B_PROT_SUCCESS;
+	return fQuit ? B_PROT_ABORTED : B_PROT_SUCCESS;
 }
 
 
@@ -763,12 +757,12 @@ BUrlProtocolHttp::_AddHeaders()
 
 		fOutputHeaders.AddHeader("Accept", "*/*");
 		fOutputHeaders.AddHeader("Accept-Encoding", "chunked");
-			//  Allow the remote server to send dynamic content by chunks
+			// Allow the remote server to send dynamic content by chunks
 			// rather than waiting for the full content to be generated and
 			// sending us data.
 
 		fOutputHeaders.AddHeader("Connection", "close");
-			//  Let the remote server close the connection after response since
+			// Let the remote server close the connection after response since
 			// we don't handle multiple request on a single connection
 	}
 
@@ -823,30 +817,14 @@ BUrlProtocolHttp::_AddHeaders()
 		&& (fRequestMethod == B_HTTP_POST || fRequestMethod == B_HTTP_PUT))
 		fOutputHeaders.AddHeader("Transfer-Encoding", "chunked");
 
-	// Request headers	
-	for (int32 headerIndex = 0; 
-		headerIndex < fRequestHeaders.CountHeaders(); 
-		headerIndex++) {
-		BHttpHeader& optHeader = fRequestHeaders[headerIndex];
-		int32 replaceIndex = fOutputHeaders.HasHeader(optHeader.Name());
-			
-		//  Add or replace the current option header to the
-		// output header list
-		if (replaceIndex == -1)
-			fOutputHeaders.AddHeader(optHeader.Name(), optHeader.Value());
-		else				
-			fOutputHeaders[replaceIndex].SetValue(optHeader.Value());
-	}
-	
 	// Optional headers specified by the user
 	if (fOptHeaders != NULL) {
-		for (int32 headerIndex = 0; 
+		for (int32 headerIndex = 0; headerIndex < fOptHeaders->CountHeaders();
-			headerIndex < fOptHeaders->CountHeaders(); 
+				headerIndex++) {
-			headerIndex++) {
 			BHttpHeader& optHeader = (*fOptHeaders)[headerIndex];
 			int32 replaceIndex = fOutputHeaders.HasHeader(optHeader.Name());
 
-			//  Add or replace the current option header to the
+			// Add or replace the current option header to the
 			// output header list
 			if (replaceIndex == -1)
 				fOutputHeaders.AddHeader(optHeader.Name(), optHeader.Value());
@@ -859,17 +837,15 @@ BUrlProtocolHttp::_AddHeaders()
 	if (fOptSetCookies && (fContext != NULL)) {
 		BNetworkCookie* cookie;
 
-		for (BNetworkCookieJar::UrlIterator 
+		for (BNetworkCookieJar::UrlIterator it
-			it(fContext->GetCookieJar().GetUrlIterator(fUrl));
+				= fContext->GetCookieJar().GetUrlIterator(fUrl);
-			(cookie = it.Next()) != NULL;
+				(cookie = it.Next()) != NULL;)
-			)
+			fOutputHeaders.AddHeader("Cookie", cookie->RawCookie(false));
-				fOutputHeaders.AddHeader("Cookie", cookie->RawCookie(false));
 	}
 
 	// Write output headers to output stream
-	for (int32 headerIndex = 0;
+	for (int32 headerIndex = 0; headerIndex < fOutputHeaders.CountHeaders();
-		headerIndex < fOutputHeaders.CountHeaders(); 
+			headerIndex++)
-		headerIndex++)
 		_AddOutputBufferLine(fOutputHeaders.HeaderAt(headerIndex).Header());
 }
 

src/kits/network/libnetapi/UrlRequest.cpp

@@ -13,27 +13,15 @@
 #include <Debug.h>
 
 
-BUrlRequest::BUrlRequest(const BUrl& url, BUrlProtocolListener* listener)
+BUrlRequest::BUrlRequest(const BUrl& url, BUrlProtocolListener* listener,
+	BUrlContext* context)
 	:
 	fListener(listener),
 	fUrlProtocol(NULL),
 	fResult(url),
-	fContext(),
+	fContext(context),
 	fUrl(),
-	fReady(false)
+	fInitStatus(B_ERROR)
-{
-	SetUrl(url);
-}
-
-
-BUrlRequest::BUrlRequest(const BUrl& url)
-	: 
-	fListener(NULL),
-	fUrlProtocol(NULL),
-	fResult(url),
-	fContext(),
-	fUrl(),
-	fReady(false)
 {
 	SetUrl(url);
 }
@@ -46,12 +34,18 @@ BUrlRequest::BUrlRequest(const BUrlRequest& other)
 	fResult(other.fUrl),
 	fContext(),
 	fUrl(),
-	fReady(false)
+	fInitStatus(B_ERROR)
 {
 	*this = other;
 }
 
 
+BUrlRequest::~BUrlRequest()
+{
+	if (fUrlProtocol != NULL)
+		delete fUrlProtocol;
+}
+
 // #pragma mark Request parameters modification
 
 
@@ -61,15 +55,13 @@ BUrlRequest::SetUrl(const BUrl& url)
 	fUrl = url;
 	fResult.SetUrl(url);
 
-	if (fUrlProtocol != NULL && url.Protocol() == fUrl.Protocol())
+	if (fUrlProtocol != NULL && url.Protocol() == fUrl.Protocol()) {
 		fUrlProtocol->SetUrl(url);
-	else {
+		return B_OK;
-		status_t err = Identify();
-		if (err != B_OK)
-			return err;
 	}
 
-	return B_OK;
+	fInitStatus = _SetupProtocol();
+	return fInitStatus;
 }
 
 
@@ -129,33 +121,7 @@ BUrlRequest::Url()
 
 
 status_t
-BUrlRequest::Identify()
+BUrlRequest::Start()
-{
-	// TODO: instanciate the correct BUrlProtocol w/ the services roster
-	delete fUrlProtocol;
-	fUrlProtocol = NULL;
-	
-	if (fUrl.Protocol() == "http") {
-		fUrlProtocol = new(std::nothrow) BUrlProtocolHttp(fUrl, false, "HTTP", 
-			                                              fListener, fContext, 
-														  &fResult);
-		fReady = true;
-		return B_OK;
-	} else if (fUrl.Protocol() == "https") {
-		fUrlProtocol = new(std::nothrow) BUrlProtocolHttp(fUrl, true, "HTTPS", 
-			                                              fListener, fContext, 
-														  &fResult);
-		fReady = true;
-		return B_OK;
-	}
-	
-	fReady = false;
-	return B_NO_HANDLER_FOR_PROTOCOL;
-}
-
-
-status_t
-BUrlRequest::Perform()
 {
 	if (fUrlProtocol == NULL) {
 		PRINT(("BUrlRequest::Perform() : Oops, no BUrlProtocol defined!\n"));
@@ -208,10 +174,10 @@ BUrlRequest::Abort()
 // #pragma mark Request informations
 
 
-bool
+status_t
 BUrlRequest::InitCheck() const
 {
-	return fReady;
+	return fInitStatus;
 }
 
 
@@ -251,3 +217,26 @@ BUrlRequest::operator=(const BUrlRequest& other)
 
 	return *this;
 }
+
+
+status_t
+BUrlRequest::_SetupProtocol()
+{
+	// TODO: instanciate the correct BUrlProtocol w/ the services roster
+	delete fUrlProtocol;
+	fUrlProtocol = NULL;
+
+	if (fUrl.Protocol() == "http")
+		fUrlProtocol = new(std::nothrow) BUrlProtocolHttp(fUrl, false, "HTTP",
+			fListener, fContext, &fResult);
+	else if (fUrl.Protocol() == "https")
+		fUrlProtocol = new(std::nothrow) BUrlProtocolHttp(fUrl, true, "HTTPS",
+			fListener, fContext, &fResult);
+	else
+		return B_NO_HANDLER_FOR_PROTOCOL;
+
+	if (fUrlProtocol == NULL)
+		return B_NO_MEMORY;
+
+	return B_OK;
+}

src/kits/network/libnetapi/UrlSynchronousRequest.cpp

@@ -27,7 +27,7 @@ BUrlSynchronousRequest::Perform()
 	SetProtocolListener(this);
 	fRequestComplete = false;
 
-	return BUrlRequest::Perform();
+	return Start();
 }