From 85a31867030c68f0c9cf12402aec4c2ccb05c7e1 Mon Sep 17 00:00:00 2001 From: zarazaex69 Date: Fri, 15 May 2026 15:45:07 +0300 Subject: [PATCH] refactor(jitsi): extract helpers and simplify tests --- internal/auth/jitsi/jitsi.go | 6 +-- internal/auth/jitsi/jitsi_test.go | 19 +++++--- internal/e2e/tunnel_test.go | 1 + internal/engine/jitsi/jitsi.go | 73 +++++++++++++++++------------ internal/engine/jitsi/jitsi_test.go | 49 ++++++++++--------- 5 files changed, 88 insertions(+), 60 deletions(-) diff --git a/internal/auth/jitsi/jitsi.go b/internal/auth/jitsi/jitsi.go index 1584c97..6415552 100644 --- a/internal/auth/jitsi/jitsi.go +++ b/internal/auth/jitsi/jitsi.go @@ -67,7 +67,7 @@ func (Provider) Issue(_ context.Context, cfg auth.Config) (auth.Credentials, err // Accepts URLs with or without scheme. The host part is the segment before // the first "/" after stripping the scheme; the room is everything that // follows, with leading/trailing slashes trimmed. -func parseRoomURL(raw string) (host string, room string, err error) { +func parseRoomURL(raw string) (string, string, error) { raw = strings.TrimSpace(raw) if raw == "" { return "", "", auth.ErrRoomIDRequired @@ -81,8 +81,8 @@ func parseRoomURL(raw string) (host string, room string, err error) { if slash <= 0 { return "", "", fmt.Errorf("%w: %q", ErrInvalidRoomURL, raw) } - host = strings.TrimSpace(raw[:slash]) - room = strings.Trim(raw[slash+1:], "/") + host := strings.TrimSpace(raw[:slash]) + room := strings.Trim(raw[slash+1:], "/") if host == "" || room == "" { return "", "", fmt.Errorf("%w: %q", ErrInvalidRoomURL, raw) } diff --git a/internal/auth/jitsi/jitsi_test.go b/internal/auth/jitsi/jitsi_test.go index 64d9120..2f25aee 100644 --- a/internal/auth/jitsi/jitsi_test.go +++ b/internal/auth/jitsi/jitsi_test.go @@ -8,6 +8,11 @@ import ( "github.com/openlibrecommunity/olcrtc/internal/auth" ) +const ( + testRoom = "myroom" + testHost = "meet.example" +) + func TestParseRoomURL(t *testing.T) { tests := []struct { name string @@ -16,15 +21,15 @@ func TestParseRoomURL(t *testing.T) { room string wantErr bool }{ - {name: "https url", raw: "https://meet.cryptopro.ru/myroom", host: "meet.cryptopro.ru", room: "myroom"}, - {name: "http url", raw: "http://meet.example/myroom", host: "meet.example", room: "myroom"}, - {name: "scheme-less", raw: "meet.example.com/myroom", host: "meet.example.com", room: "myroom"}, - {name: "trailing slash", raw: "https://meet.example/myroom/", host: "meet.example", room: "myroom"}, - {name: "double slash leader", raw: "//meet.example/myroom", host: "meet.example", room: "myroom"}, - {name: "uppercase room", raw: "https://meet.example/MyRoom", host: "meet.example", room: "MyRoom"}, + {name: "https url", raw: "https://meet.cryptopro.ru/" + testRoom, host: "meet.cryptopro.ru", room: testRoom}, + {name: "http url", raw: "http://" + testHost + "/" + testRoom, host: testHost, room: testRoom}, + {name: "scheme-less", raw: "meet.example.com/" + testRoom, host: "meet.example.com", room: testRoom}, + {name: "trailing slash", raw: "https://" + testHost + "/" + testRoom + "/", host: testHost, room: testRoom}, + {name: "double slash leader", raw: "//" + testHost + "/" + testRoom, host: testHost, room: testRoom}, + {name: "uppercase room", raw: "https://" + testHost + "/MyRoom", host: testHost, room: "MyRoom"}, {name: "empty", raw: "", wantErr: true}, {name: "host only", raw: "meet.example.com", wantErr: true}, - {name: "no room", raw: "https://meet.example/", wantErr: true}, + {name: "no room", raw: "https://" + testHost + "/", wantErr: true}, {name: "scheme only", raw: "https://", wantErr: true}, } for _, tc := range tests { diff --git a/internal/e2e/tunnel_test.go b/internal/e2e/tunnel_test.go index 2d2ed89..6fad26b 100644 --- a/internal/e2e/tunnel_test.go +++ b/internal/e2e/tunnel_test.go @@ -349,6 +349,7 @@ func builtInTransportNames() []string { return []string{transportData, transportVideo, transportSEI, transportVP8} } +//nolint:cyclop // matrix of carrier×transport expectations is naturally branchy func realE2ECaseExpectation(carrierName, transportName string) realE2EExpectation { switch carrierName { case "telemost": diff --git a/internal/engine/jitsi/jitsi.go b/internal/engine/jitsi/jitsi.go index 4000e7c..f83288d 100644 --- a/internal/engine/jitsi/jitsi.go +++ b/internal/engine/jitsi/jitsi.go @@ -138,30 +138,35 @@ func sanitiseNick(raw string) string { b.Grow(len(raw)) prevDash := false for _, r := range raw { - switch { - case r >= 'a' && r <= 'z', - r >= 'A' && r <= 'Z', - r >= '0' && r <= '9': - b.WriteRune(r) - prevDash = false - case r == '-' || r == '_': - b.WriteRune(r) - prevDash = false - default: - if !prevDash && b.Len() > 0 { - b.WriteRune('-') - prevDash = true - } - } if b.Len() >= maxNickLen { break } + if isNickRune(r) { + b.WriteRune(r) + prevDash = false + continue + } + if !prevDash && b.Len() > 0 { + b.WriteRune('-') + prevDash = true + } } - out := strings.Trim(b.String(), "-") - if out == "" { - return "" + return strings.Trim(b.String(), "-") +} + +// isNickRune reports whether r is allowed verbatim in a sanitised nick. +func isNickRune(r rune) bool { + switch { + case r >= 'a' && r <= 'z': + return true + case r >= 'A' && r <= 'Z': + return true + case r >= '0' && r <= '9': + return true + case r == '-' || r == '_': + return true } - return out + return false } // Capabilities reports what this engine can do. @@ -351,21 +356,31 @@ func (s *Session) recvLoop() { case <-s.done: return case msg, ok := <-msgs: - if !ok { - if !s.closed.Load() { - s.signalEnded("jitsi bridge closed") - } + if !s.deliverBridgeMessage(msg, ok) { return } - payload := decodeRaw(msg) - if payload == nil { - continue - } - s.onData(payload) } } } +// deliverBridgeMessage decodes a single incoming bridge message and forwards +// any raw payload to onData. Returns false to signal that the recv loop +// should exit (channel closed or session ended). +func (s *Session) deliverBridgeMessage(msg j.BridgeMessage, ok bool) bool { + if !ok { + if !s.closed.Load() { + s.signalEnded("jitsi bridge closed") + } + return false + } + payload := decodeRaw(msg) + if payload == nil { + return true + } + s.onData(payload) + return true +} + // decodeRaw extracts the bytes from an EndpointMessage produced by the j // library's BridgeSendRaw helper. Mirrors the unexported colibri.DecodeRaw — // the j library's BridgeMessage type alias keeps the necessary fields public, @@ -480,7 +495,7 @@ func (s *Session) GetBufferedAmount() uint64 { if depth <= 0 { return 0 } - return uint64(depth) * uint64(bridgeMaxMessageSize) //nolint:gosec // depth is small and bounded by queue cap + return uint64(depth) * uint64(bridgeMaxMessageSize) } // AddVideoTrack publishes a video track to the Jitsi conference. diff --git a/internal/engine/jitsi/jitsi_test.go b/internal/engine/jitsi/jitsi_test.go index 80ab1c4..7fb1ff1 100644 --- a/internal/engine/jitsi/jitsi_test.go +++ b/internal/engine/jitsi/jitsi_test.go @@ -8,17 +8,24 @@ import ( "github.com/openlibrecommunity/olcrtc/internal/engine" ) +const ( + testHost = "meet.example.com" + testRoom = "myroom" + rawFieldKey = "raw" + classEndpoint = "EndpointMessage" +) + func TestNormaliseHost(t *testing.T) { tests := []struct { raw string want string }{ - {"meet.example.com", "meet.example.com"}, - {"https://meet.example.com", "meet.example.com"}, - {"https://meet.example.com/", "meet.example.com"}, - {"https://meet.example.com/path", "meet.example.com"}, - {"//meet.example.com", "meet.example.com"}, - {" https://meet.example.com ", "meet.example.com"}, + {testHost, testHost}, + {"https://" + testHost, testHost}, + {"https://" + testHost + "/", testHost}, + {"https://" + testHost + "/path", testHost}, + {"//" + testHost, testHost}, + {" https://" + testHost + " ", testHost}, {"", ""}, } for _, tc := range tests { @@ -32,27 +39,27 @@ func TestNormaliseHost(t *testing.T) { func TestDecodeRaw(t *testing.T) { const payload = "hello world" - raw := encodeForTest(t, []byte(payload)) + encoded := encodeForTest(t, []byte(payload)) - got := decodeRaw(makeBridgeMessage("EndpointMessage", map[string]any{"raw": raw})) + got := decodeRaw(makeBridgeMessage(classEndpoint, map[string]any{rawFieldKey: encoded})) if string(got) != payload { t.Fatalf("decodeRaw = %q, want %q", got, payload) } - if got := decodeRaw(makeBridgeMessage("OtherClass", map[string]any{"raw": raw})); got != nil { + if got := decodeRaw(makeBridgeMessage("OtherClass", map[string]any{rawFieldKey: encoded})); got != nil { t.Fatalf("decodeRaw(other class) = %q, want nil", got) } - if got := decodeRaw(makeBridgeMessage("EndpointMessage", map[string]any{})); got != nil { + if got := decodeRaw(makeBridgeMessage(classEndpoint, map[string]any{})); got != nil { t.Fatalf("decodeRaw(no raw) = %q, want nil", got) } - if got := decodeRaw(makeBridgeMessage("EndpointMessage", map[string]any{"raw": "not-base64!!!"})); got != nil { + if got := decodeRaw(makeBridgeMessage(classEndpoint, map[string]any{rawFieldKey: "not-base64!!!"})); got != nil { t.Fatalf("decodeRaw(bad base64) = %q, want nil", got) } } func TestNewRequiresHost(t *testing.T) { _, err := New(context.Background(), engine.Config{ - Extra: map[string]string{"room": "myroom"}, + Extra: map[string]string{credentialKeyRoom: testRoom}, }) if !errors.Is(err, ErrHostRequired) { t.Fatalf("err = %v, want ErrHostRequired", err) @@ -61,7 +68,7 @@ func TestNewRequiresHost(t *testing.T) { func TestNewRequiresRoom(t *testing.T) { _, err := New(context.Background(), engine.Config{ - URL: "meet.example.com", + URL: testHost, }) if !errors.Is(err, ErrRoomRequired) { t.Fatalf("err = %v, want ErrRoomRequired", err) @@ -70,8 +77,8 @@ func TestNewRequiresRoom(t *testing.T) { func TestNewSucceeds(t *testing.T) { sess, err := New(context.Background(), engine.Config{ - URL: "https://meet.example.com", - Extra: map[string]string{"room": "myroom"}, + URL: "https://" + testHost, + Extra: map[string]string{credentialKeyRoom: testRoom}, Name: "olcrtc-test", }) if err != nil { @@ -86,8 +93,8 @@ func TestNewSucceeds(t *testing.T) { func TestSendBeforeConnect(t *testing.T) { sess, err := New(context.Background(), engine.Config{ - URL: "meet.example.com", - Extra: map[string]string{"room": "myroom"}, + URL: testHost, + Extra: map[string]string{credentialKeyRoom: testRoom}, OnData: func([]byte) {}, }) if err != nil { @@ -101,8 +108,8 @@ func TestSendBeforeConnect(t *testing.T) { func TestSendAfterClose(t *testing.T) { sess, err := New(context.Background(), engine.Config{ - URL: "meet.example.com", - Extra: map[string]string{"room": "myroom"}, + URL: testHost, + Extra: map[string]string{credentialKeyRoom: testRoom}, }) if err != nil { t.Fatalf("New: %v", err) @@ -139,8 +146,8 @@ func TestSanitiseNick(t *testing.T) { func TestEngineRegistration(t *testing.T) { if _, err := engine.New(context.Background(), "jitsi", engine.Config{ - URL: "meet.example.com", - Extra: map[string]string{"room": "myroom"}, + URL: testHost, + Extra: map[string]string{credentialKeyRoom: testRoom}, }); err != nil { t.Fatalf("engine.New(jitsi) = %v, want nil", err) }