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

Singleton Headers (such as User-Agent) are separated at comma when using httpadapter.NewV2 #178

Open
hohmannr opened this issue May 22, 2023 · 1 comment

Comments

@hohmannr
Copy link
Contributor

hohmannr commented May 22, 2023

Bug

Description

When using *httpadapter.HandlerAdapterV2 to proxy a events.APIGatewayV2HTTPRequest, single-value headers, such as User-Agent, Authorization, etc. which may contain commas in their value, are cut into multiple http.Header values when proxying. This leads to strange behaviour with methods like *http.Request.UserAgent(), which results in a cut-off User-Agent.

Reproduction

package main

import (
	"context"
	"fmt"
	"net/http"

	"github.com/aws/aws-lambda-go/events"
	"github.com/awslabs/aws-lambda-go-api-proxy/httpadapter"
)

func Example() {
	adapter := httpadapter.NewV2(http.HandlerFunc(handler))

	// User-Agent gets cut at its comma value
	adapter.ProxyWithContext(context.Background(), apiGatewayV2Request())
	// Output: User-Agent: Mozilla/5.0 (Linux; Android 11; Pixel 5 Build/RQ3A.210805.001.A1; wv) AppleWebKit/537.36 (KHTML
}

func apiGatewayV2Request() events.APIGatewayV2HTTPRequest {
	return events.APIGatewayV2HTTPRequest{
		Headers: map[string]string{
			// normal multi-valued header
			"Some-Multi-Value-Header": "value1,value2,value3",
			// singleton header (only a single value is allowed, which can be comma-separated)
			"User-Agent": "Mozilla/5.0 (Linux; Android 11; Pixel 5 Build/RQ3A.210805.001.A1; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/92.0.4515.159 Mobile Safari/537.36",
		},
	}
}

func handler(w http.ResponseWriter, r *http.Request) {
	fmt.Printf("User-Agent: %s", r.UserAgent())

	w.WriteHeader(http.StatusOK)
}

Fix

Firstly, in *RequestAccessorV2.EventToRequest one should split the header fields into single-value (singleton) headers and multi-value capabale (multitons) headers. Then parse the singletons as a single value and the multitons as a comma separted list. This fix has been implemented, see !179. I have used
mozilla's list for singleton headers, as well as net/textproto.CanonicalMIMEHeaderKey to make the singleton-multiton-split case-insensitive. Test cases have also been added (disclaimer: its the first time for me to use ginkgo, hopefully what I did is correct).

hohmannr added a commit to hohmannr/aws-lambda-go-api-proxy that referenced this issue May 22, 2023
sapessi added a commit that referenced this issue Sep 22, 2023
fix(requestv2): fixes multiton/singleton header parsing (#178)
@givsly-stephen
Copy link

givsly-stephen commented Dec 14, 2023

I am noticing that the Stripe-Signature header is being truncated. I am still investigating. But it's happening in my golang Lambda with Fiber v2 and this adapter.

Stripe-Signature: t=1492774577,v1=5257a869e7ecebeda32affa62cdca3fa51cad7e77a0e56ff536d0ce8e108d8bd,v0=6ffbb59b2300aae63f272406069a9788598b792a944a07aba816edb039989a39

So, it the fix might not include this scenario.

Using
github.com/awslabs/aws-lambda-go-api-proxy v0.16.0

Confirmed by looking at the raw headers that Stripe-Signature was SPLIT into when using c.GetReqHeaders()

Stripe-Signature:[t=1702555126 v1=a9a17a234ecb883eda3863ba1aa2a19a1783a585dbac352c3e0194db6f16524e v0=a7f227f2366b925a99a278f9a972c69c3c85de2461b9b5b4dd1cf53e931a2ac1]

I can re-join for now, but this is still a bug then.

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

No branches or pull requests

2 participants