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

_TEST_FILE_SIZE not 10 KiB #3846

Closed
d912e3 opened this issue Sep 28, 2014 · 1 comment
Closed

_TEST_FILE_SIZE not 10 KiB #3846

d912e3 opened this issue Sep 28, 2014 · 1 comment

Comments

@d912e3
Copy link
Contributor

@d912e3 d912e3 commented Sep 28, 2014

In test mode, only the first 10 KiB of a file are said to be downloaded and its MD5 hash compared to the value specified in the test case.

While writing a new extractor, I naturally tried to compute the hash as follows:

$ curl $URL | head -c 10K | md5sum

Which only gave a hash that wouldn’t match…

Looking up the definition of the _TEST_FILE_SIZE and the use of it in an HTTP Range request revealed that it isn’t 10 KiB (10240 B) that is downloaded but actually 10241 B since Range: bytes=0-10240 requests the bytes from 0 to 10240, inclusively:

$ curl -r 0-10240 $URL | wc -c
10241
$ curl -r 0-10239 $URL | wc -c
10240

I have no idea what’s the best way to fix this. Changing the definition of _TEST_FILE_SIZE would render probably almost every test case invalid. Updating the respective line in the template may be sufficient.

But maybe the hash is not even meant to be computed the way I tried. Maybe you just enter an arbitrary string for the hash and let the test fail and use the expected string instead…

@dstftw
Copy link
Collaborator

@dstftw dstftw commented Sep 28, 2014

Test file size is indeed 10241 bytes and this is probably due to historical reasons. As you noticed, changing the _TEST_FILE_SIZE will require checksum recalculation for each test.
Hovewer, there is an idiomatic and expected way to fetch the file of proper size - you just pass --test to youtube-dl along with URL (this results in downloading first 10241 bytes of the media) following by md5sum on the downloaded file.
Thanks for the report.

@dstftw dstftw closed this Sep 28, 2014
dstftw added a commit that referenced this issue Sep 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.