Bug 127421 – [curl] Improve realm string parsing in WWW-Authenticate headers

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Szabolcs David]

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"

[Szabolcs David]

Created attachment 221852 [details]
Proposed patch

[Brent Fulgham]

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) ...

[Szabolcs David]

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.

[Brent Fulgham]

Comment on attachment 221966 [details]
Proposed patch II.

r=me

[WebKit Commit Bot]

Comment on attachment 221966 [details]
Proposed patch II.

Clearing flags on attachment: 221966

Committed r163091: <http://trac.webkit.org/changeset/163091>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.