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

[jwplatfom] do not match video URLs(#20596) #22148

Merged
merged 1 commit into from
Sep 23, 2019
Merged

Conversation

ipaha
Copy link
Contributor

@ipaha ipaha commented Aug 18, 2019

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

URL 'https://content.jwplatform.com/videos/WbFsN7Kl-WdGqK18o.mp3?exp=1566156755\&sig=e60548f2434c12279baff74cf8327b9bd9ffc7383'
was broken by 7c072f0 and approach for the fix was introduced here
9c01725

[JWPlatform] WbFsN7Kl: Downloading JSON metadata
ERROR: Unable to download JSON metadata: HTTP Error 403: Forbidden (caused by HTTPError()); please report this issue on https://yt-dl.org/bug . Make sure you are using the latest version; see https://yt-dl.org/update on how to update. Be sure to call youtube-dl with the --verbose flag and include its complete output.

@dstftw
Copy link
Collaborator

dstftw commented Aug 27, 2019

This has no effect.

@dstftw dstftw closed this Aug 27, 2019
@dstftw dstftw added the invalid label Aug 27, 2019
@ipaha
Copy link
Contributor Author

ipaha commented Sep 1, 2019

I've checked the fix locally. Why you have left jwplatfom video URL broken?

@dstftw
Copy link
Collaborator

dstftw commented Sep 1, 2019

There are no telepathists here to know what you have checked. The URL provided does not work in both cases making this PR useless.

@ipaha
Copy link
Contributor Author

ipaha commented Sep 5, 2019

Sorry for the mess. Just copy and past here mail text to the author of the code line I am fixing. The author is @remitamine

Here we have 2 problems after this commit 7c072f0
Broken URLS:

  1. https://content.jwplatform.com/manifests/NdSc5Qlg.m3u8?exp=1554638831&sig=b7f01a3ce1e275dffe6ff74599f02ac5
  2. https://content.jwplatform.com/videos/WbFsN7Kl-WdGqK18o.mp3?exp=1566156755\&sig=e60548f2434c12279baff74cf8327b9bd9ffc7383'
    Both URLs were processed in youtube_dl/extractor/common.py
    before your commit.
    And now it is processed by youtube_dl/extractor/jwplatform.py

Both errorrs the same with different ID
[JWPlatform] WbFsN7Kl: Downloading JSON metadata
ERROR: Unable to download JSON metadata: HTTP Error 403: Forbidden (caused by HTTPError());

This URL https://content.jwplatform.com/manifests/ you have fixed here 9c01725
And the output in this email chain is for manifest URL. Now manifest URL is working and processed by extractor/common.py
https://content.jwplatform.com/videos/ was fixed by me here https://github.com/ytdl-org/youtube-dl/pull/22148/files with the same approach and similar success download not listed here.

To check URL I use my paid account with temporary URL with my credentials and expiration as HTTP GET parameters ?exp=1566156755&sig=e60548f2434c12279baff74cf8327b9bd9ffc7383
That is why @dstftw can't use this url directly to check downloading.

I need to contribute the fix. Can you support?
Please

And @remitamine aswer:
the PR is not handled by me, so it would be better to explain directly on the PR.

@dstftw
Help me. Please.

@remitamine remitamine reopened this Sep 23, 2019
@remitamine remitamine merged commit 7d327fe into ytdl-org:master Sep 23, 2019
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.

3 participants