Bug 205462 – Fetch: handle empty Location value

• Component: Page Loading
• Status: RESOLVED FIXED

[Rob Buis]

Handle empty Location value.

[Rob Buis]

Created attachment 386126 [details]
Patch

[youenn fablet]

Comment on attachment 386126 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386126&action=review

> Source/WebCore/loader/SubresourceLoader.cpp:405
> +        if (options().redirect == FetchOptions::Redirect::Follow && response.isRedirection()) {

response.isRedirection() is not needed here.

> Source/WebCore/loader/SubresourceLoader.cpp:406
> +            if (response.httpHeaderFields().contains(HTTPHeaderName::Location) && response.httpHeaderField(HTTPHeaderName::Location).isEmpty()) {

Can we just have one check with something like !isNull() && isEmpty(). or impl() && impl()->isEmpty() maybe?

> Source/WebCore/loader/SubresourceLoader.cpp:410
> +            }

Would be nice if we could do something like:
if (options().redirect == FetchOptions::Redirect::Follow && isLocationURLFailure(response)) {
    ...
    return;
}

[Rob Buis]

Created attachment 386200 [details]
Patch

[Rob Buis]

Comment on attachment 386126 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386126&action=review

>> Source/WebCore/loader/SubresourceLoader.cpp:405
>> +        if (options().redirect == FetchOptions::Redirect::Follow && response.isRedirection()) {
> 
> response.isRedirection() is not needed here.

Ouch! Should have seen that, fixed.

>> Source/WebCore/loader/SubresourceLoader.cpp:406
>> +            if (response.httpHeaderFields().contains(HTTPHeaderName::Location) && response.httpHeaderField(HTTPHeaderName::Location).isEmpty()) {
> 
> Can we just have one check with something like !isNull() && isEmpty(). or impl() && impl()->isEmpty() maybe?

Yes, the former, I think it just uses the latter, but best to stick with the public API (even though StringImpl is not likely to change).

>> Source/WebCore/loader/SubresourceLoader.cpp:410
>> +            }
> 
> Would be nice if we could do something like:
> if (options().redirect == FetchOptions::Redirect::Follow && isLocationURLFailure(response)) {
>     ...
>     return;
> }

Done.

[WebKit Commit Bot]

Comment on attachment 386200 [details]
Patch

Clearing flags on attachment: 386200

Committed r253814: <https://trac.webkit.org/changeset/253814>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/58111194>