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

Cookie fixes #6897

Merged
merged 8 commits into from May 29, 2023
Merged

Cookie fixes #6897

merged 8 commits into from May 29, 2023

Conversation

mbway
Copy link
Contributor

@mbway mbway commented Apr 23, 2023

Description of your pull request and other information

Brings --cookies-from-browser up to date with recent changes to chromium.

Prompted by #6880 I had a look at what had changed in chromium (note: link takes a long time to load) since the feature was initially added. It turned out none of the changes were relevant to that issue though. The notable changes relevant to yt-dlp are:

  • more supported (auto detected) Linux desktop environments
  • better KDE/KWallet support (including KDE6 which I didn't test because even KDE Neon testing edition is still on plasma 5 and I couldn't be bothered to compile from source)
  • fallback to empty password on Linux (chromium found and worked around the bug that I found first 😄 )
    • The implementation is probably quite fragile as yt-dlp only detects decryption failure by whether the value isn't valid UTF-8 but chromium seems to detect problems at the decryption level. Still, this situation should be rare enough not to worry about
  • on Windows the DPAPI encryption key is now labelled with the name of the browser but this doesn't seem to affect decryption
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?


else:
self._logger.warning(f'unknown cookie version: "{version}"', only_once=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be nice to log unknown prefixes on the other platforms as well to debug issues like #6880 but because mac and windows sometimes store plaintext cookies the log would be spammed with random values. In the Linux case though there is no valid reason for there to be an unknown prefix so it is helpful to log the value (eg if chromium added a v12 encryption or something)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, you can add it.

@Tatsh
Copy link
Contributor

Tatsh commented Apr 26, 2023

This seems to work on Linux but I am still getting this line:

[Cookies] Loading cookie      0/  1192WARNING: failed to decrypt cookie (AES-CBC) because UTF-8 decoding failed. Possibly the key is wrong?
[Cookies] Extracted 1192 cookies from chrome

The extraction does succeed with the cookies it could decrypt (which seems like all?).

@mbway
Copy link
Contributor Author

mbway commented May 1, 2023

how about now? I thought the fallback to empty case would be really rare but maybe not. Because none of the cookies eventually fail decryption my guess would be that the decryption with the password fails but then succeeds when attempting with the empty password

@Tatsh
Copy link
Contributor

Tatsh commented May 1, 2023

how about now? I thought the fallback to empty case would be really rare but maybe not. Because none of the cookies eventually fail decryption my guess would be that the decryption with the password fails but then succeeds when attempting with the empty password

Just updated and no longer getting that error, but I am still getting an error attempting to access a Patreon URI that worked before the Chrome change. Testing with this branch.

I use the code with this branch in my project patreon-archiver and this line seems to work or I do not see how I would be making the API requests. But at the bottom when YouTubeDL is called I always get the error ERROR: [Patreon] ...: You do not have access to this post.

Same happens on command line when I run this branch's yt-dlp.

 $ poetry show | grep -F yt-dlp
yt-dlp             2023.3.4 86283d1 A youtube-dl fork with additional featu...
 $ command -v yt-dlp
/home/tatsh/.cache/pypoetry/virtualenvs/patreon-archiver-ldBlP-qe-py3.10/bin/yt-dlp
 $ yt-dlp --verbose 'https://www.patreon.com/posts/...-...'
[debug] Command-line config: ['--verbose', 'https://www.patreon.com/posts/...-...']
[debug] User config "/home/tatsh/.config/yt-dlp/config": ['--add-metadata', '--all-subs', '--convert-subs', 'srt', '--cookies-from-browser', 'chrome:Default', '--download-archive', '~/.local/share/youtube-dl-archive', '--embed-chapters', '--embed-metadata', '--embed-subs', '--embed-thumbnail', '--exec', 'mp4json {}', '--geo-bypass', '--hls-use-mpegts', '--merge-output-format', 'mkv', '--netrc', '--no-overwrites', '--output', '%(title).128s___src=%(extractor)s___id=%(id)s.%(ext)s', '--restrict-filenames', '--skip-unavailable-fragments', '--sponsorblock-mark', 'all', '--sub-langs', 'all', '--write-info-json', '--write-subs']
[debug] Encodings: locale UTF-8, fs utf-8, pref UTF-8, out utf-8, error utf-8, screen utf-8
[debug] yt-dlp version stable@2023.03.04 [392389b7d]
[debug] Lazy loading extractors is disabled
[debug] Python 3.10.11 (CPython x86_64 64bit) - Linux-6.3.0-gentoo-limelight-x86_64-11th_Gen_Intel-R-_Core-TM-_i9-11900K_@_3.50GHz-with-glibc2.37 (OpenSSL 1.1.1t  7 Feb 2023, glibc 2.37)
[debug] exe versions: ffmpeg 4.4.4 (fdk,setts), ffprobe 4.4.4, phantomjs 2.1.1, rtmpdump 2.4
[debug] Optional libraries: Cryptodome-3.17, brotli-1.0.9, certifi-2022.12.07, mutagen-1.46.0, sqlite3-2.6.0, websockets-11.0.2
[Cookies] Extracting cookies from chrome
[debug] Extracting cookies from: "/home/tatsh/.config/google-chrome/Default/Cookies"
[Cookies] Loading cookie      0/  1053[debug] detected desktop environment: KDE5
[debug] Chosen keyring: KWALLET5
[debug] using kwallet-query to obtain password from KWALLET5
[debug] NetworkWallet = "kdewallet"
[debug] password found
[Cookies] Extracted 1053 cookies from chrome
[debug] cookie version breakdown: {'v10': 3, 'v11': 1036, 'other': 0, 'unencrypted': 14}
[debug] Proxy map: {}
[debug] Loaded 1802 extractors
[debug] Loading archive file '/home/tatsh/.local/share/youtube-dl-archive'
[Patreon] Extracting URL: https://www.patreon.com/posts/...-...
[Patreon] ...: Downloading API JSON
ERROR: [Patreon] ...: You do not have access to this post
  File "/home/tatsh/.cache/pypoetry/virtualenvs/patreon-archiver-ldBlP-qe-py3.10/lib/python3.10/site-packages/yt_dlp/extractor/common.py", line 694, in extract
    ie_result = self._real_extract(url)
  File "/home/tatsh/.cache/pypoetry/virtualenvs/patreon-archiver-ldBlP-qe-py3.10/lib/python3.10/site-packages/yt_dlp/extractor/patreon.py", line 287, in _real_extract
    self.raise_no_formats('You do not have access to this post', video_id=video_id, expected=True)
  File "/home/tatsh/.cache/pypoetry/virtualenvs/patreon-archiver-ldBlP-qe-py3.10/lib/python3.10/site-packages/yt_dlp/extractor/common.py", line 1172, in raise_no_formats
    raise ExtractorError(msg, expected=expected, video_id=video_id)

@Tatsh
Copy link
Contributor

Tatsh commented May 1, 2023

I debugged some more and the session_id cookie is coming out empty.

 Cookie(version=0, name='session_id', value='', port=None, port_specified=False, domain='.patreon.com', domain_specified=True, domain_initial_dot=True, path='/', path_specified=True, secure=1, expires=13352869710437688, discard=False, comment=None, comment_url=None, rest={}, rfc2109=False

In the browser it definitely has a value:

image

Copy link
Member

@pukkandan pukkandan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I merge this, or are you looking into #6880, #6897 (comment)?


else:
self._logger.warning(f'unknown cookie version: "{version}"', only_once=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, you can add it.

@Tatsh
Copy link
Contributor

Tatsh commented May 7, 2023

@mbway I think you need to rebase this.

@pukkandan
Copy link
Member

I think you need to rebase this.

No

@Tatsh
Copy link
Contributor

Tatsh commented May 7, 2023

For me this change fixes a lot but the session cookie is still coming as empty string.

@mbway
Copy link
Contributor Author

mbway commented May 8, 2023

Should I merge this

It seems that @Tatsh is still having an issue so maybe not.
#6880 is unrelated to these changes I think.

I have already looked over the code again to see why the session cookie might not be decrypting properly but I can't see any reason for it. Since you are a developer @Tatsh do you think you could provide a minimal example that is failing for you? so perhaps a cookie database containing junk data and let me know what desktop environment you are using

@Tatsh
Copy link
Contributor

Tatsh commented May 8, 2023

I will look into the issue today. I too would like to see this merged ASAP.

@Tatsh
Copy link
Contributor

Tatsh commented May 12, 2023

Had a deep look and tested on another machine. Cookie extraction is working well. The above issue I reported is no longer happening. Let's merge this?

@pukkandan pukkandan force-pushed the master branch 2 times, most recently from ee280c7 to 7aeda6c Compare May 24, 2023 18:09
@pukkandan pukkandan merged commit b38d4c9 into yt-dlp:master May 29, 2023
11 checks passed
aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
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