From 32993eb95f0369e593564abb05327b549b599cbb Mon Sep 17 00:00:00 2001 From: mateea326 Date: Sun, 19 Apr 2026 13:13:31 +0300 Subject: [PATCH] fixed case sensitive origin check and update tests --- pkg/channels/pico/pico.go | 34 ++++++++++++++-------------------- pkg/channels/pico/pico_test.go | 32 +++++--------------------------- 2 files changed, 19 insertions(+), 47 deletions(-) diff --git a/pkg/channels/pico/pico.go b/pkg/channels/pico/pico.go index 38efe28ff..0940e98bc 100644 --- a/pkg/channels/pico/pico.go +++ b/pkg/channels/pico/pico.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "net/http" - "net/url" "strings" "sync" "sync/atomic" @@ -72,27 +71,22 @@ func NewPicoChannel(cfg config.PicoConfig, messageBus *bus.MessageBus) (*PicoCha base := channels.NewBaseChannel("pico", cfg, messageBus, cfg.AllowFrom) - allowOrigins := cfg.AllowOrigins - checkOrigin := func(r *http.Request) bool { - origin := r.Header.Get("Origin") - // If no origins are configured, allow same-origin only (default Gorilla behavior). - if len(allowOrigins) == 0 { - if origin == "" { - return true + // When AllowOrigins is empty, leave CheckOrigin nil so gorilla/websocket + // uses its built-in same-origin check (handles case-insensitive hostnames + // and port matching correctly). Only set a custom function when explicit + // origins are configured. + var checkOrigin func(*http.Request) bool + if len(cfg.AllowOrigins) > 0 { + allowOrigins := cfg.AllowOrigins + checkOrigin = func(r *http.Request) bool { + origin := r.Header.Get("Origin") + for _, allowed := range allowOrigins { + if allowed == "*" || allowed == origin { + return true + } } - u, err := url.Parse(origin) - if err != nil { - return false - } - return u.Host == r.Host + return false } - // If origins are configured, check for '*' or exact match. - for _, allowed := range allowOrigins { - if allowed == "*" || allowed == origin { - return true - } - } - return false } return &PicoChannel{ diff --git a/pkg/channels/pico/pico_test.go b/pkg/channels/pico/pico_test.go index ac1bee364..d121042e9 100644 --- a/pkg/channels/pico/pico_test.go +++ b/pkg/channels/pico/pico_test.go @@ -148,33 +148,11 @@ func (c *PicoChannel) addConnForTest(pc *picoConn) { func TestNewPicoChannel_UsesSameOriginCheckWithoutAllowOrigins(t *testing.T) { ch := newTestPicoChannel(t) - // If CheckOrigin is nil, the implementation is relying on gorilla/websocket's - // default same-origin enforcement. In that case we don't assert the internal - // behaviour here to avoid coupling this test to that implementation detail. - if ch.upgrader.CheckOrigin == nil { - t.Skip("CheckOrigin is nil; relying on default same-origin enforcement") - } - - if !ch.upgrader.CheckOrigin(&http.Request{ - Host: "example.com", - Header: http.Header{ - "Origin": []string{"https://example.com"}, - }, - }) { - t.Fatal("CheckOrigin rejected same-origin request") - } - - if ch.upgrader.CheckOrigin(&http.Request{ - Host: "example.com", - Header: http.Header{ - "Origin": []string{"https://other.example"}, - }, - }) { - t.Fatal("CheckOrigin accepted cross-origin request") - } - - if !ch.upgrader.CheckOrigin(&http.Request{Host: "example.com"}) { - t.Fatal("CheckOrigin rejected request without Origin header") + // When AllowOrigins is empty, CheckOrigin must be nil so gorilla/websocket + // uses its built-in same-origin enforcement (which correctly handles + // case-insensitive hostnames and port matching). + if ch.upgrader.CheckOrigin != nil { + t.Fatal("CheckOrigin should be nil when AllowOrigins is empty, to use gorilla's default same-origin check") } }