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

runtime.MarshalerForRequest: Content-Type should have been parsed before querying mimeMap #1505

Closed
movsb opened this issue Jul 6, 2020 · 2 comments · Fixed by #1506
Closed

Comments

@movsb
Copy link
Contributor

movsb commented Jul 6, 2020

🐛 Bug Report

The Content-Type header field of http.Request may have parameters (MDN):

Content-Type: text/html; charset=UTF-8
Content-Type: multipart/form-data; boundary=something

But when we register a mime into mimeMap, we don't care them:

var m your.MsgPackMarshaler
mux := runtime.NewServeMux(
    runtime.WithMarshalerOption("application/x-msgpack", m),
)

A Content-Type with parameters causes no match:

for _, contentTypeVal := range r.Header[contentTypeHeader] {
if m, ok := mux.marshalers.mimeMap[contentTypeVal]; ok {
inbound = m
break
}
}

To Reproduce

Upload a file and we can see that Content-Type gets different parameters, every time:

$ curl -vF file=@go.mod localhost:2564/v3/ping 
*   Trying ::1...
* TCP_NODELAY set
* Connection failed
* connect to ::1 port 2564 failed: Connection refused
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 2564 (#0)
> POST /v3/ping HTTP/1.1
> Host: localhost:2564
> User-Agent: curl/7.64.1
> Accept: */*
> Content-Length: 1312
> Content-Type: multipart/form-data; boundary=------------------------574891a52a1a9324
> Expect: 100-continue
> 
< HTTP/1.1 100 Continue
* We are completely uploaded and fine
< HTTP/1.1 200 OK
< Content-Type: multipart/form-data
< Date: Mon, 06 Jul 2020 17:19:45 GMT
< Content-Length: 15
< 
* Connection #0 to host localhost left intact
{"pong":"pong"}* Closing connection 0

Expected behavior

Parse Content-Type before querying mimeMap:

diff --git a/runtime/marshaler_registry.go b/runtime/marshaler_registry.go
index 5cc53ae..17e63e4 100644
--- a/runtime/marshaler_registry.go
+++ b/runtime/marshaler_registry.go
@@ -2,6 +2,7 @@ package runtime
 
 import (
 	"errors"
+	"mime"
 	"net/http"
 )
 
@@ -31,7 +32,11 @@ func MarshalerForRequest(mux *ServeMux, r *http.Request) (inbound Marshaler, out
 	}
 
 	for _, contentTypeVal := range r.Header[contentTypeHeader] {
-		if m, ok := mux.marshalers.mimeMap[contentTypeVal]; ok {
+		contentType, _, err := mime.ParseMediaType(contentTypeVal)
+		if err != nil {
+			continue
+		}
+		if m, ok := mux.marshalers.mimeMap[contentType]; ok {
 			inbound = m
 			break
 		}

Actual Behavior

No marshaler was found.

Your Environment

Any.

@johanbrandhorst
Copy link
Collaborator

Hey, great find! Your fix looks pretty simple, would you care to create a pull request so we can get this fix in? Thanks!

@movsb
Copy link
Contributor Author

movsb commented Jul 6, 2020

Hi, @johanbrandhorst , pull request sent. Thanks. If test cases are required, I'll be willing to write one.

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 a pull request may close this issue.

2 participants