Bug 197650 – [Curl] Suppress extra didReceiveAuthenticationChallenge call when accessing a server which checks basic auth.

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Takashi Komori]

When Curl port accesses a page which checks Basic Authentication credential and server trust challenge occurs, Curl port calls extra didReceiveAuthenticationChallenge unnecessarily.
This is because Curl port discards information about allowed server trust challenge before in NetworkDataTaskCurl::restartWithCredential.

[Takashi Komori]

Created attachment 369257 [details]
Patch

[EWS Watchlist]

Comment on attachment 369257 [details]
Patch

Attachment 369257 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12121376

New failing tests:
http/tests/misc/certificate-and-authentication.html

[EWS Watchlist]

Created attachment 369262 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369257 [details]
Patch

Attachment 369257 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12121562

New failing tests:
http/tests/misc/certificate-and-authentication.html

[EWS Watchlist]

Created attachment 369265 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369257 [details]
Patch

Attachment 369257 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12121606

New failing tests:
webgl/2.0.0/conformance/context/context-release-upon-reload.html
http/tests/misc/certificate-and-authentication.html

[EWS Watchlist]

Created attachment 369269 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369257 [details]
Patch

Attachment 369257 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12121633

New failing tests:
http/tests/misc/certificate-and-authentication.html

[EWS Watchlist]

Created attachment 369270 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.4

[EWS Watchlist]

Comment on attachment 369257 [details]
Patch

Attachment 369257 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12121882

New failing tests:
http/tests/misc/certificate-and-authentication.html

[EWS Watchlist]

Created attachment 369272 [details]
Archive of layout-test-results from ews214 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews214  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Takashi Komori]

Created attachment 369456 [details]
Patch

[Takashi Komori]

(In reply to Takashi Komori from comment #12)
> Created attachment 369456 [details]
> Patch

Skip added test on other ports.
Soup doesn't use didReceiveChallenge for server trust evaluation.
We use certificate-and-authentication.html test for only wincairo.

[EWS Watchlist]

Comment on attachment 369456 [details]
Patch

Attachment 369456 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12139490

New failing tests:
http/tests/css/filters-on-iframes.html

[EWS Watchlist]

Created attachment 369459 [details]
Archive of layout-test-results from ews213 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews213  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Takashi Komori]

Created attachment 369469 [details]
Patch

[Fujii Hironori]

Comment on attachment 369469 [details]
Patch

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

> Source/WebCore/ChangeLog:7
> +

Please add the comment #0 here.

> LayoutTests/http/tests/misc/resources/certificate-and-authentication.php:13
> +parent.postMessage("loaded", "*");

Do you need to use postMessage? Can you use onload as well as http/tests/ssl/certificate-validation.html .
If so, you can reuse a existing auth cgi.

And, I think it looks cool using async and await such like http/tests/ssl/certificate-validation.html.

[Takashi Komori]

(In reply to Fujii Hironori from comment #17)
> Do you need to use postMessage? Can you use onload as well as
> http/tests/ssl/certificate-validation.html .
> If so, you can reuse a existing auth cgi.

postMessage might be replaced by onload.
What does "existing auth cgi" refer to exactly?

[Fujii Hironori]

For example, can you use tests/xmlhttprequest/resources/basic-auth/basic-auth.php ?

[Basuke Suzuki]

(In reply to Fujii Hironori from comment #19)
> For example, can you use
> tests/xmlhttprequest/resources/basic-auth/basic-auth.php ?

I've got a review that we shuoldn't reuse resources other bug created except those inside root /resources/. In this case, it's easy to find similar feature from there.

[Basuke Suzuki]

(In reply to Basuke Suzuki from comment #20)
> (In reply to Fujii Hironori from comment #19)
> > For example, can you use
> > tests/xmlhttprequest/resources/basic-auth/basic-auth.php ?
> 
> I've got a review that we shuoldn't reuse resources other bug created except
> those inside root /resources/. In this case, it's easy to find similar
> feature from there.

Oh my got, the only one in /resources/ was for digest-auth, which was posted by me lol. Forget about my comment. Sorry.

[Takashi Komori]

Created attachment 369540 [details]
Patch

[Takashi Komori]

(In reply to Fujii Hironori from comment #17)
> Comment on attachment 369469 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=369469&action=review
> 
> > Source/WebCore/ChangeLog:7
> > +

Added.

> Please add the comment #0 here.
> 
> > LayoutTests/http/tests/misc/resources/certificate-and-authentication.php:13
> > +parent.postMessage("loaded", "*");
> 
> Do you need to use postMessage? Can you use onload as well as
> http/tests/ssl/certificate-validation.html .
> If so, you can reuse a existing auth cgi.
> 
> And, I think it looks cool using async and await such like
> http/tests/ssl/certificate-validation.html.

Fixed with tests/xmlhttprequest/resources/basic-auth/basic-auth.php

[Fujii Hironori]

Comment on attachment 369540 [details]
Patch

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

> LayoutTests/TestExpectations:451
> +http/tests/misc/certificate-and-authentication.html [ Skip ]

This test is skipped not only becasue it is only work in WebKit2, but also because it is only for curl port.
Let's rename to http/tests/ssl/curl/certificate-and-authentication.html,
and skip the directory.
http/tests/ssl/curl [ Skip ]

> LayoutTests/http/tests/misc/certificate-and-authentication.html:32
> +    const iframe = await with_iframe("https://localhost:8443/xmlhttprequest/resources/basic-auth/basic-auth.php?uid=user");

See Basuke's Comment 20. You should move the script into the http/tests/resources or copy the script.

[Takashi Komori]

Created attachment 369649 [details]
Patch

[Takashi Komori]

(In reply to Fujii Hironori from comment #24)
> Comment on attachment 369540 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=369540&action=review
> 
> > LayoutTests/TestExpectations:451
> > +http/tests/misc/certificate-and-authentication.html [ Skip ]
> 
> This test is skipped not only becasue it is only work in WebKit2, but also
> because it is only for curl port.
> Let's rename to http/tests/ssl/curl/certificate-and-authentication.html,
> and skip the directory.
> http/tests/ssl/curl [ Skip ]

Renamed.

> > LayoutTests/http/tests/misc/certificate-and-authentication.html:32
> > +    const iframe = await with_iframe("https://localhost:8443/xmlhttprequest/resources/basic-auth/basic-auth.php?uid=user");
> 
> See Basuke's Comment 20. You should move the script into the
> http/tests/resources or copy the script.

Copied.

[WebKit Commit Bot]

Comment on attachment 369649 [details]
Patch

Clearing flags on attachment: 369649

Committed r245215: <https://trac.webkit.org/changeset/245215>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/50709033>