Send Content-Type once on a delivery (closes #246)
A delivery set Content-Type from the event's ContentType and then added the inbound Content-Type from the event's stored headers, so the target could receive two values. The inbound Content-Type is no longer forwarded from the stored headers; the receiver already saved it as the event's ContentType. Which value wins is now stated at applyRequestHeaders: a Content-Type configured on the target, otherwise the event's ContentType, otherwise none. Model: opus-5-5
This commit is contained in:
@@ -1166,6 +1166,10 @@ func TestIsForwardableHeader(t *testing.T) {
|
|||||||
assert.False(t,
|
assert.False(t,
|
||||||
delivery.ExportIsForwardableHeader("Content-Length"),
|
delivery.ExportIsForwardableHeader("Content-Length"),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
assert.False(t,
|
||||||
|
delivery.ExportIsForwardableHeader("Content-Type"),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestTruncate(t *testing.T) {
|
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(
|
func TestProcessDelivery_RoutesToCorrectHandler(
|
||||||
t *testing.T,
|
t *testing.T,
|
||||||
) {
|
) {
|
||||||
|
|||||||
@@ -11,10 +11,11 @@ import (
|
|||||||
"sneak.berlin/go/webhooker/internal/delivery"
|
"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.
|
// keep-forever archive config each have one definition.
|
||||||
const (
|
const (
|
||||||
headerAuthorization = "Authorization"
|
headerAuthorization = "Authorization"
|
||||||
|
headerContentType = "Content-Type"
|
||||||
bearerValue = "Bearer abc"
|
bearerValue = "Bearer abc"
|
||||||
archiveConfigNever = "{\"expiry\":\"never\"}"
|
archiveConfigNever = "{\"expiry\":\"never\"}"
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -541,6 +541,11 @@ func isForwardableHeader(name string) bool {
|
|||||||
"Upgrade", "Proxy-Authorization",
|
"Upgrade", "Proxy-Authorization",
|
||||||
"Proxy-Connection", "Content-Length":
|
"Proxy-Connection", "Content-Length":
|
||||||
return false
|
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:
|
default:
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
@@ -553,6 +558,10 @@ func isForwardableHeader(name string) bool {
|
|||||||
// policy strips exactly that set on a hop that leaves the origin,
|
// 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
|
// so the forward set is decided here and only here — a header added
|
||||||
// to it is covered off-origin without a second edit elsewhere.
|
// 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(
|
func applyRequestHeaders(
|
||||||
req *http.Request,
|
req *http.Request,
|
||||||
event *database.Event,
|
event *database.Event,
|
||||||
@@ -573,10 +582,10 @@ func applyRequestHeaders(
|
|||||||
|
|
||||||
req.Header.Set("User-Agent", "webhooker/1.0")
|
req.Header.Set("User-Agent", "webhooker/1.0")
|
||||||
|
|
||||||
// Content-Type describes the body being sent rather than the
|
// A Content-Type configured on the target describes the body
|
||||||
// sender, and the delivery path sets it from the event itself.
|
// being sent rather than the sender. A 307/308 preserves the
|
||||||
// A 307/308 preserves the body across hosts, so stripping it
|
// body across hosts, so stripping it would send that body
|
||||||
// would send that body untyped.
|
// untyped.
|
||||||
delete(originScoped, "Content-Type")
|
delete(originScoped, "Content-Type")
|
||||||
|
|
||||||
// User-Agent is overwritten just above, so an inbound one never
|
// User-Agent is overwritten just above, so an inbound one never
|
||||||
|
|||||||
Reference in New Issue
Block a user