Bug 167050 – ASSERTION FAILED: referrer == URL(URL(), referrer).strippedForUseAsReferrer() in WebCore::SecurityPolicy::generateReferrerHeader on https://trac.webkit.org/browser

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Michael Catanzaro]

This assertion is reproducible on https://trac.webkit.org/browser:

(1) Click on one of the four expander arrows, e.g. the one immediately to the left of "trunk" in order to expand the directory layout. Note that, while the contents of the page change, the page URL remains https://trac.webkit.org/browser.
(2) Click on any link, e.g. the WebKit logo in the top left of the page. Wait for the page to load.
(3) Click Back. Note the page URL is now different, it has gained a fragment: https://trac.webkit.org/browser#trunk
(4) Click on any link, e.g. the WebKit logo in the top-left again.

Now in WebCore::SecurityPolicy::generateReferrerHeader, we have:

 * referrer is "https://trac.webkit.org/browser#trunk"
 * URL(URL(), referrer).strippedForUseAsReferrer() is "https://trac.webkit.org/browser"

URL::strippedForUseAsReferrer intentionally removes the fragment, as required by RFC 2616. So referrer is somehow incorrect here. It triggers this assertion:

ASSERTION FAILED: referrer == URL(URL(), referrer).strippedForUseAsReferrer()
../../Source/WebCore/page/SecurityPolicy.cpp(73) : static WTF::String WebCore::SecurityPolicy::generateReferrerHeader(WebCore::ReferrerPolicy, const WebCore::URL&, const WTF::String&)
1   0x7f0f496b92d2 /home/mcatanzaro/Projects/GNOME/install/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7f0f496b92d2]
2   0x7f0f52b0c877 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore14SecurityPolicy22generateReferrerHeaderENS_14ReferrerPolicyERKNS_3URLERKN3WTF6StringE+0x1dd) [0x7f0f52b0c877]
3   0x7f0f5294ad37 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore11FrameLoader16loadFrameRequestERKNS_16FrameLoadRequestEPNS_5EventEPNS_9FormStateE+0x265) [0x7f0f5294ad37]
4   0x7f0f52947553 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore11FrameLoader11urlSelectedERKNS_16FrameLoadRequestEPNS_5EventE+0x23b) [0x7f0f52947553]
5   0x7f0f529472c1 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore11FrameLoader11urlSelectedERKNS_3URLERKN3WTF6StringEPNS_5EventENS_11LockHistoryENS_19LockBackForwardListENS_18ShouldSendReferrerENS_28ShouldOpenExternalURLsPolicyESt8optionalINS_20NewFrameOpenerPolicyEERKNS4_12AtomicStringE+0x10d) [0x7f0f529472c1]
6   0x7f0f52698211 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore17HTMLAnchorElement11handleClickERNS_5EventE+0x387) [0x7f0f52698211]
7   0x7f0f52696f8c /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore17HTMLAnchorElement19defaultEventHandlerERNS_5EventE+0xe8) [0x7f0f52696f8c]
8   0x7f0f52492894 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x5e28894) [0x7f0f52492894]
9   0x7f0f52492f78 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15EventDispatcher13dispatchEventERNS_4NodeERNS_5EventE+0x3cf) [0x7f0f52492f78]
10  0x7f0f524e0bad /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore4Node13dispatchEventERNS_5EventE+0x53) [0x7f0f524e0bad]
11  0x7f0f5247311a /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore7Element18dispatchMouseEventERKNS_18PlatformMouseEventERKN3WTF12AtomicStringEiPS0_+0x19c) [0x7f0f5247311a]
12  0x7f0f52a877b5 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore12EventHandler18dispatchMouseEventERKN3WTF12AtomicStringEPNS_4NodeEbiRKNS_18PlatformMouseEventEb+0xfd) [0x7f0f52a877b5]
13  0x7f0f52a85409 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore12EventHandler23handleMouseReleaseEventERKNS_18PlatformMouseEventE+0x60d) [0x7f0f52a85409]
14  0x7f0f530f85dc /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15UserInputBridge23handleMouseReleaseEventERKNS_18PlatformMouseEventENS_11InputSourceE+0x3c) [0x7f0f530f85dc]
15  0x7f0f519ed152 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x5383152) [0x7f0f519ed152]
16  0x7f0f519ed436 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit7WebPage10mouseEventERKNS_13WebMouseEventE+0x1f4) [0x7f0f519ed436]
17  0x7f0f51d79df1 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC22callMemberFunctionImplIN6WebKit7WebPageEMS2_FvRKNS1_13WebMouseEventEESt5tupleIJS3_EEJLm0EEEEvPT_T0_OT1_St16integer_sequenceImJXspT2_EEE+0x80) [0x7f0f51d79df1]
18  0x7f0f51d7838a /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC18callMemberFunctionIN6WebKit7WebPageEMS2_FvRKNS1_13WebMouseEventEESt5tupleIJS3_EESt16integer_sequenceImJLm0EEEEEvOT1_PT_T0_+0x41) [0x7f0f51d7838a]
19  0x7f0f51d716ee /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC13handleMessageIN8Messages7WebPage10MouseEventEN6WebKit7WebPageEMS5_FvRKNS4_13WebMouseEventEEEEvRNS_7DecoderEPT0_T1_+0x9b) [0x7f0f51d716ee]
20  0x7f0f51d6c68d /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit7WebPage24didReceiveWebPageMessageERN3IPC10ConnectionERNS1_7DecoderE+0x597) [0x7f0f51d6c68d]
21  0x7f0f519f37ca /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit7WebPage17didReceiveMessageERN3IPC10ConnectionERNS1_7DecoderE+0x240) [0x7f0f519f37ca]
22  0x7f0f515e55b5 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC18MessageReceiverMap15dispatchMessageERNS_10ConnectionERNS_7DecoderE+0x125) [0x7f0f515e55b5]
23  0x7f0f51872f8a /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit10WebProcess17didReceiveMessageERN3IPC10ConnectionERNS1_7DecoderE+0x4c) [0x7f0f51872f8a]
24  0x7f0f515cae6a /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC10Connection15dispatchMessageERNS_7DecoderE+0x3a) [0x7f0f515cae6a]
25  0x7f0f515cafd4 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC10Connection15dispatchMessageESt10unique_ptrINS_7DecoderESt14default_deleteIS2_EE+0x166) [0x7f0f515cafd4]
26  0x7f0f515cb1c6 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC10Connection18dispatchOneMessageEv+0xc8) [0x7f0f515cb1c6]
27  0x7f0f515cad0a /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x4f60d0a) [0x7f0f515cad0a]
28  0x7f0f515d10aa /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x4f670aa) [0x7f0f515d10aa]
29  0x7f0f5159456d /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZNK3WTF8FunctionIFvvEEclEv+0x37) [0x7f0f5159456d]
30  0x7f0f496d8a68 /home/mcatanzaro/Projects/GNOME/install/lib/libjavascriptcoregtk-4.0.so.18(_ZN3WTF7RunLoop11performWorkEv+0xce) [0x7f0f496d8a68]
31  0x7f0f4971f8e8 /home/mcatanzaro/Projects/GNOME/install/lib/libjavascriptcoregtk-4.0.so.18(+0x25008e8) [0x7f0f4971f8e8]

