The http/tests/misc/authentication-redirect-3/authentication-sent-to-redirect-same-origin-with-location-credentials.html layout test is failing after r161176. http://trac.webkit.org/changeset/161176 http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fmisc%2Fauthentication-redirect-3%2Fauthentication-sent-to-redirect-same-origin-with-location-credentials.html Here's the diff: --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/http/tests/misc/authentication-redirect-3/authentication-sent-to-redirect-same-origin-with-location-credentials-expected.txt +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/http/tests/misc/authentication-redirect-3/authentication-sent-to-redirect-same-origin-with-location-credentials-actual.txt @@ -9,19 +9,19 @@ -------- Frame: '<!--framePath //<!--frame0-->-->' -------- -Resource loaded with HTTP authentication username 'redirectuser' and password 'redirectpassword' +Resource loaded with HTTP authentication username 'testUser' and password 'testPassword' -------- Frame: '<!--framePath //<!--frame1-->-->' -------- -Resource loaded with HTTP authentication username 'redirectuser' and password 'redirectpassword' +Resource loaded with HTTP authentication username 'testUser' and password 'testPassword' -------- Frame: '<!--framePath //<!--frame2-->-->' -------- -Resource loaded with HTTP authentication username 'redirectuser' and password 'redirectpassword' +Resource loaded with HTTP authentication username 'testUser' and password 'testPassword' -------- Frame: '<!--framePath //<!--frame3-->-->' -------- -Resource loaded with HTTP authentication username 'redirectuser' and password 'redirectpassword' +Resource loaded with HTTP authentication username 'testUser' and password 'testPassword'
And http/tests/security/redirect-BLOCKED-to-localURL.html as well. Diff: --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/http/tests/security/redirect-BLOCKED-to-localURL-expected.txt +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/http/tests/security/redirect-BLOCKED-to-localURL-actual.txt @@ -1,2 +1,3 @@ +CONSOLE MESSAGE: Not allowed to load local resource: file-redirect-target.html This attempts to open a redirect link to a file URL, which should be blocked.
Carlos, are you able to fix this or should we roll out the change for now?
(In reply to comment #2) > Carlos, are you able to fix this or should we roll out the change for now? I'll take a look at it today.
hmm, both tests pass for me with wk1 and wk2. I indeed ran the tests before landing r161176. Is it flaky in the bots?
I think I know how to fix it in any case, but I'm unable to reproduce it, I'll fix it tomorrow, today I've been busy working on all other layout tests in the end (see bug #126570).
Created attachment 220610 [details] Temptative fix This patch restores the previous behaviour, clearing the credentials form the request before calling client()->willSendRequest(), but applying them right before creating the new SoupRequest. I don't know if it actually fixes the problem, because I haven't been able to reproduce it.
Comment on attachment 220610 [details] Temptative fix View in context: https://bugs.webkit.org/attachment.cgi?id=220610&action=review > Source/WebCore/ChangeLog:11 > + > + Clear the credentials before calling willSendRequest on the client > + to avoid sending the credentials to the API layer, but apply them > + again to the request right before creating the new SoupRequest. > + Hrm. Where are the credentials cleared? I only see you removing newRequest.removeCredentials(); in doRedirect.
(In reply to comment #7) > (From update of attachment 220610 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220610&action=review > > > Source/WebCore/ChangeLog:11 > > + > > + Clear the credentials before calling willSendRequest on the client > > + to avoid sending the credentials to the API layer, but apply them > > + again to the request right before creating the new SoupRequest. > > + > > Hrm. Where are the credentials cleared? I only see you removing newRequest.removeCredentials(); in doRedirect. http://trac.webkit.org/browser/trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp#L507
Committed r161549: <http://trac.webkit.org/changeset/161549>
(In reply to comment #1) > And http/tests/security/redirect-BLOCKED-to-localURL.html as well. > > Diff: > --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/http/tests/security/redirect-BLOCKED-to-localURL-expected.txt > +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/http/tests/security/redirect-BLOCKED-to-localURL-actual.txt > @@ -1,2 +1,3 @@ > +CONSOLE MESSAGE: Not allowed to load local resource: file-redirect-target.html > > This attempts to open a redirect link to a file URL, which should be blocked. This failure still remains. The console message is not expected both in the GTK-specific and Mac-specific baselines, but it is in the global baseline. Is it OK to remove the GTK-specific baseline? Is the console message a progression? http://trac.webkit.org/browser/trunk/LayoutTests/http/tests/security/redirect-BLOCKED-to-localURL-expected.txt http://trac.webkit.org/browser/trunk/LayoutTests/platform/gtk/http/tests/security/redirect-BLOCKED-to-localURL-expected.txt http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/http/tests/security/redirect-BLOCKED-to-localURL-expected.txt
(In reply to comment #10) > Is it OK to remove the GTK-specific baseline? Is the console message a progression? It seems like the test is passing as long as you don't see: "FAIL: This page shouldn't load via HTTP redirect to file:"
(In reply to comment #11) > (In reply to comment #10) > > > Is it OK to remove the GTK-specific baseline? Is the console message a progression? > > > It seems like the test is passing as long as you don't see: "FAIL: This page shouldn't load via HTTP redirect to file:" Right.
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > > > Is it OK to remove the GTK-specific baseline? Is the console message a progression? > > > > > > It seems like the test is passing as long as you don't see: "FAIL: This page shouldn't load via HTTP redirect to file:" > > Right. Thanks for the clarifications. Removed in r162068. http://trac.webkit.org/changeset/162068