Fix dashboard buttons and add video support - #4
Open
dnicolson wants to merge 6 commits into
Open
Conversation
Co-Authored-By: codex <codex@openai.com>
https://developers.google.com/cast/docs/media/messages Co-Authored-By: codex <codex@openai.com>
Co-Authored-By: codex <codex@openai.com>
Co-Authored-By: codex <codex@openai.com>
Co-Authored-By: codex <codex@openai.com>
Co-Authored-By: codex <codex@openai.com>
Reviewer's GuideThis PR refactors TLS handling and media session management to add robust video playback support, improve Chromecast protocol compatibility, and enhance the web dashboard with a same-origin HTML5 video player and richer status/control APIs. Sequence diagram for TLS probing and legacy TLS 1.2 fallbacksequenceDiagram
participant Client
participant Server as GoCastServer
participant HR as handleRawConnection
participant Legacy as legacyTLS12Conn
participant TLS as tls.Server
participant Cast as cast.HandleConnection
Client->>Server: TCP connect
Server->>HR: handleRawConnection(conn, tlsCfg, cert, receiver)
HR->>HR: probeConnectionStart(conn, br)
alt [ClientHello has trailing-dot SNI and legacyTLS12FallbackAvailable]
HR->>Legacy: newLegacyTLS12ServerConn(bufferedConn, br, cert, hello)
Note right of Legacy: handshake()
HR->>Cast: cast.HandleConnection(Legacy, receiver)
else [default TLS path]
HR->>TLS: tls.Server(bufferedConn, cfg)
TLS-->>HR: tlsConn
HR->>Cast: cast.HandleConnection(tlsConn, receiver)
end
Sequence diagram for dashboard HTML5 video playback and media proxysequenceDiagram
participant Browser
participant Dash as Dashboard/API
participant Rec as Receiver
participant Stream as streamHandler
participant Proxy as ProxyMedia
loop poll status
Browser->>Dash: GET /api/status
Dash->>Rec: read MediaSession
Rec-->>Dash: playerState, contentId, mediaSessionId, streamReady
Dash-->>Browser: JSON status
end
alt [status.streamReady]
Browser->>Stream: GET /stream?session=...
Stream->>Rec: read Media.CachePath & Media.Media
alt [cacheComplete]
Stream->>Proxy: ServeCachedMedia(w, r, cachePath, contentType)
else [no cache file]
Stream->>Proxy: ProxyMedia(w, r, source)
end
Proxy-->>Browser: media bytes (same-origin)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The TLS client hello probing and legacy TLS 1.2 fallback logic added to main.go and legacy_tls12.go is quite large and intertwined; consider extracting this into a dedicated package or subsystem to keep main.go focused on startup wiring and to make the handshake path easier to reason about and maintain.
- In handleRawConnection, you clone tlsCfg into cfg but GetConfigForClient returns tlsCfg instead of cfg; if the intent is to customize per-client configuration (and use the modified MinVersion etc.), consider returning cfg from GetConfigForClient to avoid subtle differences between the initial and per-client configs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The TLS client hello probing and legacy TLS 1.2 fallback logic added to main.go and legacy_tls12.go is quite large and intertwined; consider extracting this into a dedicated package or subsystem to keep main.go focused on startup wiring and to make the handshake path easier to reason about and maintain.
- In handleRawConnection, you clone tlsCfg into cfg but GetConfigForClient returns tlsCfg instead of cfg; if the intent is to customize per-client configuration (and use the modified MinVersion etc.), consider returning cfg from GetConfigForClient to avoid subtle differences between the initial and per-client configs.
## Individual Comments
### Comment 1
<location path="cast/receiver.go" line_range="74-76" />
<code_context>
return &Receiver{
sessions: make(map[string]*Session),
Media: &MediaSession{
- PlayerState: "IDLE",
+ PlayerState: "IDLE",
+ PlaybackRate: 1,
+ Volume: Volume{Level: 1, Muted: false},
},
</code_context>
<issue_to_address>
**issue (bug_risk):** Initial PlaybackRate is set to 1 while PlayerState is IDLE, and dashboard controls don’t keep PlaybackRate in sync with state.
NewReceiver sets Media.PlaybackRate to 1 while PlayerState is "IDLE", but playbackRateForState returns 0 for non-playing states. The dashboard controlHandler mutates PlayerState without updating PlaybackRate, so PLAY/PAUSE/STOP actions can leave PlaybackRate inconsistent with PlayerState. Please initialize PlaybackRate to 0 for IDLE and update it via playbackRateForState whenever PlayerState changes (including in dashboard.go) to keep media status consistent.
</issue_to_address>
### Comment 2
<location path="cast/httpproxy.go" line_range="71-63" />
<code_context>
+ http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+ return
+ }
+ if !hasScheme(source) {
+ if _, err := os.Stat(source); err != nil {
+ http.Error(w, "media file unavailable", http.StatusNotFound)
+ return
+ }
+ writeProxyCORS(w)
+ http.ServeFile(w, r, source)
+ return
</code_context>
<issue_to_address>
**🚨 issue (security):** Serving arbitrary local file paths via ProxyMedia can expose unexpected files or paths to the dashboard origin.
Because any source without a scheme is treated as a local path and passed directly to http.ServeFile if os.Stat succeeds, a sender-controlled contentId like "../../somefile" could trigger path traversal and expose arbitrary local files. Please constrain local file access to a specific base directory (or known cache location), normalize and validate paths, and reject requests that resolve outside that directory to prevent unintended file exposure.
</issue_to_address>
### Comment 3
<location path="cast/httpproxy_test.go" line_range="11-15" />
<code_context>
+ "testing"
+)
+
+func TestServeCachedMediaSupportsRanges(t *testing.T) {
+ f, err := os.CreateTemp(t.TempDir(), "media-*")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := f.WriteString("abcdef"); err != nil {
+ t.Fatal(err)
</code_context>
<issue_to_address>
**suggestion (testing):** ProxyMedia behavior (local file vs remote URL, method handling, OPTIONS/CORS) is not currently exercised
ServeCachedMedia, hasScheme, and normalizeHTTPMediaURL are covered, but ProxyMedia’s key behaviors remain untested: 404 when `source == ""`, OPTIONS/CORS with 204, rejection of non-GET/HEAD methods, serving local files, and proxying remote URLs with CORS headers. Please add httptest-based tests for at least the OPTIONS path, a local file source, and a simple HTTP upstream (e.g., via httptest.Server) to validate the new streaming behavior end-to-end.
Suggested implementation:
```golang
package cast
import (
"io"
"net/http"
"net/http/httptest"
"net/url"
"os"
"testing"
)
func TestServeCachedMediaSupportsRanges(t *testing.T) {
f, err := os.CreateTemp(t.TempDir(), "media-*")
if err != nil {
t.Fatal(err)
}
if _, err := f.WriteString("abcdef"); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
req := httptest.NewRequest(http.MethodGet, "/stream", nil)
req.Header.Set("Range", "bytes=2-4")
rr := httptest.NewRecorder()
ServeCachedMedia(rr, req, f.Name())
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusPartialContent {
t.Fatalf("expected status %d, got %d", http.StatusPartialContent, res.StatusCode)
}
got, err := io.ReadAll(res.Body)
if err != nil {
t.Fatal(err)
}
if string(got) != "cde" {
t.Fatalf("expected body %q, got %q", "cde", string(got))
}
}
func TestProxyMediaEmptySourceReturnsNotFound(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/proxy", nil)
rr := httptest.NewRecorder()
ProxyMedia(rr, req, "")
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusNotFound {
t.Fatalf("expected status %d for empty source, got %d", http.StatusNotFound, res.StatusCode)
}
}
func TestProxyMediaRejectsNonGETOrHEAD(t *testing.T) {
for _, method := range []string{http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch} {
t.Run(method, func(t *testing.T) {
req := httptest.NewRequest(method, "/proxy", nil)
rr := httptest.NewRecorder()
ProxyMedia(rr, req, "/does/not/matter")
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusMethodNotAllowed {
t.Fatalf("expected status %d for method %s, got %d", http.StatusMethodNotAllowed, method, res.StatusCode)
}
})
}
}
func TestProxyMediaOptionsReturnsNoContentWithCORS(t *testing.T) {
req := httptest.NewRequest(http.MethodOptions, "/proxy", nil)
req.Header.Set("Origin", "http://example.com")
rr := httptest.NewRecorder()
ProxyMedia(rr, req, "/ignored-for-options")
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusNoContent {
t.Fatalf("expected status %d for OPTIONS, got %d", http.StatusNoContent, res.StatusCode)
}
allowOrigin := res.Header.Get("Access-Control-Allow-Origin")
if allowOrigin == "" {
t.Fatalf("expected Access-Control-Allow-Origin header to be set")
}
if res.Header.Get("Access-Control-Allow-Methods") == "" {
t.Fatalf("expected Access-Control-Allow-Methods header to be set")
}
}
func TestProxyMediaServesLocalFile(t *testing.T) {
f, err := os.CreateTemp(t.TempDir(), "proxy-local-*")
if err != nil {
t.Fatal(err)
}
const content = "local-file-content"
if _, err := f.WriteString(content); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
req := httptest.NewRequest(http.MethodGet, "/proxy", nil)
rr := httptest.NewRecorder()
ProxyMedia(rr, req, f.Name())
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
t.Fatalf("expected status %d for local file, got %d", http.StatusOK, res.StatusCode)
}
body, err := io.ReadAll(res.Body)
if err != nil {
t.Fatal(err)
}
if string(body) != content {
t.Fatalf("expected body %q, got %q", content, string(body))
}
if res.Header.Get("Access-Control-Allow-Origin") == "" {
t.Fatalf("expected CORS headers on local file response")
}
}
func TestProxyMediaProxiesRemoteURLWithCORS(t *testing.T) {
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/plain")
_, _ = io.WriteString(w, "upstream-body")
}))
defer upstream.Close()
u, err := url.Parse(upstream.URL)
if err != nil {
t.Fatal(err)
}
req := httptest.NewRequest(http.MethodGet, "/proxy", nil)
req.Header.Set("Origin", "http://example-client")
rr := httptest.NewRecorder()
ProxyMedia(rr, req, u.String())
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
t.Fatalf("expected status %d for remote URL, got %d", http.StatusOK, res.StatusCode)
}
body, err := io.ReadAll(res.Body)
if err != nil {
t.Fatal(err)
}
if string(body) != "upstream-body" {
t.Fatalf("expected proxied body %q, got %q", "upstream-body", string(body))
}
if res.Header.Get("Access-Control-Allow-Origin") == "" {
t.Fatalf("expected CORS headers on proxied response")
}
}
```
The above tests assume the following signatures and behaviors, which you may need to align with your existing implementation:
1. `ServeCachedMedia(w http.ResponseWriter, r *http.Request, path string)`:
- Supports HTTP Range requests on a local file at `path`, returning `StatusPartialContent` and the requested byte range. If the API differs, adjust the call and assertions accordingly.
2. `ProxyMedia(w http.ResponseWriter, r *http.Request, source string)`:
- Returns `StatusNotFound` when `source == ""`.
- Returns `StatusMethodNotAllowed` for non-GET/HEAD methods.
- Handles `OPTIONS` requests with `StatusNoContent` and appropriate CORS headers (e.g., `Access-Control-Allow-Origin`, `Access-Control-Allow-Methods`).
- When `source` is a local file path, streams its contents with `StatusOK` and applies CORS headers.
- When `source` is an HTTP/HTTPS URL, proxies the upstream response body with `StatusOK` and applies CORS headers.
If your CORS implementation uses specific values (such as `"*"` for `Access-Control-Allow-Origin` or a fixed method list), update the header assertions to match those values. Also ensure the import list matches your project conventions (remove `io` or `net/url` if they are unused after aligning with the real functions).
</issue_to_address>
### Comment 4
<location path="cast/server_test.go" line_range="5-8" />
<code_context>
+
+import "testing"
+
+func TestSourceIDForNamespaceUsesAppTransportForMedia(t *testing.T) {
+ if got := sourceIDForNamespace(nsMedia); got != defaultTransport {
+ t.Fatalf("sourceIDForNamespace(nsMedia) = %q, want %q", got, defaultTransport)
+ }
+}
+
</code_context>
<issue_to_address>
**suggestion (testing):** Session send/sendRaw behavior could be tested to confirm source/destination IDs and logging changes
These tests cover sourceIDForNamespace, but the higher-level Session APIs (send, sendRaw, rememberSender, senderDestination) remain untested. Given the routing changes, please also add tests that:
- Create a Session with a fake connection and assert send() routes via defaultTransport for nsMedia and defaultReceiverID for other namespaces.
- Exercise rememberSender/senderDestination and confirm sendRaw targets the remembered sender rather than the receiver.
This will verify the new Cast routing behavior end-to-end.
Suggested implementation:
```golang
package cast
import "testing"
func TestSourceIDForNamespaceUsesAppTransportForMedia(t *testing.T) {
if got := sourceIDForNamespace(nsMedia); got != defaultTransport {
t.Fatalf("sourceIDForNamespace(nsMedia) = %q, want %q", got, defaultTransport)
}
}
// fakeConn is a test double for the underlying connection used by Session.
// It records the last message sent so tests can inspect source/destination IDs.
type fakeConn struct {
lastNamespace string
lastSourceID string
lastDestination string
lastPayload []byte
}
func (c *fakeConn) Send(namespace, sourceID, destination string, payload []byte) error {
c.lastNamespace = namespace
c.lastSourceID = sourceID
c.lastDestination = destination
c.lastPayload = payload
return nil
}
func TestSessionSendRoutesMediaViaDefaultTransport(t *testing.T) {
conn := &fakeConn{}
// NOTE: Adjust this constructor call to match the actual Session initializer.
// For example, if it is NewSession(conn) or NewSessionWithDefaults(conn, defaultReceiverID, defaultTransport).
s := &Session{
conn: conn,
defaultReceiver: defaultReceiverID,
defaultTransport: defaultTransport,
}
payload := []byte(`{"type":"MEDIA_STATUS"}`)
// Media namespace should use defaultTransport as source and defaultReceiverID as destination.
if err := s.send(nsMedia, payload); err != nil {
t.Fatalf("send(nsMedia) error = %v, want nil", err)
}
if conn.lastNamespace != nsMedia {
t.Fatalf("send(nsMedia) namespace = %q, want %q", conn.lastNamespace, nsMedia)
}
if conn.lastSourceID != defaultTransport {
t.Fatalf("send(nsMedia) sourceID = %q, want %q", conn.lastSourceID, defaultTransport)
}
if conn.lastDestination != defaultReceiverID {
t.Fatalf("send(nsMedia) destination = %q, want %q", conn.lastDestination, defaultReceiverID)
}
// Non-media namespaces should use the receiver as both source and destination.
if err := s.send(nsReceiver, payload); err != nil {
t.Fatalf("send(nsReceiver) error = %v, want nil", err)
}
if conn.lastNamespace != nsReceiver {
t.Fatalf("send(nsReceiver) namespace = %q, want %q", conn.lastNamespace, nsReceiver)
}
if conn.lastSourceID != defaultReceiverID {
t.Fatalf("send(nsReceiver) sourceID = %q, want %q", conn.lastSourceID, defaultReceiverID)
}
if conn.lastDestination != defaultReceiverID {
t.Fatalf("send(nsReceiver) destination = %q, want %q", conn.lastDestination, defaultReceiverID)
}
}
func TestSessionSendRawUsesRememberedSenderDestination(t *testing.T) {
conn := &fakeConn{}
// NOTE: Adjust this constructor call to match the actual Session initializer.
s := &Session{
conn: conn,
defaultReceiver: defaultReceiverID,
defaultTransport: defaultTransport,
}
// Remember a sender (e.g., an app transport) as the destination for subsequent sendRaw calls.
appSenderID := "sender-123"
s.rememberSender(appSenderID)
payload := []byte(`{"type":"CUSTOM"}`)
// sendRaw should route to the remembered sender, not the receiver.
if err := s.sendRaw(nsMedia, payload); err != nil {
t.Fatalf("sendRaw(nsMedia) error = %v, want nil", err)
}
if conn.lastNamespace != nsMedia {
t.Fatalf("sendRaw(nsMedia) namespace = %q, want %q", conn.lastNamespace, nsMedia)
}
// Source should typically remain the transport for media.
if conn.lastSourceID != defaultTransport {
t.Fatalf("sendRaw(nsMedia) sourceID = %q, want %q", conn.lastSourceID, defaultTransport)
}
// Destination should be the remembered sender, not the receiver.
if conn.lastDestination != appSenderID {
t.Fatalf("sendRaw(nsMedia) destination = %q, want %q", conn.lastDestination, appSenderID)
}
// Verify senderDestination accessor reflects the remembered sender.
if got := s.senderDestination(); got != appSenderID {
t.Fatalf("senderDestination() = %q, want %q", got, appSenderID)
}
}
func TestSourceIDForNamespaceUsesReceiverForReceiverNamespaces(t *testing.T) {
for _, ns := range []string{nsReceiver, nsHeartbeat, nsConnection} {
```
1. Align the `fakeConn` test double with the actual connection interface/struct used in `Session`. If the `Session` writes protocol messages via a different method signature (e.g. `Send(*Message)`), adjust `fakeConn` accordingly and update the tests to capture `sourceID`/`destination` from the real message type.
2. Replace the `Session` construction in the tests with the real initializer. For example, if there is a `NewSession(conn)` function or fields are unexported, use the appropriate constructor and remove direct field assignment.
3. Ensure the `Session.send`, `Session.sendRaw`, `Session.rememberSender`, and `Session.senderDestination` signatures match the calls in the tests. If their parameter lists differ (e.g. they accept a context, message type, or additional flags), update the tests to pass the required arguments.
4. If routing logic around `defaultTransport`, `defaultReceiverID`, and media namespaces behaves slightly differently (e.g. additional namespaces, different defaults), adjust the test expectations (`conn.lastSourceID` and `conn.lastDestination` checks) to match the actual routing rules.
5. If additional imports (such as `context` or other packages) are needed for the real `Session` constructor or message type, add them to the import block and ensure there is still only one consolidated `import` section.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
74
to
+76
| Media: &MediaSession{ | ||
| PlayerState: "IDLE", | ||
| PlayerState: "IDLE", | ||
| PlaybackRate: 1, |
There was a problem hiding this comment.
issue (bug_risk): Initial PlaybackRate is set to 1 while PlayerState is IDLE, and dashboard controls don’t keep PlaybackRate in sync with state.
NewReceiver sets Media.PlaybackRate to 1 while PlayerState is "IDLE", but playbackRateForState returns 0 for non-playing states. The dashboard controlHandler mutates PlayerState without updating PlaybackRate, so PLAY/PAUSE/STOP actions can leave PlaybackRate inconsistent with PlayerState. Please initialize PlaybackRate to 0 for IDLE and update it via playbackRateForState whenever PlayerState changes (including in dashboard.go) to keep media status consistent.
| return | ||
| } | ||
| if r.Method == http.MethodOptions { | ||
| writeProxyCORS(w) |
There was a problem hiding this comment.
🚨 issue (security): Serving arbitrary local file paths via ProxyMedia can expose unexpected files or paths to the dashboard origin.
Because any source without a scheme is treated as a local path and passed directly to http.ServeFile if os.Stat succeeds, a sender-controlled contentId like "../../somefile" could trigger path traversal and expose arbitrary local files. Please constrain local file access to a specific base directory (or known cache location), normalize and validate paths, and reject requests that resolve outside that directory to prevent unintended file exposure.
Comment on lines
+11
to
+15
| func TestServeCachedMediaSupportsRanges(t *testing.T) { | ||
| f, err := os.CreateTemp(t.TempDir(), "media-*") | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } |
There was a problem hiding this comment.
suggestion (testing): ProxyMedia behavior (local file vs remote URL, method handling, OPTIONS/CORS) is not currently exercised
ServeCachedMedia, hasScheme, and normalizeHTTPMediaURL are covered, but ProxyMedia’s key behaviors remain untested: 404 when source == "", OPTIONS/CORS with 204, rejection of non-GET/HEAD methods, serving local files, and proxying remote URLs with CORS headers. Please add httptest-based tests for at least the OPTIONS path, a local file source, and a simple HTTP upstream (e.g., via httptest.Server) to validate the new streaming behavior end-to-end.
Suggested implementation:
package cast
import (
"io"
"net/http"
"net/http/httptest"
"net/url"
"os"
"testing"
)
func TestServeCachedMediaSupportsRanges(t *testing.T) {
f, err := os.CreateTemp(t.TempDir(), "media-*")
if err != nil {
t.Fatal(err)
}
if _, err := f.WriteString("abcdef"); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
req := httptest.NewRequest(http.MethodGet, "/stream", nil)
req.Header.Set("Range", "bytes=2-4")
rr := httptest.NewRecorder()
ServeCachedMedia(rr, req, f.Name())
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusPartialContent {
t.Fatalf("expected status %d, got %d", http.StatusPartialContent, res.StatusCode)
}
got, err := io.ReadAll(res.Body)
if err != nil {
t.Fatal(err)
}
if string(got) != "cde" {
t.Fatalf("expected body %q, got %q", "cde", string(got))
}
}
func TestProxyMediaEmptySourceReturnsNotFound(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/proxy", nil)
rr := httptest.NewRecorder()
ProxyMedia(rr, req, "")
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusNotFound {
t.Fatalf("expected status %d for empty source, got %d", http.StatusNotFound, res.StatusCode)
}
}
func TestProxyMediaRejectsNonGETOrHEAD(t *testing.T) {
for _, method := range []string{http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch} {
t.Run(method, func(t *testing.T) {
req := httptest.NewRequest(method, "/proxy", nil)
rr := httptest.NewRecorder()
ProxyMedia(rr, req, "/does/not/matter")
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusMethodNotAllowed {
t.Fatalf("expected status %d for method %s, got %d", http.StatusMethodNotAllowed, method, res.StatusCode)
}
})
}
}
func TestProxyMediaOptionsReturnsNoContentWithCORS(t *testing.T) {
req := httptest.NewRequest(http.MethodOptions, "/proxy", nil)
req.Header.Set("Origin", "http://example.com")
rr := httptest.NewRecorder()
ProxyMedia(rr, req, "/ignored-for-options")
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusNoContent {
t.Fatalf("expected status %d for OPTIONS, got %d", http.StatusNoContent, res.StatusCode)
}
allowOrigin := res.Header.Get("Access-Control-Allow-Origin")
if allowOrigin == "" {
t.Fatalf("expected Access-Control-Allow-Origin header to be set")
}
if res.Header.Get("Access-Control-Allow-Methods") == "" {
t.Fatalf("expected Access-Control-Allow-Methods header to be set")
}
}
func TestProxyMediaServesLocalFile(t *testing.T) {
f, err := os.CreateTemp(t.TempDir(), "proxy-local-*")
if err != nil {
t.Fatal(err)
}
const content = "local-file-content"
if _, err := f.WriteString(content); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
req := httptest.NewRequest(http.MethodGet, "/proxy", nil)
rr := httptest.NewRecorder()
ProxyMedia(rr, req, f.Name())
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
t.Fatalf("expected status %d for local file, got %d", http.StatusOK, res.StatusCode)
}
body, err := io.ReadAll(res.Body)
if err != nil {
t.Fatal(err)
}
if string(body) != content {
t.Fatalf("expected body %q, got %q", content, string(body))
}
if res.Header.Get("Access-Control-Allow-Origin") == "" {
t.Fatalf("expected CORS headers on local file response")
}
}
func TestProxyMediaProxiesRemoteURLWithCORS(t *testing.T) {
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/plain")
_, _ = io.WriteString(w, "upstream-body")
}))
defer upstream.Close()
u, err := url.Parse(upstream.URL)
if err != nil {
t.Fatal(err)
}
req := httptest.NewRequest(http.MethodGet, "/proxy", nil)
req.Header.Set("Origin", "http://example-client")
rr := httptest.NewRecorder()
ProxyMedia(rr, req, u.String())
res := rr.Result()
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
t.Fatalf("expected status %d for remote URL, got %d", http.StatusOK, res.StatusCode)
}
body, err := io.ReadAll(res.Body)
if err != nil {
t.Fatal(err)
}
if string(body) != "upstream-body" {
t.Fatalf("expected proxied body %q, got %q", "upstream-body", string(body))
}
if res.Header.Get("Access-Control-Allow-Origin") == "" {
t.Fatalf("expected CORS headers on proxied response")
}
}The above tests assume the following signatures and behaviors, which you may need to align with your existing implementation:
-
ServeCachedMedia(w http.ResponseWriter, r *http.Request, path string):- Supports HTTP Range requests on a local file at
path, returningStatusPartialContentand the requested byte range. If the API differs, adjust the call and assertions accordingly.
- Supports HTTP Range requests on a local file at
-
ProxyMedia(w http.ResponseWriter, r *http.Request, source string):- Returns
StatusNotFoundwhensource == "". - Returns
StatusMethodNotAllowedfor non-GET/HEAD methods. - Handles
OPTIONSrequests withStatusNoContentand appropriate CORS headers (e.g.,Access-Control-Allow-Origin,Access-Control-Allow-Methods). - When
sourceis a local file path, streams its contents withStatusOKand applies CORS headers. - When
sourceis an HTTP/HTTPS URL, proxies the upstream response body withStatusOKand applies CORS headers.
- Returns
If your CORS implementation uses specific values (such as "*" for Access-Control-Allow-Origin or a fixed method list), update the header assertions to match those values. Also ensure the import list matches your project conventions (remove io or net/url if they are unused after aligning with the real functions).
Comment on lines
+5
to
+8
| func TestSourceIDForNamespaceUsesAppTransportForMedia(t *testing.T) { | ||
| if got := sourceIDForNamespace(nsMedia); got != defaultTransport { | ||
| t.Fatalf("sourceIDForNamespace(nsMedia) = %q, want %q", got, defaultTransport) | ||
| } |
There was a problem hiding this comment.
suggestion (testing): Session send/sendRaw behavior could be tested to confirm source/destination IDs and logging changes
These tests cover sourceIDForNamespace, but the higher-level Session APIs (send, sendRaw, rememberSender, senderDestination) remain untested. Given the routing changes, please also add tests that:
- Create a Session with a fake connection and assert send() routes via defaultTransport for nsMedia and defaultReceiverID for other namespaces.
- Exercise rememberSender/senderDestination and confirm sendRaw targets the remembered sender rather than the receiver.
This will verify the new Cast routing behavior end-to-end.
Suggested implementation:
package cast
import "testing"
func TestSourceIDForNamespaceUsesAppTransportForMedia(t *testing.T) {
if got := sourceIDForNamespace(nsMedia); got != defaultTransport {
t.Fatalf("sourceIDForNamespace(nsMedia) = %q, want %q", got, defaultTransport)
}
}
// fakeConn is a test double for the underlying connection used by Session.
// It records the last message sent so tests can inspect source/destination IDs.
type fakeConn struct {
lastNamespace string
lastSourceID string
lastDestination string
lastPayload []byte
}
func (c *fakeConn) Send(namespace, sourceID, destination string, payload []byte) error {
c.lastNamespace = namespace
c.lastSourceID = sourceID
c.lastDestination = destination
c.lastPayload = payload
return nil
}
func TestSessionSendRoutesMediaViaDefaultTransport(t *testing.T) {
conn := &fakeConn{}
// NOTE: Adjust this constructor call to match the actual Session initializer.
// For example, if it is NewSession(conn) or NewSessionWithDefaults(conn, defaultReceiverID, defaultTransport).
s := &Session{
conn: conn,
defaultReceiver: defaultReceiverID,
defaultTransport: defaultTransport,
}
payload := []byte(`{"type":"MEDIA_STATUS"}`)
// Media namespace should use defaultTransport as source and defaultReceiverID as destination.
if err := s.send(nsMedia, payload); err != nil {
t.Fatalf("send(nsMedia) error = %v, want nil", err)
}
if conn.lastNamespace != nsMedia {
t.Fatalf("send(nsMedia) namespace = %q, want %q", conn.lastNamespace, nsMedia)
}
if conn.lastSourceID != defaultTransport {
t.Fatalf("send(nsMedia) sourceID = %q, want %q", conn.lastSourceID, defaultTransport)
}
if conn.lastDestination != defaultReceiverID {
t.Fatalf("send(nsMedia) destination = %q, want %q", conn.lastDestination, defaultReceiverID)
}
// Non-media namespaces should use the receiver as both source and destination.
if err := s.send(nsReceiver, payload); err != nil {
t.Fatalf("send(nsReceiver) error = %v, want nil", err)
}
if conn.lastNamespace != nsReceiver {
t.Fatalf("send(nsReceiver) namespace = %q, want %q", conn.lastNamespace, nsReceiver)
}
if conn.lastSourceID != defaultReceiverID {
t.Fatalf("send(nsReceiver) sourceID = %q, want %q", conn.lastSourceID, defaultReceiverID)
}
if conn.lastDestination != defaultReceiverID {
t.Fatalf("send(nsReceiver) destination = %q, want %q", conn.lastDestination, defaultReceiverID)
}
}
func TestSessionSendRawUsesRememberedSenderDestination(t *testing.T) {
conn := &fakeConn{}
// NOTE: Adjust this constructor call to match the actual Session initializer.
s := &Session{
conn: conn,
defaultReceiver: defaultReceiverID,
defaultTransport: defaultTransport,
}
// Remember a sender (e.g., an app transport) as the destination for subsequent sendRaw calls.
appSenderID := "sender-123"
s.rememberSender(appSenderID)
payload := []byte(`{"type":"CUSTOM"}`)
// sendRaw should route to the remembered sender, not the receiver.
if err := s.sendRaw(nsMedia, payload); err != nil {
t.Fatalf("sendRaw(nsMedia) error = %v, want nil", err)
}
if conn.lastNamespace != nsMedia {
t.Fatalf("sendRaw(nsMedia) namespace = %q, want %q", conn.lastNamespace, nsMedia)
}
// Source should typically remain the transport for media.
if conn.lastSourceID != defaultTransport {
t.Fatalf("sendRaw(nsMedia) sourceID = %q, want %q", conn.lastSourceID, defaultTransport)
}
// Destination should be the remembered sender, not the receiver.
if conn.lastDestination != appSenderID {
t.Fatalf("sendRaw(nsMedia) destination = %q, want %q", conn.lastDestination, appSenderID)
}
// Verify senderDestination accessor reflects the remembered sender.
if got := s.senderDestination(); got != appSenderID {
t.Fatalf("senderDestination() = %q, want %q", got, appSenderID)
}
}
func TestSourceIDForNamespaceUsesReceiverForReceiverNamespaces(t *testing.T) {
for _, ns := range []string{nsReceiver, nsHeartbeat, nsConnection} {- Align the
fakeConntest double with the actual connection interface/struct used inSession. If theSessionwrites protocol messages via a different method signature (e.g.Send(*Message)), adjustfakeConnaccordingly and update the tests to capturesourceID/destinationfrom the real message type. - Replace the
Sessionconstruction in the tests with the real initializer. For example, if there is aNewSession(conn)function or fields are unexported, use the appropriate constructor and remove direct field assignment. - Ensure the
Session.send,Session.sendRaw,Session.rememberSender, andSession.senderDestinationsignatures match the calls in the tests. If their parameter lists differ (e.g. they accept a context, message type, or additional flags), update the tests to pass the required arguments. - If routing logic around
defaultTransport,defaultReceiverID, and media namespaces behaves slightly differently (e.g. additional namespaces, different defaults), adjust the test expectations (conn.lastSourceIDandconn.lastDestinationchecks) to match the actual routing rules. - If additional imports (such as
contextor other packages) are needed for the realSessionconstructor or message type, add them to the import block and ensure there is still only one consolidatedimportsection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds support for video playback and fixes some playback issues.
Can be tested with the nightly VLC build:
Summary by Sourcery
Add spec-compliant media session handling, TLS compatibility improvements, and a dashboard video player with streaming proxy support.
New Features:
Bug Fixes:
Enhancements:
Tests: