Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Importing a netscape format cookie file fails if a cookie is not assigned a value #173

Open
dzrdmwjx opened this issue Mar 13, 2024 · 6 comments

Comments

@dzrdmwjx
Copy link

Fails with Error: "The input is not in a valid format"

It doesn't have a problem with blank cookies otherwise.
It's even able to export these cookies to netscape format, but can't import what it just exported if one of the cookies is blank.
If exporting to JSON format, export & import functions work as expected.

@dzrdmwjx
Copy link
Author

Cookie-Editor: v1.13.0

Tested on:

  • Chrome 122.0.6261.129 (Official Build) (64-bit) (cohort: Stable)
  • Microsoft Edge: 122.0.2365.80 (Official build) (64-bit)

Both are affected by this issue.

To reproduce the issue:

  • Just pick any site and launch cookie-editor popup
  • Modify any cookie to set a blank value and save
  • Export cookies in 'netscape' format
  • Import cookies from the clipboard that were just exported
  • Import will fail with "The input is not in a valid format" error

Thanks for looking into this.

@volovikariel
Copy link
Contributor

volovikariel commented Mar 20, 2024

Error is thrown because an empty value is not supported, we expect all 7 fields to be present:

const elements = line.split('\t');
if (elements.length != 7) {
throw new Error('Invalid netscape format');
}

But there is no official spec that I can find online. If desired, an empty value can easily be supported.
Whether it is supported depends entirely on @Moustachauve

Side-note: A spec is mentioned in the code

let netscapeCookies = '# Netscape HTTP Cookie File';
netscapeCookies += '\n# http://curl.haxx.se/rfc/cookie_spec.html';
netscapeCookies += '\n# This file was generated by Cookie-Editor';

But it doesn't seem to be followed (e.g: the date format isn't of type Wdy, DD-Mon-YYYY HH:MM:SS GMT) - so I'm not too sure why it's mentioned.

@Moustachauve
Copy link
Owner

Moustachauve commented Mar 21, 2024

For the Netscape format, I mostly based myself on the info available on curl's website: https://curl.se/docs/http-cookies.html and from info found in the discussion of #17.

an empty value can easily be supported by removing this check.

I didn't get a chance to try it out, but in theory an empty value should work if there is still the right amount of tabs, split should just leave it empty.

But it doesn't even seem to be followed (e.g: the date format isn't of type Wdy, DD-Mon-YYYY HH:MM:SS GMT)

There's so many places mentioning using a timestamp instead of a date format, it's honestly confusing. The specs officially says it's supposed to be a long date format, but for now I'll keep the timestamp since it works and that's what curl also mentions.

so I'm not too sure why it's mentioned.

It seems to be pretty standard to include this header when saving cookies to a netscape format, so I also included it. It's not strictly needed.

@dzrdmwjx
Copy link
Author

For the Netscape format, I mostly based myself on the info available on curl's website: https://curl.se/docs/http-cookies.html and from info found in the discussion of #17.

an empty value can easily be supported by removing this check.

I didn't get a chance to try it out, but in theory an empty value should work if there is still the right amount of tabs, split should just leave it empty.

It does export the cookies in the correct format with a trailing tab at the end of the line for blank cookies, not sure why does it have a problem importing them. You are right in that it should work - split() returns the correct number of tokens in this case. Perhaps the problem lies elsewhere?

But it doesn't even seem to be followed (e.g: the date format isn't of type Wdy, DD-Mon-YYYY HH:MM:SS GMT)

There's so many places mentioning using a timestamp instead of a date format, it's honestly confusing. The specs officially says it's supposed to be a long date format, but for now I'll keep the timestamp since it works and that's what curl also mentions.

Maybe that's a very old version of the "spec", as Netscape itself is quite ancient. There was a version that even preceded this with just 6 fields as described here: https://www.ibm.com/support/pages/how-identify-version-0-netscape-cookies
The timestamp for the expiration is pretty standard now, I haven't seen the date format in use anywhere. And so is the #HttpOnly_ prefix for http-only cookies, which is also missing from this spec but is correctly handled by Cookie-Editor.

so I'm not too sure why it's mentioned.

It seems to be pretty standard to include this header when saving cookies to a netscape format, so I also included it. It's not strictly needed.

I believe the first line was required as part of the spec and some software/libraries still require it, but the next two lines are just comments that curl added to its version of the cookies file. In fact, the current version of curl uses a different URL now - the one you referenced above (https://curl.se/docs/http-cookies.html), which in turn links to this old spec. Wget adds a similar header too, but it doesn't reference any doc or spec. I think it would be better to refer to https://curl.se/docs/http-cookies.html here instead, as it has more context and provides the details of the file format that's actually being used.

@dzrdmwjx
Copy link
Author

It does export the cookies in the correct format with a trailing tab at the end of the line for blank cookies, not sure why does it have a problem importing them. You are right in that it should work - split() returns the correct number of tokens in this case. Perhaps the problem lies elsewhere?

Okay, I figured where the problem is. It's the trim() being called for each line read, before the split() is called - that gets rid of the trailing '\t' leading to this error. Removing 'line = line.trim(); ' on line 17 and changing the subsequent check on line 18 to 'if (!line.trim().length)' should take care of this issue.

for (let line of lines) {
line = line.trim();
if (!line.length) {
continue;
}
const isHttpOnly = line.startsWith(httpOnlyPrefix);
if (isHttpOnly) {
line = line.substring(httpOnlyPrefix.length);
}
// Skip comments
if (line[0] == '#') {
continue;
}
const elements = line.split('\t');
if (elements.length != 7) {
throw new Error('Invalid netscape format');
}

@volovikariel
Copy link
Contributor

Another alternative would be to replace line.trim() with line.trimStart() instead, and leave line 18 as is.

That way we'd still allow for some flexibility when writing out the cookies, since we rely on the HttpPrefix and the comments starting at index 0.

Good on you for finding the issue 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants