[2.4.0] Access-Control-Expose-Headers is overridden, breaking browser Swarm apps

[Cafe137]

Context

2.4.0

Summary

Access-Control-Expose-Headers is overridden, impairing browser functionality

Actual behavior

This is the line that causes the problem.

https://github.com/ethersphere/bee/blob/master/pkg/api/bzz.go#L307

Take the /feeds/:owner/:topic endpoint for example. additionalHeaders is set there to allow browsers to read swarm-feed-index, swarm-feed-index-next. It seems that the line in bzz.go discards the previously set headers.

Edit: It seems there is a second issue. The headers are joined with ;, but it should be ,.

[Cafe137]

These are my local fixes which I can confirm work, but obviously bad quality code 🙂

diff --git a/pkg/api/bzz.go b/pkg/api/bzz.go
index 14a236c1..704cf636 100644
--- a/pkg/api/bzz.go
+++ b/pkg/api/bzz.go
@@ -616,13 +616,18 @@ func (s *Service) downloadHandler(logger log.Logger, w http.ResponseWriter, r *h
 
        // include additional headers
        for name, values := range additionalHeaders {
-               w.Header().Set(name, strings.Join(values, "; "))
+               w.Header().Set(name, strings.Join(values, ", "))
        }
        if etag {
                w.Header().Set(ETagHeader, fmt.Sprintf("%q", reference))
        }
        w.Header().Set(ContentLengthHeader, strconv.FormatInt(l, 10))
-       w.Header().Set("Access-Control-Expose-Headers", ContentDispositionHeader)
+       if w.Header().Get("Access-Control-Expose-Headers") != "" {
+               w.Header().Set("Access-Control-Expose-Headers", w.Header().Get("Access-Control-Expose-Headers") + ", " + ContentDispositionHeader)
+       } else {
+               w.Header().Set("Access-Control-Expose-Headers", ContentDispositionHeader)
+       }
+
 
        if headersOnly {
                w.WriteHeader(http.StatusOK)

[gacevicljubisa]

After reviewing the problem and discussing it, here are some thoughts

1. Regarding the Delimiter (; vs ,)

While it's true that ; is valid in specific scenarios (e.g., Content-Type: text/html; charset=ISO-8859-4, per RFC9110), Access-Control-Expose-Headers explicitly requires comma-separated values as per MDN's documentation.

In our case, it seems ; is being used incorrectly, leading to browser behavior issues, especially in Chromium-based browsers (e.g., Brave). Changing the delimiter to , resolved the issue in those cases. This aligns with the StackOverflow discussion, which highlights that commas are used to separate distinct header values, whereas semicolons are typically used for parameters within a single value.

2. Header Overwriting Issue

We identified that the issue stems from headers being overwritten instead of appended. The suggested fix using w.Header().Add seems correct and should prevent any overwriting while properly joining multiple header values with a ,.

Here’s the relevant snippet to address this:

// include additional headers
for name, values := range additionalHeaders {
    for _, value := range values {
        w.Header().Add(name, value)
    }
}
if etag {
    w.Header().Set(ETagHeader, fmt.Sprintf("%q", reference))
}
w.Header().Set(ContentLengthHeader, strconv.FormatInt(l, 10))
w.Header().Add("Access-Control-Expose-Headers", ContentDispositionHeader)

3. Why This Matters

The confusion seems to arise because semicolons (;) are valid for specific header parameters, but Access-Control-Expose-Headers uses commas (,) to distinguish separate values. Semicolons here can lead to misinterpretation by browsers.

TL;DR:

• ; is for parameters within one value (e.g., charset=ISO-8859-4).
• , is for distinct values (e.g., multiple exposed headers).

4. Note

The Access-Control-Expose-Headers is set in a few places in the code. Some comments regarding this can be found here:

    // this header might be overriding others. handle with care. in the future
    // we should implement an append functionality for this specific header,
    // since different parts of handlers might be overriding others' values
    // resulting in inconsistent headers in the response.
    w.Header().Set("Access-Control-Expose-Headers", SwarmFeedIndexHeader)

5. Solution

Use w.Header().Add to append headers instead of overwriting them.

Any opinions @Cafe137, @istae ?

[Cafe137]

I tried the branch with Etherjot, and the feed related headers can be now accessed, fixing the previous issue.

[gacevicljubisa]

I tried the branch with Etherjot, and the feed related headers can be now accessed, fixing the previous issue.

Thank you for the feadback.