|Summary:||[curl] Improve realm string parsing in WWW-Authenticate headers|
|Product:||WebKit||Reporter:||Szabolcs David <davidsz>|
|Component:||WebKit Misc.||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||bfulgham, commit-queue, galpeter|
|Version:||528+ (Nightly build)|
Description Szabolcs David 2014-01-22 03:24:13 PST
The realm string contains quotes at the beginning and end - this is the opposite of the libsoup implementation. Furthermore, if the header is concatenated from two or more another headers, it contains more incorrect part. For example, if the header is: WWW-Authenticate: Basic realm="First realm", Basic realm="Second realm" realm string will be: "First realm", Basic realm="Second realm"
Comment 2 Brent Fulgham 2014-01-22 09:54:16 PST
Comment on attachment 221852 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=221852&action=review Looks good. I have a couple of minor comments for your consideration. > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:351 > String authHeader = response.httpHeaderField("WWW-Authenticate"); I just noticed that this could probably be const, since we aren't modifying it. > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:358 > + if (realm.startsWith('"') && realm.endsWith('"') && realm.length() > 1) What happens if we get the input ""? Are we supposed to create a protection space for the "" realm? Or should we be bailing out early? > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:359 > + realm = realm.substring(1, realm.length()-2); This might be clearer if it were wrapped up as a little function: static void removeLeadingAndTrailingQuotes(String& value) ...
Comment 3 Szabolcs David 2014-01-23 02:36:06 PST
Created attachment 221966 [details] Proposed patch II. Thanks for your comments! > What happens if we get the input ""? Are we supposed to create a protection space for the "" realm? Or should we be bailing out early? I think we should accept the empty string as realm, because the most popular browsers accept it too.
Comment 4 Brent Fulgham 2014-01-30 10:30:09 PST
Comment on attachment 221966 [details] Proposed patch II. r=me
Comment 5 WebKit Commit Bot 2014-01-30 10:57:46 PST
Comment on attachment 221966 [details] Proposed patch II. Clearing flags on attachment: 221966 Committed r163091: <http://trac.webkit.org/changeset/163091>
Comment 6 WebKit Commit Bot 2014-01-30 10:57:48 PST
All reviewed patches have been landed. Closing bug.