Bug 188154 – [Curl] Bug fix for pre-supplied credential information handling.

• Status: RESOLVED
• Resolution: INVALID
• Component: Platform

[Basuke Suzuki]

Say user and password is supplied when ResourceHandle is created, such as XMLHTTPRequest or credentials in a url. If the credential was wrong, it keeps using that information to retry without asking client and it will crash by stack overflow.

[Basuke Suzuki]

Created attachment 346050 [details]
PATCH

[Fujii Hironori]

Comment on attachment 346050 [details]
PATCH

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

> Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:362
> +        d->m_pass = String();

Why does curl port clear m_user and m_pass twice while Mac port and CF port do once?
First in ResourceHandle::didReceiveAuthenticationChallenge and second in ResourceHandle::getCredential.
IIUC, m_user and m_pass are used to remember user/pass in URL during the first request without user/pass.
It is weird for me that m_user and m_pass are cleared in ResourceHandle::getCredential.

[EWS Watchlist]

Comment on attachment 346050 [details]
PATCH

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

New failing tests:
http/tests/security/canvas-remote-read-remote-video-hls.html

[EWS Watchlist]

Created attachment 346057 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Basuke Suzuki]

Comment on attachment 346050 [details]
PATCH

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

>> Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:362
>> +        d->m_pass = String();
> 
> Why does curl port clear m_user and m_pass twice while Mac port and CF port do once?
> First in ResourceHandle::didReceiveAuthenticationChallenge and second in ResourceHandle::getCredential.
> IIUC, m_user and m_pass are used to remember user/pass in URL during the first request without user/pass.
> It is weird for me that m_user and m_pass are cleared in ResourceHandle::getCredential.

Right. clearing them in getCredential() isn't required maybe. The crash will be avoided by moving the clearing of m_user and m_pass causes stack overflow because everything run in main thread in recursive(). This is not good ...

[Basuke Suzuki]

More over, even for the success test after this patch, I found a difference in the access log of HTTPD. The pattern is not same with mac port result. Something is wrong.

127.0.0.1 - -       [30/Jul/2018:15:50:03 -0700] "GET /xmlhttprequest/failed-auth.html HTTP/1.1" 200 1941 1002
127.0.0.1 - -       [30/Jul/2018:15:51:05 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login1 HTTP/1.1" 401 23 2006
127.0.0.1 - badname [30/Jul/2018:15:51:06 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login2 HTTP/1.1" 401 23 1003
127.0.0.1 - -       [30/Jul/2018:15:51:06 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login2 HTTP/1.1" 401 23 2011
127.0.0.1 - badname [30/Jul/2018:15:51:06 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login2 HTTP/1.0" 401 23 4011
127.0.0.1 - -       [30/Jul/2018:15:51:07 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login3 HTTP/1.1" 401 23 6013
127.0.0.1 - -       [30/Jul/2018:15:51:07 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login3 HTTP/1.1" 401 23 2013
127.0.0.1 - badname [30/Jul/2018:15:51:07 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login3 HTTP/1.0" 401 23 2006
127.0.0.1 - -       [30/Jul/2018:15:51:08 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login4 HTTP/1.1" 401 23 2006
127.0.0.1 - badname [30/Jul/2018:15:51:08 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login4 HTTP/1.0" 401 23 2006
127.0.0.1 - -       [30/Jul/2018:15:51:08 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login4 HTTP/1.1" 401 23 2006
127.0.0.1 - badname [30/Jul/2018:15:51:08 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login4 HTTP/1.0" 401 23 2006

[Basuke Suzuki]

Mac result:

127.0.0.1 - -       [30/Jul/2018:16:32:49 -0700] "GET /xmlhttprequest/failed-auth.html HTTP/1.1" 200 1941 4547
127.0.0.1 - -       [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login1 HTTP/1.1" 401 23 6037
127.0.0.1 - -       [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login1 HTTP/1.1" 401 23 375
127.0.0.1 - -       [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login2 HTTP/1.1" 401 23 332
127.0.0.1 - badname [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login2 HTTP/1.1" 401 23 350
127.0.0.1 - -       [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login2 HTTP/1.1" 401 23 306
127.0.0.1 - -       [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login3 HTTP/1.1" 401 23 1136
127.0.0.1 - -       [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login3 HTTP/1.1" 401 23 371
127.0.0.1 - -       [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login4 HTTP/1.1" 401 23 398
127.0.0.1 - badname [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login4 HTTP/1.1" 401 23 325
127.0.0.1 - -       [30/Jul/2018:16:32:50 -0700] "GET /xmlhttprequest/resources/basic-auth/basic-auth.php?uid=login4 HTTP/1.1" 401 23 332

[Basuke Suzuki]

In Mac port, success case (1, 3) access twice and failure cases access 3 times. Curl port doesn't follow this pattern. It is bad.

[Basuke Suzuki]

Crash itself was fixed here.
https://bugs.webkit.org/show_bug.cgi?id=188206

still there're other issues, but not related to this bug.