Make handling of Http Authentication thread safe

* Each BHttpAuthentication object is locked on all field accesses,
* They are owned by the BUrlContext and never deleted, so there is no
need for reference-counting them,
* The BUrlContext itself is now reference counted, and all BUrlRequests
hold a reference to it.

This makes sure using the BHttpAuthentication objects from requests is
thread-safe.
This commit is contained in:
Adrien Destugues
2014-06-11 14:11:01 +02:00
parent 463ffbfde4
commit 895fa41e0b
6 changed files with 50 additions and 27 deletions
+5 -2
View File
@@ -1,13 +1,14 @@
/* /*
* Copyright 2010 Haiku Inc. All rights reserved. * Copyright 2010-2014 Haiku Inc. All rights reserved.
* Distributed under the terms of the MIT License. * Distributed under the terms of the MIT License.
*/ */
#ifndef _B_HTTP_AUTHENTICATION_H_ #ifndef _B_HTTP_AUTHENTICATION_H_
#define _B_HTTP_AUTHENTICATION_H_ #define _B_HTTP_AUTHENTICATION_H_
#include <Url.h> #include <Locker.h>
#include <String.h> #include <String.h>
#include <Url.h>
// HTTP authentication method // HTTP authentication method
enum BHttpAuthenticationMethod { enum BHttpAuthenticationMethod {
@@ -89,6 +90,8 @@ private:
BHttpAuthenticationQop fDigestQop; BHttpAuthenticationQop fDigestQop;
BString fAuthorizationString; BString fAuthorizationString;
mutable BLocker fLock;
}; };
#endif // _B_HTTP_AUTHENTICATION_H_ #endif // _B_HTTP_AUTHENTICATION_H_
+6 -5
View File
@@ -1,5 +1,5 @@
/* /*
* Copyright 2010 Haiku Inc. All rights reserved. * Copyright 2010-2014 Haiku Inc. All rights reserved.
* Distributed under the terms of the MIT License. * Distributed under the terms of the MIT License.
*/ */
#ifndef _B_URL_CONTEXT_H_ #ifndef _B_URL_CONTEXT_H_
@@ -8,15 +8,16 @@
#include <HttpAuthentication.h> #include <HttpAuthentication.h>
#include <NetworkCookieJar.h> #include <NetworkCookieJar.h>
#include <Referenceable.h>
namespace BPrivate { namespace BPrivate {
template <class key, class value> class HashMap; template <class key, class value> class SynchronizedHashMap;
class HashString; class HashString;
} }
class BUrlContext { class BUrlContext: public BReferenceable {
public: public:
BUrlContext(); BUrlContext();
~BUrlContext(); ~BUrlContext();
@@ -25,7 +26,7 @@ public:
void SetCookieJar( void SetCookieJar(
const BNetworkCookieJar& cookieJar); const BNetworkCookieJar& cookieJar);
void AddAuthentication(const BUrl& url, void AddAuthentication(const BUrl& url,
BHttpAuthentication* const authentication); const BHttpAuthentication& authentication);
// Context accessors // Context accessors
BNetworkCookieJar& GetCookieJar(); BNetworkCookieJar& GetCookieJar();
@@ -33,7 +34,7 @@ public:
private: private:
BNetworkCookieJar fCookieJar; BNetworkCookieJar fCookieJar;
typedef BPrivate::HashMap<BPrivate::HashString, typedef BPrivate::SynchronizedHashMap<BPrivate::HashString,
BHttpAuthentication*> BHttpAuthenticationMap; BHttpAuthentication*> BHttpAuthenticationMap;
BHttpAuthenticationMap* fAuthenticationMap; BHttpAuthenticationMap* fAuthenticationMap;
}; };
+3 -2
View File
@@ -1,5 +1,5 @@
/* /*
* Copyright 2010 Haiku Inc. All rights reserved. * Copyright 2010-2014 Haiku Inc. All rights reserved.
* Distributed under the terms of the MIT License. * Distributed under the terms of the MIT License.
*/ */
#ifndef _B_URL_REQUEST_H_ #ifndef _B_URL_REQUEST_H_
@@ -11,6 +11,7 @@
#include <UrlProtocolListener.h> #include <UrlProtocolListener.h>
#include <UrlResult.h> #include <UrlResult.h>
#include <OS.h> #include <OS.h>
#include <Referenceable.h>
class BUrlRequest { class BUrlRequest {
@@ -52,7 +53,7 @@ protected:
const char* format, ...); const char* format, ...);
protected: protected:
BUrl fUrl; BUrl fUrl;
BUrlContext* fContext; BReference<BUrlContext> fContext;
BUrlProtocolListener* fListener; BUrlProtocolListener* fListener;
bool fQuit; bool fQuit;
@@ -9,9 +9,12 @@
#include <HttpAuthentication.h> #include <HttpAuthentication.h>
#include <cstdlib> #include <stdlib.h>
#include <stdio.h>
#include <AutoLocker.h>
#include <cstdio>
#if DEBUG > 0 #if DEBUG > 0
#define PRINT(x) printf x #define PRINT(x) printf x
#else #else
@@ -56,27 +59,35 @@ BHttpAuthentication::BHttpAuthentication(const BString& username, const BString&
void void
BHttpAuthentication::SetUserName(const BString& username) BHttpAuthentication::SetUserName(const BString& username)
{ {
fLock.Lock();
fUserName = username; fUserName = username;
fLock.Unlock();
} }
void void
BHttpAuthentication::SetPassword(const BString& password) BHttpAuthentication::SetPassword(const BString& password)
{ {
fLock.Lock();
fPassword = password; fPassword = password;
fLock.Unlock();
} }
void void
BHttpAuthentication::SetMethod(BHttpAuthenticationMethod method) BHttpAuthentication::SetMethod(BHttpAuthenticationMethod method)
{ {
fLock.Lock();
fAuthenticationMethod = method; fAuthenticationMethod = method;
fLock.Unlock();
} }
status_t status_t
BHttpAuthentication::Initialize(const BString& wwwAuthenticate) BHttpAuthentication::Initialize(const BString& wwwAuthenticate)
{ {
BPrivate::AutoLocker<BLocker> lock(fLock);
fAuthenticationMethod = B_HTTP_AUTHENTICATION_NONE; fAuthenticationMethod = B_HTTP_AUTHENTICATION_NONE;
fDigestQop = B_HTTP_QOP_NONE; fDigestQop = B_HTTP_QOP_NONE;
@@ -171,6 +182,7 @@ BHttpAuthentication::Initialize(const BString& wwwAuthenticate)
const BString& const BString&
BHttpAuthentication::UserName() const BHttpAuthentication::UserName() const
{ {
BPrivate::AutoLocker<BLocker> lock(fLock);
return fUserName; return fUserName;
} }
@@ -178,6 +190,7 @@ BHttpAuthentication::UserName() const
const BString& const BString&
BHttpAuthentication::Password() const BHttpAuthentication::Password() const
{ {
BPrivate::AutoLocker<BLocker> lock(fLock);
return fPassword; return fPassword;
} }
@@ -185,6 +198,7 @@ BHttpAuthentication::Password() const
BHttpAuthenticationMethod BHttpAuthenticationMethod
BHttpAuthentication::Method() const BHttpAuthentication::Method() const
{ {
BPrivate::AutoLocker<BLocker> lock(fLock);
return fAuthenticationMethod; return fAuthenticationMethod;
} }
@@ -192,6 +206,7 @@ BHttpAuthentication::Method() const
BString BString
BHttpAuthentication::Authorization(const BUrl& url, const BString& method) const BHttpAuthentication::Authorization(const BUrl& url, const BString& method) const
{ {
BPrivate::AutoLocker<BLocker> lock(fLock);
BString authorizationString; BString authorizationString;
switch (fAuthenticationMethod) { switch (fAuthenticationMethod) {
+8 -9
View File
@@ -378,15 +378,14 @@ BHttpRequest::_ProtocolLoop()
if (authentication->Method() == B_HTTP_AUTHENTICATION_NONE) { if (authentication->Method() == B_HTTP_AUTHENTICATION_NONE) {
// There is no authentication context for this // There is no authentication context for this
// url yet, so let's create one. // url yet, so let's create one.
authentication BHttpAuthentication newAuth;
= new(std::nothrow) BHttpAuthentication(); newAuth.Initialize(fHeaders["WWW-Authenticate"]);
if (authentication == NULL) fContext->AddAuthentication(fUrl, newAuth);
status = B_NO_MEMORY;
else { // Get the copy of the authentication we just added.
status = authentication->Initialize( // That copy is owned by the BUrlContext and won't be
fHeaders["WWW-Authenticate"]); // deleted (unlike the temporary object above)
fContext->AddAuthentication(fUrl, authentication); authentication = &fContext->GetAuthentication(fUrl);
}
} }
newRequest = false; newRequest = false;
+11 -7
View File
@@ -52,21 +52,25 @@ BUrlContext::SetCookieJar(const BNetworkCookieJar& cookieJar)
void void
BUrlContext::AddAuthentication(const BUrl& url, BUrlContext::AddAuthentication(const BUrl& url,
BHttpAuthentication* const authentication) const BHttpAuthentication& authentication)
{ {
BString domain = url.Host(); BString domain = url.Host();
domain += url.Path(); domain += url.Path();
BPrivate::HashString hostHash(domain.String(), domain.Length()); BPrivate::HashString hostHash(domain.String(), domain.Length());
fAuthenticationMap->Lock();
BHttpAuthentication* previous = fAuthenticationMap->Get(hostHash); BHttpAuthentication* previous = fAuthenticationMap->Get(hostHash);
// Make sure we don't leak memory by overriding a previous if (previous)
// authentication for the same domain. *previous = authentication;
if (authentication != previous) { else {
fAuthenticationMap->Put(hostHash, authentication); BHttpAuthentication* copy
// replaces the old one, or adds it in case previous == NULL = new(std::nothrow) BHttpAuthentication(authentication);
delete previous; fAuthenticationMap->Put(hostHash, copy);
} }
fAuthenticationMap->Unlock();
} }