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

Fix incorrect handling of cookiejars with '0' or empty 'expires' #12929

Closed
wants to merge 1 commit into from

Conversation

elipsitz
Copy link
Contributor

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • 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?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

This fixes cookies not being sent by youtube-dl if the entry in the cookiejar has an 'expires' column set to either 0 or '' (empty). Both of these indicate that the cookie is a session cookie (expires only when the browser is closed). At least YouTube uses this type of cookie to store/send important session information. Previously, you would have to manually set the 'expires' column to some date in the distant future for all of these cookies in your cookiejar if you wanted youtube-dl to use them.

This is related to a Python bug https://bugs.python.org/issue17164 in MozillaCookieJar.
Despite the ignore_expires=True line, truly expired cookies (0 < expires_at <= current_time) will not actually be sent -- urllib2/cookielib do not send cookies that are expired anyway. This just prevents the cookiejar library from prematurely discarding 'expires_at=0' cookies.

Example of a cookie that would normally be sent:
.youtube.com TRUE / FALSE 1498715626 PREF f1=50000000&hl=en

Example of cookies that would not have been sent before (but should have been):
.youtube.com TRUE / FALSE 0 PREF f1=50000000&hl=en
.youtube.com TRUE / FALSE PREF f1=50000000&hl=en

@dstftw
Copy link
Collaborator

dstftw commented Apr 30, 2017

Technically session cookies should not even be stored in a cookie file since by definition it's bound to browser's session time. Needless to say it's very questionable whether they should be sent when loaded from cookie file.

@elipsitz
Copy link
Contributor Author

I'd argue that's the correct behavior though. A very common use case is to pass in a cookiejar to manually handle a complicated login. In that case, you're suspending and restarting the same browser session in youtube-dl by dumping the session's cookies in the cookiejar and loading them in youtube-dl -- a process that should be seamless.

Besides, the session cookies are still discarded when they're saved back to the cookiejar at the end of execution (one "browser session").

@elipsitz
Copy link
Contributor Author

@dstftw would it be possible to take a look at this again? I really feel as though there's a strong argument for this behavior being the 'correct' behavior.

@dstftw
Copy link
Collaborator

dstftw commented May 26, 2017

Which problem concretely this is supposed to fix? Namely what concretely does not work currently and does work after this change.

@elipsitz
Copy link
Contributor Author

Currently, when youtube-dl reads the cookies file passed in with --cookies, it ignores any cookies (and fails to send them) that have a '0' or '' (blank string) in the 'expires' column. This is supposed to indicate that the specified cookie is a session cookie [and thus should expire when, e.g. the browser window is closed]. YouTube/Google also uses these cookies to store critical session information.

Thus, when the cookies are exported from the browser (for example, with this popular Chrome extension: https://chrome.google.com/webstore/detail/cookiestxt/njabckikapfpffapmjgojcnbfjonfjfg ), youtube-dl fails to send them, as a result cannot properly authenticate to YouTube, and so the download of private/restricted videos fails.

This problem is related to a bug in the Python libraries (see the original post I made for the link), and my changes make youtube-dl send these cookies in requests, but not save them to the resulting cookiejar when the script is done executing.

@dstftw
Copy link
Collaborator

dstftw commented May 26, 2017

You didn't read my question. What concretely does not work due to dropping browser session cookies?

@elipsitz
Copy link
Contributor Author

elipsitz commented May 26, 2017

Sorry, I thought I answered it.
Downloading private videos using a dump of browser cookies (passed in with --cookies) does not work due to this problem.

[[ See also a reference to this convention (0 or blank for session cookie expiration times) in the documentation for wget (under --keep-session-cookies) https://www.gnu.org/software/wget/manual/html_node/HTTP-Options.html ]]

@dstftw
Copy link
Collaborator

dstftw commented May 26, 2017

Private videos work just fine for me with current cookie code.

@elipsitz
Copy link
Contributor Author

What method are you using to export your browser's cookies? Also, what is the expiration time listed for the cookie ".youtube.com : SID"? I'm still unable to do this, even with the latest version (2017.05.26).

@dstftw
Copy link
Collaborator

dstftw commented May 26, 2017

I use plugins from https://github.com/rg3/youtube-dl#how-do-i-pass-cookies-to-youtube-dl. Expiry time for SID is about two years from now.

@elipsitz
Copy link
Contributor Author

elipsitz commented May 26, 2017

Okay, I think I see the reason for why this isn't working for me, but it is for you.
When I log in with my personal email@gmail.com, everything works fine, cookie expiration dates are as expected.
When I log in with my school email address (which uses Google Apps -- this was my original use case... normal --username and --password login does not work with Google Apps accounts), email@berkeley.edu, the cookie expiration dates are 0, not actual dates in the future. I suspect this might have to do with some sort of default 'remember me = false' option, but I do not know for sure.

Regardless, updated problem: Downloading private videos using a dump of browser cookies from a Google Apps login (passed in with --cookies) does not work.

@elipsitz
Copy link
Contributor Author

elipsitz commented Jul 3, 2017

Any chance of this getting merged? It is a bit of a niche situation, I realize, but I do believe it is the correct behavior.

@elipsitz
Copy link
Contributor Author

@dstftw Is it possible to get another look at this?

@nyanpasu64
Copy link

Surprised to bump into another Berkeley student on an obscure Github issue!...

Anyway will this be merged anytime soon?

@elipsitz
Copy link
Contributor Author

elipsitz commented Oct 8, 2017

@jimbo1qaz as a temporary workaround, you can change all of the '0's for the expiration time in your cookies.txt to some large number so they're not removed

@nyanpasu64
Copy link

I already implemented that workaround.

I was trying to use mpv to crop webcasts, but I couldn't find any way to crop 50% of the screen. I ended up using CSS to hide half the video.

@dstftw dstftw closed this in 1bab343 Dec 8, 2018
haraldmartin referenced this pull request in allears-ai/youtube-dl Dec 12, 2018
lkho referenced this pull request in lkho/youtube-dl Dec 24, 2018
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

Successfully merging this pull request may close these issues.

None yet

3 participants