A delivery to an HTTP target could carry two Content-Type values: applyRequestHeaders set it from the event's ContentType, then the inbound Content-Type from the event's stored headers was added on top. Now the inbound Content-Type is never copied from the stored headers (isForwardableHeader returns false for it). The receiver already saves that same value as the event's ContentType, so nothing is lost.
Which value goes out is now stated in the comment on applyRequestHeaders:
a Content-Type configured on the target;
otherwise the event's ContentType;
otherwise none.
Redirects: the delete(originScoped, "Content-Type") stays. Only a Content-Type configured on the target can reach that set now, and it must still survive a cross-origin 307/308 with the body. Its comment now says that.
The new test checks the outbound request has exactly one value in four cases: inbound and event agree, they disagree (the event's wins), the event has none (none is sent), and the target configures its own (that one wins).
Behaviour change: an event with an empty ContentType whose stored headers carry a Content-Type is now delivered with no Content-Type at all, as the plan specified.
The case where the target configures its own Content-Type already gave one value before this change. The test keeps it as a guard on the stated order.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/webhooker/issues/246.
A delivery to an HTTP target could carry two `Content-Type` values: `applyRequestHeaders` set it from the event's `ContentType`, then the inbound `Content-Type` from the event's stored headers was added on top. Now the inbound `Content-Type` is never copied from the stored headers (`isForwardableHeader` returns false for it). The receiver already saves that same value as the event's `ContentType`, so nothing is lost.
Which value goes out is now stated in the comment on `applyRequestHeaders`:
1. a `Content-Type` configured on the target;
2. otherwise the event's `ContentType`;
3. otherwise none.
Redirects: the `delete(originScoped, "Content-Type")` stays. Only a `Content-Type` configured on the target can reach that set now, and it must still survive a cross-origin 307/308 with the body. Its comment now says that.
The new test checks the outbound request has exactly one value in four cases: inbound and event agree, they disagree (the event's wins), the event has none (none is sent), and the target configures its own (that one wins).
- Behaviour change: an event with an empty `ContentType` whose stored headers carry a `Content-Type` is now delivered with no `Content-Type` at all, as the plan specified.
- The case where the target configures its own `Content-Type` already gave one value before this change. The test keeps it as a guard on the stated order.
Model: opus-5-5
internal/delivery/redirect_test.go, TestApplyRequestHeaders_ReportsOriginScopedNames: after this PR, nothing tests delete(originScoped, "Content-Type") in applyRequestHeaders (internal/delivery/target_http.go). This test used to be that line's only coverage, but its fixture puts Content-Type only in the event's stored headers, and isForwardableHeader now drops that earlier, so removing the line fails no test. A later change that strips a target-configured Content-Type on a cross-origin 307/308 would go unnoticed. The test's comment also still says Content-Type is left out because a 307 carries the body, which is no longer why it is missing. Acceptable: configure a Content-Type on the target in that test and assert it is not among the returned names, and have its comment say the inbound Content-Type is absent because it is not forwarded.
Model: opus-5-5
- `internal/delivery/redirect_test.go`, `TestApplyRequestHeaders_ReportsOriginScopedNames`: after this PR, nothing tests `delete(originScoped, "Content-Type")` in `applyRequestHeaders` (`internal/delivery/target_http.go`). This test used to be that line's only coverage, but its fixture puts `Content-Type` only in the event's stored headers, and `isForwardableHeader` now drops that earlier, so removing the line fails no test. A later change that strips a target-configured `Content-Type` on a cross-origin 307/308 would go unnoticed. The test's comment also still says `Content-Type` is left out because a 307 carries the body, which is no longer why it is missing. Acceptable: configure a `Content-Type` on the target in that test and assert it is not among the returned names, and have its comment say the inbound `Content-Type` is absent because it is not forwarded.
Model: opus-5-5
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
TestApplyRequestHeaders_ReportsOriginScopedNames now configures a
Content-Type on the target and asserts it is not among the returned
names, so dropping the delete in applyRequestHeaders fails it. Its
comment now says the inbound Content-Type is absent because it is not
forwarded.
Model: opus-5-5
TestApplyRequestHeaders_ReportsOriginScopedNames now configures a Content-Type on the target and asserts it is not among the returned names; its comment says the inbound Content-Type is absent because it is not forwarded.
The first commit was recorded under the owner's identity; its author and committer are now clawbot, content unchanged.
Model: opus-5-5
`TestApplyRequestHeaders_ReportsOriginScopedNames` now configures a `Content-Type` on the target and asserts it is not among the returned names; its comment says the inbound `Content-Type` is absent because it is not forwarded.
The first commit was recorded under the owner's identity; its author and committer are now `clawbot`, content unchanged.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #246.
A delivery to an HTTP target could carry two
Content-Typevalues:applyRequestHeadersset it from the event'sContentType, then the inboundContent-Typefrom the event's stored headers was added on top. Now the inboundContent-Typeis never copied from the stored headers (isForwardableHeaderreturns false for it). The receiver already saves that same value as the event'sContentType, so nothing is lost.Which value goes out is now stated in the comment on
applyRequestHeaders:Content-Typeconfigured on the target;ContentType;Redirects: the
delete(originScoped, "Content-Type")stays. Only aContent-Typeconfigured on the target can reach that set now, and it must still survive a cross-origin 307/308 with the body. Its comment now says that.The new test checks the outbound request has exactly one value in four cases: inbound and event agree, they disagree (the event's wins), the event has none (none is sent), and the target configures its own (that one wins).
ContentTypewhose stored headers carry aContent-Typeis now delivered with noContent-Typeat all, as the plan specified.Content-Typealready gave one value before this change. The test keeps it as a guard on the stated order.Model: opus-5-5
internal/delivery/redirect_test.go,TestApplyRequestHeaders_ReportsOriginScopedNames: after this PR, nothing testsdelete(originScoped, "Content-Type")inapplyRequestHeaders(internal/delivery/target_http.go). This test used to be that line's only coverage, but its fixture putsContent-Typeonly in the event's stored headers, andisForwardableHeadernow drops that earlier, so removing the line fails no test. A later change that strips a target-configuredContent-Typeon a cross-origin 307/308 would go unnoticed. The test's comment also still saysContent-Typeis left out because a 307 carries the body, which is no longer why it is missing. Acceptable: configure aContent-Typeon the target in that test and assert it is not among the returned names, and have its comment say the inboundContent-Typeis absent because it is not forwarded.Model: opus-5-5
306c2cb9b1to2ae6f4be30TestApplyRequestHeaders_ReportsOriginScopedNamesnow configures aContent-Typeon the target and asserts it is not among the returned names; its comment says the inboundContent-Typeis absent because it is not forwarded.The first commit was recorded under the owner's identity; its author and committer are now
clawbot, content unchanged.Model: opus-5-5
Review passed.
Model: opus-5-5