2013783 - Website keeps reloading itself

Status: NEW

(Core :: Networking: Cookies, defect, P2)

Description

[Sean Feng [:sefeng211]]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:149.0) Gecko/20100101 Firefox/149.0

Steps to reproduce:

For me, I can reproduce it in MacOS, but I can't repro it on Linux

Navigate to https://pslinks.fiu.edu/psc/jobs/CUSTOMER/HRMS/c/HRS_HRAM_FL.HRS_CG_SEARCH_FL.GBL?Page=HRS_APP_JBPST&amp&Action=U&JobOpeningId=536704&postingseq=1&SiteId=1000&REL_ACTION=Yes&src=LinkedIn&

Actual results:

The page kept reloading itself

Expected results:

The page should've loaded correctly

Comment 1

[Sean Feng [:sefeng211]]

Here's a profile: https://share.firefox.dev/4tuoBiD

UA: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:149.0) Gecko/20100101 Firefox/149.0

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Hmm, I get an error "You must have cookies enabled in order to sign in to your PeopleSoft application." with that page.
Does one need to log in to that site?

Does it help if you clear all the cookies for that site? (If so, this would be a Necko bug)

Comment 3

[Sean Feng [:sefeng211]]

Try clicking that Signin to PeopleSoft button and then open the link in a new tab Olli. That site indeed behaves a bit weird..

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Oh, now it does something else. It keeps running DoBack()

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

It does have something to do with cookies. Once it reproduced, I cleared cookies for the site and then it worked again.

Comment 6

[Valentin Gosu [:valentin]]

The problem seems to be HRPRD-PORTAL-PSJSESSIONID cookie.
If I delete it then load the link in comment 0, no loop happens.

Note that the STR are:

1. open the link in comment 0.
2. click Sign in to PeopleSoft
3. click the link in comment 0 again -> loop.

Comment 7

[Valentin Gosu [:valentin]]

The reload is coming from
https://pslinks.fiu.edu/cs/jobs/cache/PT_SAVEWARNINGSCRIPT_MIN_1.js

function UpdateDeviceCookie()
{
 var dfCookieVal = getCookie("PS_DEVICEFEATURES");  if (dfCookieVal.trim() == "new:1") { 
 var df = new ptDeviceFeatures(); df.init(); if (isFModeLayout()) 
 top.window.document.location.reload(); }
}

From what I can tell, loading the page returns this header:

nsHttpTransaction::ParseLine [Set-Cookie: PS_DEVICEFEATURES=new:1; domain=.fiu.edu; path=/; Samesite =Strict; secure] 

Then the page runs UpdateDeviceCookie, which sets the cookie to:

PS_DEVICEFEATURES:"width:1920 height:1080 pixelratio:2 touch:0 geolocation:1 websockets:1 webworkers:1 datepicker:1 dtpicker:1 timepicker:1 dnd:1 sessionstorage:1 localstorage:1 history:1 canvas:1 svg:1 postmessage:1 hc:0 maf:0"

and reloads the page (probably expecting the cookie will be sent on the next reload of the page).

It's possible the reload is cancelling the page, and the cookie message doesn't reach the parent process before the reload actually happens, so the new cookie doesn't get sent.

Comment 8

[Valentin Gosu [:valentin]]

This seems to be indeed a race happening.
The IPC message from CookieServiceChild::AddCookieFromDocument does go to the parent process, but is processed AFTER the http channel for the reload is created.
That means that http channel does not contain the cookies from the content process update, and the server will return a new Set-Cookie header which overwrites it and causes the reload loop.
That seems like something we definitely want to fix.

Comment 9

[Valentin Gosu [:valentin]]

https://hg-edge.mozilla.org/mozilla-central/pushloghtml?fromchange=9226f5b0d05bea99caf4f2f2119b47c0dadf6c69&tochange=272e82616218411bfb0b89ef7c710cd65830ece1

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1630865

Comment 11

[Valentin Gosu [:valentin]]

Actually, I've rechecked, and we don't actually race. The IPC message is processed before the channel is opened 😅
But the issue is caused by the following block:

https://searchfox.org/firefox-main/rev/cced10961b53e0d29e22e635404fec37728b2644/netwerk/cookie/CookieService.cpp#1043-1059

if (aHttpBound && aIsSameSiteForeign) {
  bool blockCookie = !ProcessSameSiteCookieForForeignRequest(
      aChannel, cookie, aIsSafeTopLevelNav, aHadCrossSiteRedirects,
      laxByDefault);

  if (blockCookie) {
    if (aHadCrossSiteRedirects) {
      CookieLogging::LogMessageToConsole(
          crc, aHostURI, nsIScriptError::warningFlag,
          CONSOLE_REJECTION_CATEGORY, "CookieBlockedCrossSiteRedirect"_ns,
          AutoTArray<nsString, 1>{
              NS_ConvertUTF8toUTF16(cookie->Name()),
          });
    }
    continue;
  }
}

That's why the STR in comment 6 work.
(In reply to Valentin Gosu [:valentin] (he/him) from comment #6)

3. click the link in comment 0 again -> loop.

If instead of loading the page via link from bugzilla, I just open a new tab and paste in the link, the page opens with no problem.
I'm not sure how it regressed in bug 1630865, but I'm looking into it.

Comment 12

[Valentin Gosu [:valentin]]

Actually, most likely caused by bug 1627653 - that involves same site changes.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1627653

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1627653

Comment 15

[Valentin Gosu [:valentin]]

Attachment: Bug 2013783 - test

Comment 16

[Valentin Gosu [:valentin]]

The test reproduces this scenario:

1. Cross-site navigation: A page on not-web-platform.test navigates to web-platform.test (which has a SameSite=Strict cookie)
2. Initial load: SameSite=Strict cookie is correctly NOT sent (cross-site navigation)
3. Reload: The page calls location.reload()
4. Bug: SameSite=Strict cookie is still NOT sent on the reload, but it should be sent because a reload is a same-site navigation (the document is reloading itself)

The underlying issue in IsSameSiteForeign():

• For the reload, triggeringPrincipal is still not-web-platform.test (from the session history entry)
• triggeringPrincipal->IsThirdPartyChannel() returns isForeign=true
• So the reload is incorrectly treated as a cross-site request