Skip to content

Commit d6c253b

Browse files
committed
fix(ui,test): same-origin port check + robust body read
Two follow-up nits from the Copilot review on PR #579: - bulkRedirectAfter's same-origin guard compared refURL.Host (hostname:port) against c.Hostname() (hostname only). A same-origin Referer whose URL included the port — common in dev (localhost:3000) and behind some reverse proxies — was incorrectly flagged as cross- origin, falling back to the bare list path and losing filters. Fixed to refURL.Hostname() vs c.Hostname(). New subtest "same-origin with explicit port preserved" locks in the behaviour. - bulk_redirect_test.go read the response body with a single Read into a fixed 256-byte buffer. Read is allowed to return partial data; if the helper output ever grew past 256 bytes or the reader fragmented the response, the assertion would silently truncate and either pass on a partial match or false-positive on a trailing diff. Replaced with io.ReadAll. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
1 parent 4d1a230 commit d6c253b

2 files changed

Lines changed: 26 additions & 6 deletions

File tree

internal/web/bulk_handlers.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,12 @@ func bulkRedirectAfter(c *fiber.Ctx, fallbackList string, dropPanel bool) string
5252

5353
// Reject cross-origin Referers. Allow relative Referers (empty Host),
5454
// which some clients still emit for same-origin POSTs.
55-
if refURL.Host != "" && refURL.Host != c.Hostname() {
55+
//
56+
// Compare hostnames via url.URL.Hostname() (strips any port) so dev
57+
// and proxy deployments on non-default ports (e.g. localhost:3000)
58+
// are not incorrectly flagged as cross-origin against Fiber's
59+
// port-less c.Hostname().
60+
if refURL.Host != "" && refURL.Hostname() != c.Hostname() {
5661
return fallbackList
5762
}
5863

internal/web/bulk_redirect_test.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package web
33

44
import (
5+
"io"
56
"net/http/httptest"
67
"testing"
78

@@ -70,6 +71,20 @@ func TestBulkRedirectAfter(t *testing.T) {
7071
dropPanel: false,
7172
want: "/users",
7273
},
74+
{
75+
// httptest.NewRequest defaults Host to "example.com"; the
76+
// absolute referer has the same hostname but an arbitrary
77+
// port. Fiber's c.Hostname() is port-less, so we must
78+
// compare against refURL.Hostname(), not refURL.Host — the
79+
// latter would treat "example.com:3000" as a different
80+
// origin and discard filters. This guards bug-fixed in the
81+
// post-review follow-up.
82+
name: "same-origin with explicit port preserved",
83+
referer: "http://example.com:3000/users?ou=Eng",
84+
fallbackList: "/users",
85+
dropPanel: false,
86+
want: "/users?ou=Eng",
87+
},
7388
{
7489
name: "unparseable referer falls back",
7590
referer: "://not-a-url",
@@ -111,11 +126,11 @@ func TestBulkRedirectAfter(t *testing.T) {
111126
}
112127
defer func() { _ = resp.Body.Close() }()
113128

114-
body := make([]byte, 256)
115-
n, _ := resp.Body.Read(body)
116-
got := string(body[:n])
117-
118-
if got != tc.want {
129+
body, err := io.ReadAll(resp.Body)
130+
if err != nil {
131+
t.Fatalf("read body: %v", err)
132+
}
133+
if got := string(body); got != tc.want {
119134
t.Errorf("got %q, want %q", got, tc.want)
120135
}
121136
})

0 commit comments

Comments
 (0)