Summary: | [curl] Improve realm string parsing in WWW-Authenticate headers | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Szabolcs David <davidsz> | ||||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, galpeter | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Szabolcs David
2014-01-22 03:24:13 PST
Created attachment 221852 [details]
Proposed patch
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) ... 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 on attachment 221966 [details]
Proposed patch II.
r=me
Comment on attachment 221966 [details] Proposed patch II. Clearing flags on attachment: 221966 Committed r163091: <http://trac.webkit.org/changeset/163091> All reviewed patches have been landed. Closing bug. |