[Alexey Proskuryakov]

Reproduces on Mac.

rdar://problem/27972404

[Alex Christensen]

Looks like we need to strip the fragment somewhere

[John Wilander]

My layout test only triggers the bug through manual clicks, not when using the eventSender.

I'll attach my test case here separately, then upload a patch without a test.

[John Wilander]

Created attachment 300342 [details]
Layout test that only triggers bug through manual clicks, not eventSender

[John Wilander]

Created attachment 300344 [details]
Patch

[Michael Catanzaro]

You could add it to the ManualTests directory, although this is not great. Strange that eventSender doesn't work....

[Alexey Proskuryakov]

Comment on attachment 300344 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        but it doesn't trigger the bug. If you run it with manual clicks it
> +        does trigger the bug but not when run with eventSender.

This seems quite surprising! Perhaps there is a timing difference.

[John Wilander]

(In reply to comment #7)
> Comment on attachment 300344 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=300344&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        but it doesn't trigger the bug. If you run it with manual clicks it
> > +        does trigger the bug but not when run with eventSender.
> 
> This seems quite surprising! Perhaps there is a timing difference.

IIRC I experimented quite heavily with timing. But I might be missing something.

As seen in the -expected.txt file all the steps are carried out which means the clicks are happening:

PASS window.frames[0].document.querySelector('h1').innerText is "Page 1"
PASS window.frames[0].document.location.hash is ""
PASS window.frames[0].document.referrer is "http://127.0.0.1:8000/loading/open-cached-frame-should-strip-outgoing-referrer-url-for-use-as-referrer.html"
PASS window.frames[0].document.querySelector('h1').innerText is "Page 1"
PASS window.frames[0].document.location.hash is "#theFragment"
PASS window.frames[0].document.referrer is "http://127.0.0.1:8000/loading/open-cached-frame-should-strip-outgoing-referrer-url-for-use-as-referrer.html"
PASS window.frames[0].document.querySelector('h1').innerText is "Nop Page"
PASS window.frames[0].document.location.hash is ""
PASS window.frames[0].document.referrer is "http://127.0.0.1:8000/loading/open-cached-frame-should-strip-outgoing-referrer-url-for-use-as-referrer.html"
PASS window.frames[0].document.querySelector('h1').innerText is "Page 1"
PASS window.frames[0].document.location.hash is "#theFragment"
PASS window.frames[0].document.referrer is "http://127.0.0.1:8000/loading/open-cached-frame-should-strip-outgoing-referrer-url-for-use-as-referrer.html"
PASS window.frames[0].document.querySelector('h1').innerText is "Nop Page"
PASS window.frames[0].document.location.hash is ""
PASS window.frames[0].document.referrer is "http://127.0.0.1:8000/loading/open-cached-frame-should-strip-outgoing-referrer-url-for-use-as-referrer.html"

[Daniel Bates]

Created attachment 300756 [details]
Layout Test

Attached is a layout tests that crashes with the reported assertion failure in DumpRenderTree and WebKitTestRunner. We should look to simplify this test case and assert something about the state of document.referrer.

[Fujii Hironori]

Created attachment 377114 [details]
Patch

[Fujii Hironori]

1. Go to https://trac.webkit.org/wiki#AboutWebKit
2. Click "WebKit Team"
3. History back
4. Click "WebKit Team"
5. The assertion fails

[youenn fablet]

Comment on attachment 377114 [details]
Patch

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

> LayoutTests/http/tests/history/fragment-change-then-back.html:26
> +        }, 2000)

