Pre Parsed Multi Value Headers not supported #33
Comments
|
@VojtechVitek @pkieltyka I have created an issue and already pushed the fix PR. Can we please get this reviewed? I ensured this would maintain the existing functionality to not be a breaking change. |
ChronosMasterOfAllTime
changed the title
|
For anyone that comes here, here is the current workaround:
func(next http.Handler) http.Handler {
// This middleware is temporary until chi cors middleware is fixed
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodOptions {
headers := r.Header.Values("Access-Control-Request-Headers")
for _, header := range headers {
if !utils.SliceContains(allowedHeaders, strings.ToLower(header)) {
logrus.Error("Invalid header: ", header)
w.Header().Del("Access-Control-Allow-Headers")
w.WriteHeader(http.StatusForbidden)
return
}
}
w.Header().Set("Access-Control-Allow-Headers", strings.Join(headers, ","))
w.WriteHeader(http.StatusOK)
} else {
next.ServeHTTP(w, r)
}
})
}) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem
HTTP headers by nature are multi value in the
http.Requesttype.When you are doing the
Access-Control-Request-Headersparse in the preflight checks, you assume it will always be a singleton. However, when using libraries such as the aws-lambda-go-api-proxy, it parses the headers into a multi value format before passing the request to chi. See splitSingletonHeaders as this tells the proxy how to handle the headers (The actual splitting work is done on requestV2#180). Once it does this, the values are already pre-split before it reaches the CORS middleware.The cors library always expects things to be in a single string, comma delimited format. By this nature, the
Getcall on the http.Header type will only return the first value and ignore the rest. This was causing our CORS preflight to fail as the response Access-Control-Allow-Headers were missing all the values.Solution
Use the
Valuesmethod and concatenate all the requestHeaders. It guarantees backwards compatibility and won't be a breaking change. I added test cases to fix this in PR #35 .The text was updated successfully, but these errors were encountered: