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

Fix mp2/mp3 checks to not catch mp1 #205

Merged
merged 1 commit into from Apr 19, 2019

Conversation

stroncium
Copy link
Contributor

@stroncium stroncium commented Apr 15, 2019

Subject.

Anyhow, checks for mp2/mp3 were incorrect and actually caught also mp1, so I fixed that.

As for mp1, mpeg 1 layer 1 crc protected collides with utf16 BOM and there is no other markers, also rarely used. Current file sources ignore it for that reason.

@sindresorhus Also, the whole mp2/3 part is kinda weird.

  • it is a stream, yes, and can be resumed, but it's weird to do that for the purpose of file type detection, as you can detect any file with embedded mp3 incorrectly. (libmagic doesn't do that for example)
  • even if there is no embedded mp3,the stream is well compressed and matching those simple checks at arbitrary offsets will bring false positives on most files around
  • checking just offsets 0 and 1 is completely counter-intuitive, it should be either all reachable offsets or just the start, otherwise it's just asking for hard to debug bugs
  • ID3 part doesn't belong there, ID3v2 is embedded either at beginning or end of file

Fixes #163

@sindresorhus sindresorhus changed the title changed mpeg1/2 masks to not catch layer 1, fixes #163 Fix mp2/mp3 checks to not catch mp1 Apr 19, 2019
@sindresorhus sindresorhus merged commit 44b3c34 into sindresorhus:master Apr 19, 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.

UTF-16LE encoded text files are identified as mp3
2 participants