1410013 - Unescaped non-ASCII cookies are corrupted in sessionstore

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect, P2)

Description

[Jan Melcher]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171016185129

Steps to reproduce:

1. Configure Firefox to restore tabs on restart
2. Visit https://pal.jan-melcher.de/firefox/cookie-bug/
3. Open Page Info > Cookies and verify pwcount is set to ”+ 1
4. Restart Firefox
5. Select tab again *without* hitting Refresh and open cookie list again


Actual results:

pwcount cookie is + 1


Expected results:

pwcount is ”+ 1

---

The website consists of this simple php file:

<?php
header("Set-Cookie: pwcount=\xe2\x80\x9d\x2b\x20\x31");
echo 'Cookie set.';

This mimicks a (arguably not well-behaved) website that sets a cookie but does not escape special characters. This works in all parts of Firefox except the sessionstore handling. In the file previous.jsonlz4, the special character is not escaped either. Upon reading this file, the special character gets corrupted.

Comment 1

[Jan Melcher]

It looks like bugzilla did not like the "actual result" I typed in. Instead of ”, it appears as a unicode box "001D", so I guess it's "\u001D" aka Group Separator.

Comment 2

[:Gijs (he/him)]

Comments in bug 1558836 pinpoint the cause of this issue, fwiw.

Comment 3

[Masatoshi Kimura [:emk]]

Session states (including cookies) are encoded in UTF-8 on save, so it should also be treated as UTF-8 on restore.
https://searchfox.org/mozilla-central/rev/d62176d3d70bbf9ad86a5d444f9edada52cbc2b1/browser/components/sessionstore/SessionWorker.js#161
https://searchfox.org/mozilla-central/rev/d62176d3d70bbf9ad86a5d444f9edada52cbc2b1/browser/components/sessionstore/SessionWorker.js#18

Comment 4

[Masatoshi Kimura [:emk]]

nsICookie changed the value property type to AUTF8String. So nsICookieManager.add should also change for consistency.
https://searchfox.org/mozilla-central/diff/280752aa618928f8fd1aad71e6c1e31f64834ed2/netwerk/cookie/nsICookie.idl#29
(Oh, and this is my change in bug 826159. Sorry for overlooking this.)

Comment 5

[Masatoshi Kimura [:emk]]

Attachment: Bug 1410013 - Change `aValue` parameter of `nsICookieManager.add` to `AUTF8String` to align with `nsICookie` change. r?ehsan

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8ccc114a50f
Change `aValue` parameter of `nsICookieManager.add` to `AUTF8String` to align with `nsICookie` change. r=Ehsan

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/d8ccc114a50f

Comment 8

[Cristian Fogel [:cfogel]]

Confirmed issue with 58.0a1 (2017-10-16).
Fix verified with 69.0b7 on Windows 10, macOS 10.13, Ubuntu 18.04.