Bug 230935 – Authorization header lost on 30x redirects

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

When making a request to an endpoint that returns 303 with a Location header, the Authorization header is not being sent on the second request. Behavior seems to be the same with the other 3xx redirect status codes. 
Example https://stackblitz.com/edit/js-xkjyaj?file=index.js

[Alexey Proskuryakov]

Confirmed that this test behaves differently between Safari and Chrome.

[Radar WebKit Bug Importer]

<rdar://problem/83689955>

[Chris Dumez]

https://fetch.spec.whatwg.org/#concept-http-redirect-fetch says to use the same request as the original one except for the URL in this case. 

NetworkResourceLoader::willSendRedirectedRequest() gets called with a redirectRequest which already doesn't include the Authorization header so I guess it is not WebKit that's dropping it but likely CFNetwork under us?

[Chris Dumez]

(In reply to Chris Dumez from comment #3)
> https://fetch.spec.whatwg.org/#concept-http-redirect-fetch says to use the
> same request as the original one except for the URL in this case. 
> 
> NetworkResourceLoader::willSendRedirectedRequest() gets called with a
> redirectRequest which already doesn't include the Authorization header so I
> guess it is not WebKit that's dropping it but likely CFNetwork under us?

I added logging inside:
- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task willPerformHTTPRedirection:(NSHTTPURLResponse *)response newRequest:(NSURLRequest *)request completionHandler:(void (^)(NSURLRequest *))completionHandler;

And I see that the NSURLRequest provided by CFNetwork for the redirect is missing the Authorization header. I believe this confirms that CFNetwork is indeed dropping the header. I guess WebKit could add the authorization header back after getting the request from CFNetwork but it would feel a bit hacking. I am curious why CFNetwork would be dropping it.

[Chris Dumez]

Created attachment 440087 [details]
WIP Patch

[Alex Christensen]

Comment on attachment 440087 [details]
WIP Patch

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

> Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:501
> +        if (auto authorization = m_firstRequest.httpHeaderField(WebCore::HTTPHeaderName::Authorization); !authorization.isNull())

We likely only want to do this if it's not a cross-origin redirect.  Line 470 could be reused to tell.

[Chris Dumez]

Comment on attachment 440087 [details]
WIP Patch

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

>> Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:501
>> +        if (auto authorization = m_firstRequest.httpHeaderField(WebCore::HTTPHeaderName::Authorization); !authorization.isNull())
> 
> We likely only want to do this if it's not a cross-origin redirect.  Line 470 could be reused to tell.

That's what I did, no? That's why I am in the else case (the if case on line 494 clears the Authorization header).

[Alex Christensen]

Comment on attachment 440087 [details]
WIP Patch

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

>>> Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:501
>>> +        if (auto authorization = m_firstRequest.httpHeaderField(WebCore::HTTPHeaderName::Authorization); !authorization.isNull())
>> 
>> We likely only want to do this if it's not a cross-origin redirect.  Line 470 could be reused to tell.
> 
> That's what I did, no? That's why I am in the else case (the if case on line 494 clears the Authorization header).

Oh, you're right.

[Chris Dumez]

Created attachment 440098 [details]
Patch

[Chris Dumez]

Created attachment 440111 [details]
Patch

[Chris Dumez]

Created attachment 440217 [details]
Patch

[EWS]

Committed r283565 (242530@main): <https://commits.webkit.org/242530@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440217 [details].

Thanks for verifying and making the change! I'm not familiar with the process, how do I find out when I can test this in browsers?

[Chris Dumez]

(In reply to rli from comment #13)
> Thanks for verifying and making the change! I'm not familiar with the
> process, how do I find out when I can test this in browsers?

We are unfortunately unable to provide insight about when Apple will ship a particular WebKit fix in Safari. What I can say is that you'll be able to test this change either in the next Safari Technology Preview build or the one after that (You can check the STP changelog to confirm this change is in there).

When it comes to regular Safari, all I can say is that historically, Safari has been picking up trunk WebKit every 6 months.

Thank you!

[Arcady Goldmints-Orlov]

*** Bug 168433 has been marked as a duplicate of this bug. ***

[youenn fablet]

*** Bug 239944 has been marked as a duplicate of this bug. ***

[youenn fablet]

It shipped in iOS 15.4/macOS12.3 Safaris