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

net/http: false positives checking for Ogg files #20513

Closed
gtownsend opened this issue May 27, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@gtownsend
Copy link

commented May 27, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.8.1 darwin/amd64

What operating system and processor architecture are you using (go env)?

MacOS Sierra 10.12.5

What did you do?

Wrote a test of http.DetectContentType() after reading the code.
(In net/http/sniff.go, the mask used to test for application/ogg is bogus.)

If possible, provide a recipe for reproducing the error.

A complete runnable program is good.
A link on play.golang.org is best.
https://play.golang.org/p/jr4yXFJZLZ

What did you expect to see?

A file beginning with "owow\000" would be classified as application/octet-stream

What did you see instead?

It is classified as application/ogg

@odeke-em

This comment has been minimized.

Copy link
Member

commented May 28, 2017

Hello there @gtownsend, thanks for filing the issue. Nice catch.

My apologies, the bug is that unfortunately I mismatched and used:

  • []byte("\x4F\x67\x67\x53\x00")
    instead of
  • []byte("\xFF\xFF\xFF\xFF\xFF")
    as the mask

    go/src/net/http/sniff.go

    Lines 109 to 113 in b9daf0a

    &maskedSig{
    mask: []byte("OggS\x00"),
    pat: []byte("\x4F\x67\x67\x53\x00"),
    ct: "application/ogg",
    },

The algorithm and reference that we use is at:
screen shot 2017-05-27 at 6 21 42 pm

I'll make a CL and a test for it, although Brad is on vacation for a week, but if anyone can help me review and merge it that'd be nice.

Anyways, the catch here is that the bug for "application/ogg" gives a false positive if the byte pattern BITWISE AND'd with 4F 67 67 53 00 in hex matches itself 4F 67 67 53 00 for example:

  • oooS
  • owow

@odeke-em odeke-em added the NeedsFix label May 28, 2017

@gopherbot

This comment has been minimized.

Copy link

commented May 28, 2017

CL https://golang.org/cl/44336 mentions this issue.

@odeke-em

This comment has been minimized.

Copy link
Member

commented Jun 13, 2017

/cc @bradfitz @rsc can this go in for Go1.9?

@gopherbot gopherbot closed this in f363817 Jun 13, 2017

@golang golang locked and limited conversation to collaborators Jun 13, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.