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

Weird videos #119

Closed
wants to merge 3 commits into from
Closed

Weird videos #119

wants to merge 3 commits into from

Conversation

psycotica0
Copy link

These commits fix some of the weird videos I've found that don't work in the current release.

On a branch I didn't push I added tests for them, but I couldn't figure out how to use the tests without putting ONLINE_TESTS in the environment.
Also, one of the tests doesn't work offline, since it actually checks the returned link.

So, I'll just outline the cases here, and you can use those to write tests as you see fit.

The first one, about age restricted videos, was to fix videos "FfM_wS7qYfY" and "pn1VGytzXus".
Both of them list an age restriction in their metadata, but don't present the sign in page. I don't know why, or what that means, but I've seen it.
Testing these videos, from my machine at least, gives a valid url, so the tests pass, but that url results in a 403 if it's fetched.
In order to help my own testing, I added code to make a head request against the url for format 18, and assert that the status code for that was not 403 (Forbidden).
I don't think this would work in offline, mode, though, so I didn't want to commit that test, because I'm not sure if you wanted it.

The second one is about video "PZp3xEdQc38", a video that runs that gamut of failures, ending up in XCDYouTubeVideoOperation's startNextRequest method's self.eventLabels.count == 0 branch.
This kicks off a chain that fetches the watch page, which fetches the embed page, which fetches the javascript, which fetches the video_info page.
This leads us to have an unscramble function, since we went through the js at some point, but the video actually doesn't have an "s" parameter or require a signature to be generated.

So, I just took out the part that leads to that being considered an error case, and the rest of the logic seems to work out.
It doesn't appear to ruin any of the other tests.

Hopefully this is useful.

I don't know why these videos exist, but sometimes there are videos which label
themselves as being age restricted, but don't require the user to sign in to
verify their age.

These were causing things to fail, since the embedPage didn't exist, since we
never needed to go through that code path.
The good news, though, is that we didn't need the embedPage because we had all
the info we needed on the current page.

We just had to reach out and use it.
So... this exists.
Apparently there are videos which are protected, but don't have an "s" parameter
and don't need signatures.

Essentially, it tries "embedded" tag, but that fails because it's protected, so
it tries "display_page".

That fails because it's age restricted.

We then do into the "no tags left" part of startNextRequest.
That loads the web, which loads the embed, which loads the javascript, which
finally loads the video_info.

We now have a playerScript, but don't need to sign requests.
We never returned the CipherRequired, we just went through a path that loaded
the JS anyway.

Neat stuff.
@psycotica0
Copy link
Author

Damn. I ran all the tests and they passed! It looks like the failures are related to the second commit, since they both deal with signed things that don't have a signature function. I'll look into that tomorrow.

Turns out there was a reason for that to fail.
If the signature should exist and doesn't, then it's a real failure.

So, in this case I've changed it to only fail if there's a playerScript without
a signature, where there was something to deescramble in the first place.
Seems to solve both issues.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.69% when pulling 9de5f49 on psycotica0:weird_videos into 646cba4 on 0xced:master.

@syrakozz
Copy link

+1

@0xced
Copy link
Owner

0xced commented Apr 28, 2015

Thank you for your pull request. I have applied these changes in 829e14a and 359b1ae and I added unit tests for those changes.

I have also just released version 2.1.2 where these issues are both fixed.

@0xced 0xced closed this Apr 28, 2015
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

4 participants