Bug 189318 – [Curl] Respond with requested authentication scheme for authentication challenge.

• Component: Platform
• Status: RESOLVED FIXED

[Basuke Suzuki]

Curl port depends on libcurl's authentication handling by enabling CURLAUTH_ANY. With this mode, the round-trip communication between the client and the server is handled by libcurl internally. That's okay for many cases. But when initial request has a credentials (i.e. XMLHttpRequest), there's no valid chance to store credential to the storage because the returned response is not 401.

[Basuke Suzuki]

Created attachment 349489 [details]
PATCH

[Basuke Suzuki]

Comment on attachment 349489 [details]
PATCH

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

> Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:335
> +std::optional<Credential> ResourceHandle::getCredential(const ResourceRequest& request, bool redirect)

These changes follow the Mac port implementation.

> Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:403
> +    }

To set the default authentication in the credential storage.

[Alex Christensen]

Comment on attachment 349489 [details]
PATCH

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

Do we need to do something similar for NetworkDataTask?

> LayoutTests/ChangeLog:3
> +        [Curl] Response with collect authentication scheme for authentication challenge.

This title doesn't make sense.

[Basuke Suzuki]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 349489 [details]
> PATCH
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=349489&action=review
> 
> Do we need to do something similar for NetworkDataTask?

Yes, but we don't get ready for WebKitTestRunner for WinCairo yet. I'm not comfortable to fix the behavior without testing because these sequence is complicated. As my first comment mentioned, the behavior is almost okay without this fix. After our effort to the WebKitTestRunner finish, we will port this to NetworkDataTask. That's my idea.

> > LayoutTests/ChangeLog:3
> > +        [Curl] Response with collect authentication scheme for authentication challenge.
> 
> This title doesn't make sense.

How about new one?
"Respond with requested authentication scheme for authentication challenge."

[Basuke Suzuki]

Created attachment 349575 [details]
FIX

[Alex Christensen]

Comment on attachment 349575 [details]
FIX

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

I think it's a bad idea to change ResourceHandle but not NetworkDataTask.

> Source/WebCore/ChangeLog:15
> +        - http/tests/websocket/tests/hybi/httponly-cookie.pl [ Failure ]

No need for [ Failure ] here.

[Basuke Suzuki]

(In reply to Alex Christensen from comment #6)
> Comment on attachment 349575 [details]
> FIX
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=349575&action=review
> 
> I think it's a bad idea to change ResourceHandle but not NetworkDataTask.

But fixing without testing is also bad. Okay, I do manual test using MiniBrowser for NDT.

[Basuke Suzuki]

Created attachment 349828 [details]
PATCH

Add NetworkDataTask implementation.

[WebKit Commit Bot]

Comment on attachment 349828 [details]
PATCH

Rejecting attachment 349828 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 349828, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=349828&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=189318&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 349828 from bug 189318.
Fetching: https://bugs.webkit.org/attachment.cgi?id=349828
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Alex Christensen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 12 diffs from patch file(s).
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/wincairo/TestExpectations
Hunk #1 FAILED at 915.
Hunk #2 FAILED at 935.
2 out of 2 hunks FAILED -- saving rejects to file LayoutTests/platform/wincairo/TestExpectations.rej
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/platform/network/ResourceHandle.h
patching file Source/WebCore/platform/network/curl/CurlContext.cpp
patching file Source/WebCore/platform/network/curl/CurlContext.h
patching file Source/WebCore/platform/network/curl/CurlRequest.cpp
patching file Source/WebCore/platform/network/curl/CurlRequest.h
patching file Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
patching file Source/WebKit/ChangeLog
patching file Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
patching file Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Alex Christensen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/9222335

[WebKit Commit Bot]

Comment on attachment 349828 [details]
PATCH

Rejecting attachment 349828 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 349828, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=349828&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=189318&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 349828 from bug 189318.
Fetching: https://bugs.webkit.org/attachment.cgi?id=349828
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Alex Christensen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 12 diffs from patch file(s).
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/wincairo/TestExpectations
Hunk #1 FAILED at 915.
Hunk #2 FAILED at 935.
2 out of 2 hunks FAILED -- saving rejects to file LayoutTests/platform/wincairo/TestExpectations.rej
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/platform/network/ResourceHandle.h
patching file Source/WebCore/platform/network/curl/CurlContext.cpp
patching file Source/WebCore/platform/network/curl/CurlContext.h
patching file Source/WebCore/platform/network/curl/CurlRequest.cpp
Hunk #2 succeeded at 211 (offset 3 lines).
patching file Source/WebCore/platform/network/curl/CurlRequest.h
Hunk #1 succeeded at 34 with fuzz 1.
Hunk #2 succeeded at 68 with fuzz 2 (offset 1 line).
Hunk #3 succeeded at 167 (offset 2 lines).
patching file Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
Hunk #1 succeeded at 85 with fuzz 2 (offset 2 lines).
Hunk #2 succeeded at 233 (offset 3 lines).
Hunk #3 succeeded at 258 (offset 3 lines).
Hunk #4 succeeded at 293 (offset 3 lines).
Hunk #5 succeeded at 335 (offset 3 lines).
Hunk #6 succeeded at 352 (offset 3 lines).
Hunk #7 FAILED at 373.
Hunk #8 FAILED at 399.
Hunk #9 succeeded at 524 with fuzz 2 (offset 7 lines).
2 out of 9 hunks FAILED -- saving rejects to file Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp.rej
patching file Source/WebKit/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
Hunk #1 FAILED at 62.
Hunk #2 FAILED at 307.
Hunk #3 succeeded at 331 (offset 7 lines).
Hunk #4 succeeded at 353 (offset 7 lines).
Hunk #5 succeeded at 381 (offset 7 lines).
Hunk #6 succeeded at 409 (offset 7 lines).
Hunk #7 succeeded at 417 (offset 7 lines).
Hunk #8 succeeded at 425 with fuzz 2 (offset 7 lines).
2 out of 8 hunks FAILED -- saving rejects to file Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp.rej
patching file Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Alex Christensen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/9244213

[Basuke Suzuki]

Created attachment 349899 [details]
PATCH

Thanks Alex for r+

[WebKit Commit Bot]

Comment on attachment 349899 [details]
PATCH

Clearing flags on attachment: 349899

Committed r236073: <https://trac.webkit.org/changeset/236073>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44527129>

[Fujii Hironori]

Following test cases are constantly failing since r236073.

http/tests/xmlhttprequest/basic-auth-default.html
http/tests/xmlhttprequest/cross-origin-authorization.html
http/tests/xmlhttprequest/logout.html
http/tests/xmlhttprequest/re-login-async.html
http/tests/xmlhttprequest/re-login.html
http/tests/xmlhttprequest/redirect-credentials-responseURL.html
http/tests/xmlhttprequest/remember-bad-password.html