diff --git a/internal/delivery/engine_test.go b/internal/delivery/engine_test.go index 213b13a..13d1625 100644 --- a/internal/delivery/engine_test.go +++ b/internal/delivery/engine_test.go @@ -1166,6 +1166,10 @@ func TestIsForwardableHeader(t *testing.T) { assert.False(t, delivery.ExportIsForwardableHeader("Content-Length"), ) + + assert.False(t, + delivery.ExportIsForwardableHeader("Content-Type"), + ) } func TestTruncate(t *testing.T) { @@ -1247,6 +1251,81 @@ func TestDoHTTPRequest_ForwardsHeaders(t *testing.T) { ) } +// The event's stored inbound headers carry the same Content-Type the +// receiver saved as the event's ContentType, so a delivery could send +// it twice. It must go out exactly once, with a Content-Type configured +// on the target winning, then the event's ContentType. +func TestApplyRequestHeaders_SendsOneContentType(t *testing.T) { + t.Parallel() + + cases := map[string]struct { + inbound string + event string + configured string + want []string + }{ + "inbound and event agree": { + inbound: testContentType, + event: testContentType, + want: []string{testContentType}, + }, + "inbound and event disagree": { + inbound: "text/plain", + event: testContentType, + want: []string{testContentType}, + }, + "event has none": { + inbound: testContentType, + want: nil, + }, + "target configures its own": { + inbound: testContentType, + event: testContentType, + configured: "application/xml", + want: []string{"application/xml"}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + inbound, err := json.Marshal(map[string][]string{ + headerContentType: {tc.inbound}, + }) + require.NoError(t, err) + + cfg := &delivery.HTTPTargetConfig{} + if tc.configured != "" { + cfg.Headers = map[string]string{ + headerContentType: tc.configured, + } + } + + req, err := http.NewRequestWithContext( + context.Background(), + http.MethodPost, + "https://target.example.com/hook", + http.NoBody, + ) + require.NoError(t, err) + + delivery.ExportApplyRequestHeaders( + req, + &database.Event{ + Headers: string(inbound), + ContentType: tc.event, + }, + cfg, + ) + + assert.Equal(t, + tc.want, req.Header.Values(headerContentType), + ) + }) + } +} + func TestProcessDelivery_RoutesToCorrectHandler( t *testing.T, ) { diff --git a/internal/delivery/target_headers_test.go b/internal/delivery/target_headers_test.go index 1b1c7ae..3237132 100644 --- a/internal/delivery/target_headers_test.go +++ b/internal/delivery/target_headers_test.go @@ -11,10 +11,11 @@ import ( "sneak.berlin/go/webhooker/internal/delivery" ) -// Literals these tests repeat, named so that the header name and the +// Literals these tests repeat, named so that the header names and the // keep-forever archive config each have one definition. const ( headerAuthorization = "Authorization" + headerContentType = "Content-Type" bearerValue = "Bearer abc" archiveConfigNever = "{\"expiry\":\"never\"}" ) diff --git a/internal/delivery/target_http.go b/internal/delivery/target_http.go index fc264f9..9c7b539 100644 --- a/internal/delivery/target_http.go +++ b/internal/delivery/target_http.go @@ -541,6 +541,11 @@ func isForwardableHeader(name string) bool { "Upgrade", "Proxy-Authorization", "Proxy-Connection", "Content-Length": return false + case "Content-Type": + // applyRequestHeaders sets Content-Type itself. The receiver + // already stored this inbound value as the event's + // ContentType, so forwarding it too would send it twice. + return false default: return true } @@ -553,6 +558,10 @@ func isForwardableHeader(name string) bool { // policy strips exactly that set on a hop that leaves the origin, // so the forward set is decided here and only here — a header added // to it is covered off-origin without a second edit elsewhere. +// +// Content-Type goes out once: a Content-Type configured on the target +// wins, otherwise the event's ContentType, otherwise none. The inbound +// Content-Type in the event's headers is never forwarded. func applyRequestHeaders( req *http.Request, event *database.Event, @@ -573,10 +582,10 @@ func applyRequestHeaders( req.Header.Set("User-Agent", "webhooker/1.0") - // Content-Type describes the body being sent rather than the - // sender, and the delivery path sets it from the event itself. - // A 307/308 preserves the body across hosts, so stripping it - // would send that body untyped. + // A Content-Type configured on the target describes the body + // being sent rather than the sender. A 307/308 preserves the + // body across hosts, so stripping it would send that body + // untyped. delete(originScoped, "Content-Type") // User-Agent is overwritten just above, so an inbound one never