This test will take 2 seconds to load, can we try to simplify it?
Or maybe John's initial test is now passing?

[Alexey Proskuryakov]

Comment on attachment 377114 [details]
Patch

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

> LayoutTests/http/tests/history/fragment-change-then-back.html:10
> +    testRunner.dumpAsText();
> +    testRunner.waitUntilDone();

Please use "jsTestIsAsync = true;" instead of these.

> LayoutTests/http/tests/history/fragment-change-then-back.html:25
> +            if (window.testRunner)
> +                testRunner.notifyDone();

And finishJSTest() instead of these.

> LayoutTests/http/tests/history/fragment-change-then-back.html:30
> +<script src="../resources/js-test-post.js"></script>

The post script should be after body. Better yet, just use js-test.js in the prefix, then no postfix is needed.

> LayoutTests/http/tests/history/fragment-change-then-back.html:33
> +<p>This test PASSED if it does not cause an assertion failure.</p>

If you make this a description() call, the order of messages in the output will be more logical.

[Fujii Hironori]

Created attachment 377227 [details]
Patch

[Fujii Hironori]

Comment on attachment 377114 [details]
Patch

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

>> LayoutTests/http/tests/history/fragment-change-then-back.html:26
>> +        }, 2000)
> 
> This test will take 2 seconds to load, can we try to simplify it?
> Or maybe John's initial test is now passing?

