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

[cookies] Support Firefox containers JSON version 5 #9655

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Zegnat
Copy link

@Zegnat Zegnat commented Apr 9, 2024

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

Firefox has a new version of the containers.json that needs to be read to extract the correct cookies. E.g. the follow command currently fails even though the default container user-context-personal exists:

yt-dlp --cookies-from-browser firefox:aaaa9aaa.default::user-context-personal "https://www.youtube.com/playlist?list=UUMOLe_q9axMaeTbjN0hy1Z9xA"
# Extracting cookies from firefox
# ERROR: could not find firefox container "user-context-personal" in containers.json

This is because version 5 of the contains.json format has the member l10nId (note the small d) with the format user-context-${name}, where version 4 had l10nID with userContext${name}.label. Compare the version 4 from an old issue comment on this repo with the following version 5:

{
  "version": 5,
  "lastUserContextId": 31566,
  "identities": [
    {
      "userContextId": 1,
      "public": true,
      "icon": "fingerprint",
      "color": "blue",
      "telemetryId": 1,
      "l10nId": "user-context-personal"
    },
    
  ]
}

This PR fixes this by still supporting version 4, but in addition supporting version 5 by testing one extra regular expression.

I was unable to find any documentation of the different versions of containers.json, I assume Firefox sees this as an internal detail only and not part of any public API.

Template

Before submitting a pull request make sure you have:

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check all of the following options that apply:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

@bashonly bashonly added the bug Bug that is not site-specific label Apr 9, 2024
@pukkandan
Copy link
Member

Why not combine the regex?

@bashonly
Copy link
Member

the key is different too

@Zegnat
Copy link
Author

Zegnat commented Apr 10, 2024

Yeah, they decided they wanted different capitalisation in the member name. It is why I wrote to note the small d 😉

Is there a code style around comments in code for this sort of stuff? It might be clearer to put a comment above each regex for posterity to mention how it parses version 4 and version 5 respectively?

@pukkandan
Copy link
Member

Gotcha. I was reading on phone and didn't notice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug that is not site-specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants