From 258817720c14e452652803c43c132c9d4b00089e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Wed, 4 Jun 2014 23:54:40 +0200 Subject: [PATCH] HttpAuthentication: improvements and fixes * BString::CopyInfo() takes length as second parameter, not the end-offset. This bug didn't have any effect, since BString clamps the length. * ',' is a comma, ':' is a colon. * When parsing "additionalData" in the loop for name=value pairs, an empty name is not useful, continue the loop early. * "value" may have the length 0, accessing it with value[0] would not lead to an access violation, since it just reads the terminating zero (I think), but it's nicer if the code makes it clear that value being empty is considered. * _H() was using a static buffer, in a heavily multi-threaded situation. I don't think this was healthy. * Implement the proposed optimization of using BString::LockBuffer(). Appending one char at a time is really bad for peformance. The Base64 encoding/decoding should really be rewritten as well for similar reasons. --- .../network/libnetapi/HttpAuthentication.cpp | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/kits/network/libnetapi/HttpAuthentication.cpp b/src/kits/network/libnetapi/HttpAuthentication.cpp index 633632b80f..f5df650865 100644 --- a/src/kits/network/libnetapi/HttpAuthentication.cpp +++ b/src/kits/network/libnetapi/HttpAuthentication.cpp @@ -88,7 +88,7 @@ BHttpAuthentication::Initialize(const BString& wwwAuthenticate) else { wwwAuthenticate.CopyInto(authRequired, 0, firstSpace); wwwAuthenticate.CopyInto(additionalData, firstSpace + 1, - wwwAuthenticate.Length()); + wwwAuthenticate.Length() - (firstSpace + 1)); } authRequired.ToLower(); @@ -102,17 +102,17 @@ BHttpAuthentication::Initialize(const BString& wwwAuthenticate) while (additionalData.Length()) { - int32 firstColon = additionalData.FindFirst(','); - if (firstColon == -1) - firstColon = additionalData.Length(); + int32 firstComma = additionalData.FindFirst(','); + if (firstComma == -1) + firstComma = additionalData.Length(); BString value; - additionalData.MoveInto(value, 0, firstColon); + additionalData.MoveInto(value, 0, firstComma); additionalData.Remove(0, 1); additionalData.Trim(); int32 equal = value.FindFirst('='); - if (equal == -1) + if (equal <= 0) continue; BString name; @@ -120,9 +120,9 @@ BHttpAuthentication::Initialize(const BString& wwwAuthenticate) value.Remove(0, 1); name.ToLower(); - if (value[0] == '"') { + if (value.Length() > 0 && value[0] == '"') { value.Remove(0, 1); - value.Remove(value.Length()-1, 1); + value.Remove(value.Length() - 1, 1); } PRINT(("HttpAuth: name=%s, value=%s\n", name.String(), @@ -287,13 +287,13 @@ BHttpAuthentication::Base64Encode(const BString& string) /*static*/ BString BHttpAuthentication::Base64Decode(const BString& string) { - BString base64Reverse(kBase64Symbols); BString result; - // Invalid input + // Check for invalid input if (string.Length() % 4 != 0) - return BString(""); + return result; + BString base64Reverse(kBase64Symbols); BString tmpString(string); while (tmpString.Length()) { @@ -373,25 +373,26 @@ BString BHttpAuthentication::_H(const BString& value) const { MD5_CTX context; - static unsigned char hashResult[MD5_DIGEST_LENGTH]; + uchar hashResult[MD5_DIGEST_LENGTH]; MD5_Init(&context); MD5_Update(&context, (void *)(value.String()), value.Length()); MD5_Final(hashResult, &context); BString result; - // TODO: This is slower than it needs to be. If we already know the - // final hash string length, we can use - // BString::LockBuffer(MD5_DIGEST_LENGTH * 2) to preallocate it. - // I am not making the change since I can not test it right now (stippi). + // Preallocate the string + char* resultChar = result.LockBuffer(MD5_DIGEST_LENGTH * 2); for (int i = 0; i < MD5_DIGEST_LENGTH; i++) { char c = ((hashResult[i] & 0xF0) >> 4); c += (c > 9) ? 'a' - 10 : '0'; - result << c; + resultChar[0] = c; + resultChar++; c = hashResult[i] & 0x0F; c += (c > 9) ? 'a' - 10 : '0'; - result << c; + resultChar[0] = c; + resultChar++; } + result.UnlockBuffer(MD5_DIGEST_LENGTH * 2); return result; }