Bug 184723 – Allow SameOrigin credentials handling for synchronous XHR

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[youenn fablet]

Allow cross-origin redirections for synchronous XHR

[youenn fablet]

Created attachment 338185 [details]
Patch

[youenn fablet]

Created attachment 338195 [details]
Patch

[Alex Christensen]

Comment on attachment 338195 [details]
Patch

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

Are these WK1 regressions, or are we just preserving existing behavior and that's hard to see in the patch?

> Source/WebKit/NetworkProcess/NetworkResourceLoader.h:134
> +    void startNetworkLoad(WebCore::ResourceRequest&&, FirstLoad = FirstLoad::Yes);

I'd prefer to not have a default parameter so we don't accidentally omit the parameter when writing redirection code.

[youenn fablet]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 338195 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=338195&action=review
> 
> Are these WK1 regressions, or are we just preserving existing behavior and
> that's hard to see in the patch?

For WK1, we keep the previous behavior (no change to WebCore nor WebKitLegacy) which is to not follow any cross origin redirection.

> 
> > Source/WebKit/NetworkProcess/NetworkResourceLoader.h:134
> > +    void startNetworkLoad(WebCore::ResourceRequest&&, FirstLoad = FirstLoad::Yes);
> 
> I'd prefer to not have a default parameter so we don't accidentally omit the
> parameter when writing redirection code.

There are 6 calls to startNetworkLoad so I would tend to prefer keeping it, but I can make the change.
If we are not using the right parameter, there will be crashes anyway since we are consuming sandbox extensions.

[Alex Christensen]

Comment on attachment 338195 [details]
Patch

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

>>> Source/WebKit/NetworkProcess/NetworkResourceLoader.h:134
>>> +    void startNetworkLoad(WebCore::ResourceRequest&&, FirstLoad = FirstLoad::Yes);
>> 
>> I'd prefer to not have a default parameter so we don't accidentally omit the parameter when writing redirection code.
> 
> There are 6 calls to startNetworkLoad so I would tend to prefer keeping it, but I can make the change.
> If we are not using the right parameter, there will be crashes anyway since we are consuming sandbox extensions.

I'd still prefer to add explicit calls.

[youenn fablet]

Created attachment 338273 [details]
Patch for landing

[youenn fablet]

(In reply to Alex Christensen from comment #5)
> Comment on attachment 338195 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=338195&action=review
> 
> >>> Source/WebKit/NetworkProcess/NetworkResourceLoader.h:134
> >>> +    void startNetworkLoad(WebCore::ResourceRequest&&, FirstLoad = FirstLoad::Yes);
> >> 
> >> I'd prefer to not have a default parameter so we don't accidentally omit the parameter when writing redirection code.
> > 
> > There are 6 calls to startNetworkLoad so I would tend to prefer keeping it, but I can make the change.
> > If we are not using the right parameter, there will be crashes anyway since we are consuming sandbox extensions.
> 
> I'd still prefer to add explicit calls.

Done in the uploaded patch

[WebKit Commit Bot]

Comment on attachment 338273 [details]
Patch for landing

Clearing flags on attachment: 338273

Committed r230791: <https://trac.webkit.org/changeset/230791>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/39548418>