[GTK][WPE][libsoup] Test imported/w3c/web-platform-tests/cookies/samesite/about-blank-toplevel.https.html crashes since it was imported

• Component: WPE WebKit
• Status: RESOLVED DUPLICATE

[Carlos Alberto Lopez Perez]

Created attachment 433172 [details]
crash log for debug build on GTK

The test imported/w3c/web-platform-tests/cookies/samesite/about-blank-toplevel.https.html imported in r279585 crashes with the attached backtrace, and the following is printed on stderr:


(process:777): libsoup-CRITICAL **: 15:24:32.986: soup_host_matches_host: assertion 'host != NULL' failed

[Carlos Alberto Lopez Perez]

Created attachment 433187 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 433187 [details]
Patch

ChangeLog of LayoutTests is  missing.

[Carlos Alberto Lopez Perez]

Committed r279778 (239545@main): <https://commits.webkit.org/239545@main>

[Arcady Goldmints-Orlov]

Created attachment 433221 [details]
Example of test actual vs expectation diff

FYI this caused a number of tests under http/tests/storageAccess/ to no longer match expectations, generally along the lines of the example, but the tests still pass.

[Carlos Alberto Lopez Perez]

(In reply to Arcady Goldmints-Orlov from comment #4)
> Created attachment 433221 [details]
> Example of test actual vs expectation diff
> 
> FYI this caused a number of tests under http/tests/storageAccess/ to no
> longer match expectations, generally along the lines of the example, but the
> tests still pass.

Thanks for noticing this Arcady.

I can confirm that after this patch those tests have different expectations.

Also i'm re-reading the doc of soup_cookie_jar_get_cookie_list_with_same_site_info() and according to it <https://libsoup.org/SoupCookieJar.html#soup-cookie-jar-get-cookie-list-with-same-site-info> null may be passed as value for the site_for_cookies argument (cookieURI).


So I will revert this, it seems it is not the right fix.

[Carlos Alberto Lopez Perez]

Reverted r279778 for reason:

It caused unexpected text diffs on http/tests/storageAccess tests

Committed r279825 (239586@main): <https://commits.webkit.org/239586@main>

[Carlos Alberto Lopez Perez]

This is the trace from the debug bot:

Thread 1 (Thread 0x7f6e62fd4ec0 (LWP 46893)):
#0  g_logv (log_domain=0x7f6e643df973 "libsoup", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=<optimized out>) at ../glib/gmessages.c:1413
#1  0x00007f6e656fb973 in g_log (log_domain=log_domain@entry=0x7f6e643df973 "libsoup", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7f6e65753ad0 "%s: assertion '%s' failed") at ../glib/gmessages.c:1451
#2  0x00007f6e656fc19d in g_return_if_fail_warning (log_domain=log_domain@entry=0x7f6e643df973 "libsoup", pretty_function=pretty_function@entry=0x7f6e643ea9f0 <__func__.3> "soup_host_matches_host", expression=expression@entry=0x7f6e643e1893 "host != NULL") at ../glib/gmessages.c:2883
#3  0x00007f6e643d2992 in soup_host_matches_host (host=<optimized out>, compare_with=compare_with@entry=0x55c8e1ef7340 "localhost") at ../libsoup/soup-misc.c:189
#4  0x00007f6e643a5e92 in cookie_is_valid_for_same_site_policy (for_http=0, is_top_level_navigation=0, cookie_uri=0x0, top_level=0x55c8e1e8e7d0, uri=0x55c8e253ab40, is_safe_method=1, cookie=0x7f6e18004b30) at ../libsoup/cookies/soup-cookie-jar.c:306
#5  get_cookies (jar=jar@entry=0x55c8e1ef8520 [SoupCookieJar], uri=uri@entry=0x55c8e253ab40, top_level=top_level@entry=0x55c8e1e8e7d0, site_for_cookies=site_for_cookies@entry=0x0, is_safe_method=is_safe_method@entry=1, for_http=for_http@entry=0, is_top_level_navigation=0, copy_cookies=1) at ../libsoup/cookies/soup-cookie-jar.c:358
#6  0x00007f6e643a654e in soup_cookie_jar_get_cookie_list_with_same_site_info (jar=0x55c8e1ef8520 [SoupCookieJar], uri=0x55c8e253ab40, top_level=0x55c8e1e8e7d0, site_for_cookies=0x0, for_http=0, is_safe_method=1, is_top_level_navigation=0) at ../libsoup/cookies/soup-cookie-jar.c:493
#7  0x00007f6e713e6a53 in WebCore::cookiesForSession(WebCore::NetworkStorageSession const&, WTF::URL const&, WTF::URL const&, WebCore::SameSiteInfo const&, std::optional<WTF::ObjectIdentifier<WebCore::FrameIdentifierType> >, std::optional<WTF::ObjectIdentifier<WebCore::PageIdentifierType> >, bool, WebCore::IncludeSecureCookies, WebCore::ShouldAskITP, WebCore::ShouldRelaxThirdPartyCookieBlocking) (session=..., firstParty=..., url=..., sameSiteInfo=..., frameID=std::optional<class WTF::ObjectIdentifier<WebCore::FrameIdentifierType>> = {...}, pageID=std::optional<class WTF::ObjectIdentifier<WebCore::PageIdentifierType>> = {...}, forHTTPHeader=false, includeSecureCookies=WebCore::IncludeSecureCookies::Yes, shouldAskITP=WebCore::ShouldAskITP::No, relaxThirdPartyCookieBlocking=WebCore::ShouldRelaxThirdPartyCookieBlocking::No) at ../../Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp:595


It seems that on #3 it ends calling soup_host_matches_host() with host=NULL (and that causes the issue)
But according to that trace it reach that point on #4, where it does:

  <https://gitlab.gnome.org/GNOME/libsoup/-/blob/2.99.9/libsoup/cookies/soup-cookie-jar.c#L306>
  return soup_host_matches_host (g_uri_get_host (cookie_uri ? cookie_uri : top_level), g_uri_get_host (uri));

cookie_uri is NULL, but it is checked so it should end calling soup_host_matches_host() with host=top_level, which is not NULL.

I'm puzzled :?
Any ideas of why this happens?

[Michael Catanzaro]

(In reply to Carlos Alberto Lopez Perez from comment #7)
> I'm puzzled :?
> Any ideas of why this happens?

I suspect it would likely become clear if we had a printf showing the values of cookie_uri and top_level.

Note the return value of g_uri_get_host() is nullable, so libsoup might be wrong to assume it can never return NULL there.

[Carlos Alberto Lopez Perez]

(In reply to Michael Catanzaro from comment #8)
> (In reply to Carlos Alberto Lopez Perez from comment #7)
> > I'm puzzled :?
> > Any ideas of why this happens?
> 
> I suspect it would likely become clear if we had a printf showing the values
> of cookie_uri and top_level.
> 

According to the trace above:

cookie_uri=0x0
top_level=0x55c8e1e8e7d0

[Michael Catanzaro]

My guess is (a) top_level just doesn't contain a host component, e.g. it might be a file:// URI, although those should never set cookies and so shouldn't reach this point, or else (b) something wrong with the GUri parser that causes it to think the URI has no host component.

[Carlos Alberto Lopez Perez]

(In reply to Michael Catanzaro from comment #10)
> My guess is (a) top_level just doesn't contain a host component, e.g. it
> might be a file:// URI, although those should never set cookies and so
> shouldn't reach this point

This is what is happening.

I put some printf's and the value of top_level is "about:blank" so it doesn't have a host component and therefore g_uri_get_host() returns null.

Seems this is a bug on libsoup? It should nullcheck what g_uri_get_host() returns, shouldn't it?

[Michael Catanzaro]

Yes, I agree this is a libsoup bug.

This is a great test. I would never have thought to test a corner-case like this. Page spawns an about:blank window, then gives it a frame that tries to set cookies. Huh.

[Arcady Goldmints-Orlov]

This appears to have been fixed by r291741. Closing as duplicate.

*** This bug has been marked as a duplicate of bug 238202 ***