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: discrepancies in the MIME Sniffing Spec and the implementation #30570

Closed
kshitij10496 opened this issue Mar 4, 2019 · 5 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@kshitij10496
Copy link
Contributor

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

$ go version
go version devel +d71d81c5a2 Mon Mar 4 16:29:25 2019 +0530 darwin/amd64

While reading the MIME Sniffing Spec and the corresponding implementation in the net/http package in sniff.go, I found few discrepancies between the two. As, I do not know whether these diffs are intentional or not (though I am curious to know), I thought it best to share my findings so that a conscious decision can be taken.

Here are few suggestions for updating the current implementation in accordance with the spec:

  1. Using pattern rather than mask for assertion and iteration in the Pattern Matching Algorithm.
    Currently, we are using m.mask instead of m.pat at both the places.
    The reason this substitution works is because their lengths are the equal (implicit) in all the currently support MIME signatures, and the logical AND operation: a & b = c => a & c = b.
    Refer:

    go/src/net/http/sniff.go

    Lines 185 to 194 in d188767

    if len(data) < len(m.mask) {
    return ""
    }
    for i, mask := range m.mask {
    db := data[i] & mask
    if db != m.pat[i] {
    return ""
    }
    }
    return m.ct

  2. Rename image/vnd.microsoft.icon to image/x-icon and add another signature for the same.
    We are using the same signature for image/vnd.microsoft.icon instead of image/x-icon. This could be a possible bug or a decision that should be documented as a divergence from the spec.
    Refer:

    &exactSig{[]byte("\x00\x00\x01\x00"), "image/vnd.microsoft.icon"},

  3. Using named strings instead of hexadecimal representation in application/zip and application/x-rar-compressed.
    Reading the source code, I experienced that using human-readable string definition of the signatures is an implicit convention over the mechanistic byte pattern sequences (as it should be!). These are a couple of places which can be changed with the sole purpose of maintaining consistency and improving visual aesthetics.
    Refer:

    go/src/net/http/sniff.go

    Lines 146 to 147 in d188767

    &exactSig{[]byte("\x52\x61\x72\x20\x1A\x07\x00"), "application/x-rar-compressed"},
    &exactSig{[]byte("\x50\x4B\x03\x04"), "application/zip"},

  4. Reordering the sniffSignatures according to the section Identifying a resource with an unknown MIME type.
    In the current implementation, the signatures are almost sorted according to the spec, with a few exceptions. For example, matching video/webm before video/mp4.

@agnivade
Copy link
Contributor

agnivade commented Mar 4, 2019

/cc @bradfitz @odeke-em

@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 4, 2019
@bradfitz bradfitz added this to the Go1.13 milestone Mar 4, 2019
@bradfitz
Copy link
Contributor

bradfitz commented Mar 4, 2019

From memory, I don't think any of this is intentional. @odeke-em, you want to handle this? You tend to work on this code most frequently lately.

@kshitij10496
Copy link
Contributor Author

I would be happy to fix this and make my first contribution to the project! 😃

@odeke-em
Copy link
Member

odeke-em commented Mar 4, 2019

Thank you for filing this issue @kshitij10496 and welcome to the Go project! Awesome, please go for it and I am looking forward to reviewing your CL.

@gopherbot
Copy link

Change https://golang.org/cl/165277 mentions this issue: net/http: remove discrepancies between the MIME Sniffing Spec and its implementation

@golang golang locked and limited conversation to collaborators Mar 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants