-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Enable use of session cookies in chrome #9747
Enable use of session cookies in chrome #9747
Conversation
Co-authored-by: Simon Sawicki <accounts@grub4k.xyz>
Browsers tested on macOS:
Will test one all those now 😄 Test command is this: I check that now the session cookie is sent in the request and it prints the JSON metadata |
Co-authored-by: pukkandan <pukkandan.ytdlp@gmail.com>
does this still work if the "Continue where you left off" option is not enabled? |
Continue where you left off:
So in summary, the browsers all behave the same that session cookies are not cleared on close. Their defaults for that setting vary. When started with "Open the new tab page" they all cleared the session cookies on startup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the patch no longer extracts a new column from the cookies DB, this is not a potentially breaking change and rigorous testing is no longer needed. The testing you've already done looks good, thanks!
I'm thinking that instead of closing the linked issue with this PR, we should repurpose it to track session cookie extraction for Firefox, since the issue has some useful information pertaining to that
@StefanLobbenmeier In light of the discussion about firefox on the other issue, do you want us to merge this first? Or would you prefer to add firefox impl into this PR? |
Let’s to Firefox in a separate PR |
IMPORTANT: PRs without the template will be CLOSED
Description of your pull request and other information
Partially Fixes #5534
The chrome (and brave, have not checked other chrome based browsers) cookies database has 2 columns related to cookie expiry: expires_utc and has_expires. yt-dlp only checks the expires_utc column:
The current logic in yt-dlp takes expires_utc only and the phyton cookie will consider a value of 0 as expired:
To support session_cookies we should therefore set expires to None, since they do not expire.
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:
What is the purpose of your pull request?