Removed the 2 seconds by using pageshow event.

>> LayoutTests/http/tests/history/fragment-change-then-back.html:30
>> +<script src="../resources/js-test-post.js"></script>
> 
> The post script should be after body. Better yet, just use js-test.js in the prefix, then no postfix is needed.

It's in LayoutTests/resources/js-test.js. Should I copy it into http/tests/resources/js-test.js ?

[Fujii Hironori]

Created attachment 377228 [details]
Patch

[youenn fablet]

Comment on attachment 377228 [details]
Patch

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

> LayoutTests/http/tests/navigation/page-cache-fragment-referrer.html:22
> +    shouldBeEqualToString('xhr.responseText', 'http://127.0.0.1:8000/navigation/page-cache-fragment-referrer.html');

Can we compute the expected value with document.location.href.
Something like shouldBeEqualToString('xhr.responseText' + window.location.hash, window.location.href);

Also, I would use either async XHR or fetch.
jsTestIsAsync = true;
window.addEventListener('pageshow', async () => {
     const response = fetch('resources/referrer.php');
     shouldBeEqualToString('xhr.responseText' + window.location.hash, window.location.href);
     finishJSTest();
});

It was not initially clear to me that we were doing the XHR before going to success.html and after doing a back navigation.

We could register page show after back navigation, using some state kept through local storage.
Or we could queue the load/back navigation after the first xhr/fetch test, do the second xhr/fetch test and call finishJSTest (again using local storage).

[Alexey Proskuryakov]

> It's in LayoutTests/resources/js-test.js. Should I copy it into http/tests/resources/js-test.js ?

I think that using js-test-post is OK for now, but we should look into reducing the number of harness variations (I see that there is also some mystery js-test-post-async.js in http).

[Fujii Hironori]

Comment on attachment 377228 [details]
Patch

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

>> LayoutTests/http/tests/navigation/page-cache-fragment-referrer.html:22
>> +    shouldBeEqualToString('xhr.responseText', 'http://127.0.0.1:8000/navigation/page-cache-fragment-referrer.html');
> 
> Can we compute the expected value with document.location.href.
> Something like shouldBeEqualToString('xhr.responseText' + window.location.hash, window.location.href);
> 
> Also, I would use either async XHR or fetch.
> jsTestIsAsync = true;
> window.addEventListener('pageshow', async () => {
>      const response = fetch('resources/referrer.php');
>      shouldBeEqualToString('xhr.responseText' + window.location.hash, window.location.href);
>      finishJSTest();
> });
> 
> It was not initially clear to me that we were doing the XHR before going to success.html and after doing a back navigation.
> 
> We could register page show after back navigation, using some state kept through local storage.
> Or we could queue the load/back navigation after the first xhr/fetch test, do the second xhr/fetch test and call finishJSTest (again using local storage).

Good idea. Will fix.
I don't need local storage because I'm able to use a simple variable.

[Fujii Hironori]

Oh, no. finishJSTest() doesn't work as expected in test cases using testRunner.queueBackNavigation.
It seems that I need to use sync XHR.

[Fujii Hironori]

Created attachment 377320 [details]
Patch for landing

[Fujii Hironori]

Comment on attachment 377320 [details]
Patch for landing

Clearing flags on attachment: 377320

Committed r249188: <https://trac.webkit.org/changeset/249188>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.