Bug 126518 – REGRESSION(r161176): http/tests/misc/authentication-redirect-3/authentication-sent-to-redirect-same-origin-with-location-credentials.html is failing on GTK

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Zan Dobersek]

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'

[Zan Dobersek]

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.

[Martin Robinson]

Carlos, are you able to fix this or should we roll out the change for now?

[Carlos Garcia Campos]

(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.

[Carlos Garcia Campos]

hmm, both tests pass for me with wk1 and wk2. I indeed ran the tests before landing r161176. Is it flaky in the bots?

[Carlos Garcia Campos]

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).

[Carlos Garcia Campos]

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.

[Martin Robinson]

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.

[Carlos Garcia Campos]

(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

[Carlos Garcia Campos]

Committed r161549: <http://trac.webkit.org/changeset/161549>

[Zan Dobersek]

(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

[Martin Robinson]

(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:"

[Carlos Garcia Campos]

(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.

[Zan Dobersek]

(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