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.

Sign up for GitHub

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

Jump to bottom

mime: ParseMediaType reports an error on value that is not quoted but contains a space #43721

Closed
candeemis opened this issue Jan 15, 2021 · 9 comments
Closed

mime: ParseMediaType reports an error on value that is not quoted but contains a space #43721

candeemis opened this issue Jan 15, 2021 · 9 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@candeemis
Copy link

candeemis commented Jan 15, 2021 •
edited by mdlayher
Loading

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

$ go version 1.15

Does this issue reproduce with the latest release?

Yes

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

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/candeemis/Library/Caches/go-build"
GOENV="/Users/candeemis/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/candeemis/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/candeemis/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/candeemis/play-ground/go-mime-parser/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/z9/3q3874k11pd_h45kz05lngr40000gn/T/go-build706623001=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

After spending a few hours on debugging, I found that ParseMediaType function in mime/mediatype.go module throws the following error while parsing media type from the content type as application/pdf; x-unix-mode=0644; name=RG Mellowmessage 4.12.20.pdf
error:

Failed to ReadParts: mime: invalid media parameter

ParseMediaType function internally calls another function consumeMediaParam to parse the individual media types for example from the above-mentioned content type x-unix-mode and name (which is the name of the attached file) are two separate types. consumeMediaParam incorrectly parses the media type values if they contain any white space. For example, name contains the white spaces in the above-mentioned content type. So it assigns RG only as of the value to name key and keeps trying to parse the rest. Whereas, in the rest, there isn’t any key separated by = sign. That's why it throws the above-mentioned error.

What did you expect to see?

It should parse RG Mellowmessage 4.12.20.pdf as the value of name key. Which means it should ignore the white space in the value.

What did you see instead?

The following error:

Failed to ReadParts: mime: invalid media parameter
@mdlayher mdlayher changed the title Fails to parse Media Type with space in value mime: fails to parse Media Type with space in value Jan 15, 2021
@seankhliao
Copy link
Member

based on RFC 2045 Section 5.1

 parameter := attribute "=" value

 attribute := token
              ; Matching of attributes
              ; is ALWAYS case-insensitive.

 value := token / quoted-string

 token := 1*<any (US-ASCII) CHAR except SPACE, CTLs,
             or tspecials>

That looks like a malformed header

@candeemis
Copy link
Author

@seankhliao But the following doesn't rule out spaces in the value, no? 🤔

value := token / quoted-string

     token := 1*<any (US-ASCII) CHAR except SPACE, CTLs,
                 or tspecials>

     tspecials :=  "(" / ")" / "<" / ">" / "@" /
                   "," / ";" / ":" / "\" / <">
                   "/" / "[" / "]" / "?" / "="
                   ; Must be in quoted-string,
                   ; to use within parameter values

@seankhliao
Copy link
Member

it could contain spaces, but in a quoted string, not the bare token you demonstrated

@dmitshur
Copy link
Contributor

CC @bradfitz via owners.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 20, 2021
@dmitshur dmitshur added this to the Backlog milestone Jan 20, 2021
@dmitshur
Copy link
Contributor

@candeemis Do you agree that mime.ParseMediaType behavior of reporting an error on that input is correct, in that it matches the specification?

@dmitshur dmitshur added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 20, 2021
@candeemis
Copy link
Author

@dmitshur Well, I can say that the current implementation satisfies the RFC specification. Meanwhile, I still see room for improvement to go one step further a bit different implementation. That would not only satisfy the specification but also ignore minor problems like the one mentioned in the issue. I am working on that concept, but due to time constraint, I may take a bit of time.

@dmitshur dmitshur removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 20, 2021
@dmitshur dmitshur changed the title mime: fails to parse Media Type with space in value mime: ParseMediaType reports an error on value that is not quoted but contains a space Jan 20, 2021
@kode4food
Copy link

@candeemis Do you mean like observing Postel's Law?

@candeemis
Copy link
Author

Sorry for the very late reply. But yes, as kode4food mentioned above, we should be a bit easy on the implementation as suggested by the Postal's Law/Robustness Principle

@seankhliao
Copy link
Member

the robustness principle is now generally considered to be a bad idea, see https://datatracker.ietf.org/doc/html/rfc9413

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Sep 5, 2024
@gabyhelp gabyhelp mentioned this issue Sep 20, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@kode4food @dmitshur @seankhliao @candeemis