Headers duplicated when sending multiple events and using WithCustomHeader context decorator

[dan-j]

These functions enable users to customise the base set of headers to send to a CE consumer: https://github.com/cloudevents/sdk-go/blob/a9224adce0899fbb0db34f3e148029493dee95e4/v2/protocol/http/headers.go#L49-L55

Then it's used in Protocol.makeRequest(context.Context) to create our new HTTP request: https://github.com/cloudevents/sdk-go/blob/a9224adce0899fbb0db34f3e148029493dee95e4/v2/protocol/http/protocol.go#L199-L203

Then when writing the event to the request, httpRequestWriter.SetAttribute(Attribute, interface{}) appends the attribute to the request.Header: https://github.com/cloudevents/sdk-go/blob/a9224adce0899fbb0db34f3e148029493dee95e4/v2/protocol/http/write_request.go#L121

Sending multiple events with the same underlying ctx is causing these headers to be duplicated on each invocation.

Can we change newRequest to clone the headers? i.e.

req := &http.Request{
    Method: http.MethodPost,
    Header: HeaderFrom(ctx).Clone(),
}

[embano1]

Thx for flagging this @dan-j !

I briefly skimmed the code, and I'm not yet 100% following. Sending multiple events should invoke Send, i.e., makeRequest every single time (per event), thus creating a new http.Request for each call:

// Request implements binding.Requester
func (p *Protocol) Request(ctx context.Context, m binding.Message, transformers ...binding.Transformer) (binding.Message, error) {
    if ctx == nil {
        return nil, fmt.Errorf("nil Context")
    } else if m == nil {
        return nil, fmt.Errorf("nil Message")
    }

    var err error
    defer func() { _ = m.Finish(err) }()

    req := p.makeRequest(ctx)

    if p.Client == nil || req == nil || req.URL == nil {
        return nil, fmt.Errorf("not initialized: %#v", p)
    }

    if err = WriteRequest(ctx, m, req, transformers...); err != nil {
        return nil, err
    }

    return p.do(ctx, req)
}

Can you elaborate more on what "sending multiple events" means in this issue? Do we also have an issue with retrying the same event btw?

[dan-j]

Hey @embano1, yeah a new request is made, but if you look at the second snippet, is initialises Request.Header with a call to HeaderFrom(ctx). This passes the same value on each invocation. Since http.Header is just a map[string][]string, it's passing the same map each time, and modifications to that map are retained.

I believe cloning the headers each time should fix this issue. I don't mind providing a PR, just it was late Friday evening when I finally spotted the bug (it's affecting us in production) so we just made a local fix on our own codebase for now.

FYI, a temporary workaround is to override the context value with a cloned copy before calling Send(). i.e:

a.sender.Send(cehttp.WithCustomHeader(ctx, cehttp.HeaderFrom(ctx).Clone()), event)