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

[utils] Handle user:pass in URLs #28801

Merged
merged 3 commits into from
Mar 4, 2024
Merged

[utils] Handle user:pass in URLs #28801

merged 3 commits into from
Mar 4, 2024

Conversation

hhirtz
Copy link
Contributor

@hhirtz hhirtz commented Apr 19, 2021

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

Fixes "nonnumeric port" errors when youtube-dl is given URLs with
usernames and passwords such as:

http://username:[email protected]/myvideo.mp4

Refs:

Fixes #18276 (point 4)
Fixes #20258
Fixes #26211 (see comment)

@hhirtz hhirtz force-pushed the user-pass branch 2 times, most recently from 4afd438 to 664d87b Compare April 19, 2021 13:15
@jeanthom
Copy link

LGTM

@hhirtz
Copy link
Contributor Author

hhirtz commented Jul 2, 2021

Simplified the code and ticked the "improvement" box instead of the "bug fix" one

Fixes "nonnumeric port" errors when youtube-dl is given URLs with
usernames and passwords such as:

    http://username:[email protected]/myvideo.mp4

Refs:
- https://en.wikipedia.org/wiki/Basic_access_authentication
- https://tools.ietf.org/html/rfc1738#section-3.1
- https://docs.python.org/3.8/library/urllib.parse.html#urllib.parse.urlsplit

Fixes ytdl-org#18276 (point 4)
Fixes ytdl-org#20258
Fixes ytdl-org#26211 (see comment)
@hhirtz
Copy link
Contributor Author

hhirtz commented Jul 6, 2021

rebased the branch

@jeanthom
Copy link

jeanthom commented Jul 6, 2021

still LGTM

@jeanthom
Copy link

Dear maintainers, please take some time to consider this issue, it finally adds a very important functionality to youtube-dl!

@hhirtz hhirtz marked this pull request as draft July 18, 2021 13:40
@hhirtz hhirtz marked this pull request as ready for review July 18, 2021 13:40
Copy link
Contributor

@dirkf dirkf left a comment

Choose a reason for hiding this comment

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

Let's finally add this, but with a structure matching yt-dlp.

Comment on lines 2158 to 2181
return compat_urllib_request.Request(sanitize_url(url), *args, **kwargs)
url = sanitize_url(url)
url, username, password = extract_user_pass(url)
if username is not None:
# password is not None
auth_payload = username + ':' + password
auth_payload = base64.b64encode(auth_payload.encode('utf-8')).decode('utf-8')
auth_header = 'Basic ' + auth_payload
headers = args[1] if len(args) >= 2 else kwargs.setdefault('headers', {})
headers['Authorization'] = auth_header
Copy link
Contributor

Choose a reason for hiding this comment

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

Align with yt-dlp:

Suggested change
return compat_urllib_request.Request(sanitize_url(url), *args, **kwargs)
url = sanitize_url(url)
url, username, password = extract_user_pass(url)
if username is not None:
# password is not None
auth_payload = username + ':' + password
auth_payload = base64.b64encode(auth_payload.encode('utf-8')).decode('utf-8')
auth_header = 'Basic ' + auth_payload
headers = args[1] if len(args) >= 2 else kwargs.setdefault('headers', {})
headers['Authorization'] = auth_header
url, auth_header = extract_basic_auth(escape_url(sanitize_url(url)))
if auth_header is not None:
headers = args[1] if len(args) > 1 else kwargs.get'headers')
headers = headers or {}
headers['Authorization'] = auth_header
if len(args) <= 1 and kwargs.get('headers') is None:
kwargs['headers'] = headers
kwargs = compat_kwargs(kwargs)

youtube_dl/utils.py Outdated Show resolved Hide resolved
test/test_utils.py Outdated Show resolved Hide resolved
@@ -65,6 +65,8 @@
sanitize_filename,
sanitize_path,
sanitize_url,
extract_user_pass,
Copy link
Contributor

Choose a reason for hiding this comment

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

Align with yt-dlp:

Suggested change
extract_user_pass,

test/test_utils.py Outdated Show resolved Hide resolved
youtube_dl/utils.py Outdated Show resolved Hide resolved
@dirkf dirkf merged commit f0812d7 into ytdl-org:master Mar 4, 2024
14 checks passed
github-actions bot added a commit to hellopony/youtube-dl that referenced this pull request Mar 4, 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
3 participants