1659578 - Ctrl-S from PDF Viewer/JSON viewer does not serve from cache, and it does not include SameSite=Strict cookies either (when privacy.partition.network_state=true)

Status: RESOLVED DUPLICATE of bug 1641270

(Core :: Privacy: Anti-Tracking, defect, P2)

Description

[Rob Wu [:robwu]]

Attachment: server.js

Bug 1639154 initially caused a regression that prevented PDFs with SameSite-cookies from being saved.
Bug 1640405 attempted to fix this, and the patch claims to have a fix for SameSite=Lax/Strict. Unfortunately, unit tests were not included, and it turns out that this only fixed the SameSite=Lax case. SameSite=Strict is still affected. And I'm not sure whether the fix is the correct; I would expect the request to be served from the cache, not via a fresh HTTP request.

STR:

1. Download and start the attached Node.js server.
2. Visit http://localhost:16567/setcookies
3. Click on the link to /testcook.pdf (this PDF is served with Cache-Control: no-store).
4. Press Ctrl-S.

Expected:

• The download is 505 bytes, consisting of the PDF.
• The server should show only one request (the initial request). The Ctrl-S request should have been served from the cache.
• Even if not served from the cache (which could be considered a separate bug), then the second request should show include the SameSite=Strict cookie.

Actual:

• The download has 15 bytes ("Missing cookies")
• The server's stdout log shows checkcookie= laxcook=lax

GOOD: Expected server log, before bug 1639154 (Firefox 77-)

/setcookies
checkcookie= laxcook=lax; strictcook=strict

BAD: Actual server log, after bug 1639154 but before bug 1640405 (Firefox 78 - 79):

/setcookies
checkcookie= laxcook=lax; strictcook=strict
checkcookie= undefined

BAD: Actual server log, after bug 1640405 until now (Firefox 79+):

/setcookies
checkcookie= laxcook=lax; strictcook=strict
checkcookie= laxcook=lax

So there are two issues to be fixed:

• Ctrl-S does not serve PDFs from the cache.
• Ctrl-S does not include sameSite=Strict cookies.

Comment 1

[Tim Huang[:timhuang]]

I cannot reproduce this issue anymore. It seems this has been fixed somewhere. I ran a mozregression and figured out that this push fixes this issue. Unfortunately, there are too many bugs in this push, so I cannot tell which bug fixes this issue.

Rob, would you be able to verify if this is fixed? Thanks.

Comment 2

[Rob Wu [:robwu]]

From the regression range, bug 1659753 looks like the most likely cause of the behavioral change.
It's not a fix for the fundamental problem, but a side effect of the implementation change (i.e. saving the already-download data as stored in a blob:-URL instead of passing a http(s):-URL).

The PDF Viewer is implemented using a stream converted. I know that the JSON Viewer is also implemented using a stream converted, and managed to reproduce the original bug on release. Same STR as original report, except json instead of pdf. I'll attach the server for testing.

Comment 3

[Rob Wu [:robwu]]

Attachment: server-json.js

STR: Same as comment 0, except with "pdf" replaced by "json".

Comment 4

[Tim Huang[:timhuang]]

This issue is that the saving channel doesn't have the correct cookieJarSettings. So, it won't use the correct partitionKey when downloading the resource. The patches of Bug 1641270 can fix this issue.

Comment 5

[Rob Wu [:robwu]]

I'll mark bug 1641270 as a dependency then, and assign this bug to you to keep track of it. If the other bug is fixed, then this one can probably be closed too (maybe even as a duplicate? maybe after QA verification?).

Comment 6

[Tim Huang[:timhuang]]

I can confirm that this issue has been fixed by Bug 1641270.