fix(playback): expose conditional range outcomes (#594)

* fix(playback): expose conditional range outcomes

* fix(playback): classify rejected If-Range requests

* fix(playback): evaluate If-Range diagnostics directly
This commit is contained in:
Quick
2026-08-11 10:31:39 -04:00
committed by GitHub
parent c644f43a02
commit 90bdbfeb84
3 changed files with 394 additions and 13 deletions
+14
View File
@@ -31,6 +31,20 @@ entire current entity as `200 OK`, preventing bytes from different revisions
from being combined. `If-None-Match` uses the same validator for ordinary
conditional requests.
For a bounded chunk read, a client can instead send `Range` and `If-Match`.
When the validator matches, the server returns the requested `206` interval.
When it does not match, the server returns `412 Precondition Failed` without a
body. This contract avoids materializing a full entity when a bounded read's
validator is stale. It does not change `If-Range`: an `If-Range` mismatch still
ignores the range and returns the full current entity as `200 OK`, as required
by the HTTP range contract.
Direct-stream completion logs record whether `If-Match` and `If-Range` were
present, a bounded conditional-result value, and short SHA-256 fingerprints for
the emitted and requested validators. The raw validators and media path are not
logged. The fingerprints are only correlation aids and are not protocol
validators.
Playback sessions already treat each transport request independently.
Sequential ranged requests refresh transport activity, and cleanup never
expires a session while one of those transport requests is active. A late
+114 -5
View File
@@ -1,13 +1,16 @@
package playback
import (
"crypto/sha256"
"fmt"
"log/slog"
"net/http"
"net/textproto"
"os"
"path/filepath"
"strconv"
"strings"
"time"
"github.com/Silo-Server/silo-server/internal/httpstream"
)
@@ -52,8 +55,8 @@ func MimeFromExtension(name string) string {
// ServeDirectPlay serves a media file with HTTP byte-range support.
// Uses http.ServeContent for proper range handling, which supports
// Range requests, conditional requests (If-Modified-Since, If-None-Match),
// and Content-Type detection.
// Range requests, conditional requests (including If-Match, If-Range, and
// If-None-Match), and Content-Type detection.
func ServeDirectPlay(w http.ResponseWriter, r *http.Request, filePath string) error {
// Media bodies routinely take longer than the server's absolute
// WriteTimeout; roll the write deadline with progress instead.
@@ -81,7 +84,8 @@ func ServeDirectPlay(w http.ResponseWriter, r *http.Request, filePath string) er
}
w.Header().Del("ETag")
if etag := directPlayEntityTag(f, stat); etag != "" {
etag := directPlayEntityTag(f, stat)
if etag != "" {
w.Header().Set("ETag", etag)
}
@@ -89,7 +93,9 @@ func ServeDirectPlay(w http.ResponseWriter, r *http.Request, filePath string) er
w.Header().Set("Content-Type", MimeFromExtension(filePath))
hadRange := len(r.Header.Values("Range")) > 0
hadIfMatch := len(r.Header.Values("If-Match")) > 0
hadIfRange := len(r.Header.Values("If-Range")) > 0
ifRangeResult := directStreamIfRangeResult(r, etag, stat.ModTime())
directStreamActive.Inc()
http.ServeContent(w, r, stat.Name(), stat.ModTime(), f)
outcome := streamWriter.Outcome(r.Context())
@@ -97,15 +103,27 @@ func ServeDirectPlay(w http.ResponseWriter, r *http.Request, filePath string) er
bytesSent := streamWriter.BytesWritten()
rangeStart := directStreamRangeStart(status, w.Header().Get("Content-Range"))
recordDirectStreamEnd(outcome, status, bytesSent, rangeStart)
slog.InfoContext(r.Context(), "direct stream ended",
logAttrs := []any{
"component", "playback",
"outcome", outcome,
"status", status,
"bytes_sent", bytesSent,
"range_requested", hadRange,
"range_start", rangeStart,
"had_if_match", hadIfMatch,
"had_if_range", hadIfRange,
)
"conditional_result", directStreamConditionalResult(status, hadIfMatch, hadIfRange, ifRangeResult),
}
if fingerprint := directStreamValidatorFingerprint(etag); fingerprint != "" {
logAttrs = append(logAttrs, "etag_fingerprint", fingerprint)
}
if fingerprint := directStreamHeaderFingerprint(r.Header, "If-Match"); fingerprint != "" {
logAttrs = append(logAttrs, "if_match_fingerprint", fingerprint)
}
if fingerprint := directStreamHeaderFingerprint(r.Header, "If-Range"); fingerprint != "" {
logAttrs = append(logAttrs, "if_range_fingerprint", fingerprint)
}
slog.InfoContext(r.Context(), "direct stream ended", logAttrs...)
return nil
}
@@ -144,3 +162,94 @@ func directStreamRangeStart(status int, contentRange string) int64 {
}
return parsedStart
}
const (
directStreamConditionalNone = "none"
directStreamConditionalIfMatchPassed = "if_match_passed"
directStreamConditionalIfMatchFailed = "if_match_failed"
directStreamConditionalIfRangeMatched = "if_range_matched"
directStreamConditionalIfRangeMismatched = "if_range_mismatched"
directStreamConditionalIfRangeNotEvaluated = "if_range_not_evaluated"
)
func directStreamConditionalResult(status int, hadIfMatch, hadIfRange bool, ifRangeResult string) string {
switch {
case hadIfMatch && status == http.StatusPreconditionFailed:
return directStreamConditionalIfMatchFailed
case ifRangeResult != "" && status != http.StatusNotModified && status != http.StatusPreconditionFailed:
return ifRangeResult
case hadIfMatch:
return directStreamConditionalIfMatchPassed
case hadIfRange:
return directStreamConditionalIfRangeNotEvaluated
default:
return directStreamConditionalNone
}
}
// directStreamIfRangeResult mirrors the If-Range decision made by
// http.ServeContent before range parsing. The final status cannot encode that
// decision reliably: ServeContent may reject a matched range with 416 or
// deliberately ignore matched aggregate ranges and return 200.
func directStreamIfRangeResult(r *http.Request, etag string, modtime time.Time) string {
if r.Method != http.MethodGet && r.Method != http.MethodHead {
return ""
}
if r.Header.Get("Range") == "" {
return ""
}
validator := r.Header.Get("If-Range")
if validator == "" {
return ""
}
if validatorETag := directStreamScanEntityTag(validator); validatorETag != "" {
if validatorETag == etag && validatorETag[0] == '"' {
return directStreamConditionalIfRangeMatched
}
return directStreamConditionalIfRangeMismatched
}
if modtime.IsZero() {
return directStreamConditionalIfRangeMismatched
}
validatorTime, err := http.ParseTime(validator)
if err == nil && validatorTime.Unix() == modtime.Unix() {
return directStreamConditionalIfRangeMatched
}
return directStreamConditionalIfRangeMismatched
}
// directStreamScanEntityTag is the narrow ETag scanner needed to mirror
// net/http's unexported scanETag behavior for If-Range diagnostics.
func directStreamScanEntityTag(value string) string {
value = textproto.TrimString(value)
start := 0
if strings.HasPrefix(value, "W/") {
start = 2
}
if len(value[start:]) < 2 || value[start] != '"' {
return ""
}
for i := start + 1; i < len(value); i++ {
character := value[i]
switch {
case character == 0x21 || character >= 0x23 && character <= 0x7e || character >= 0x80:
case character == '"':
return value[:i+1]
default:
return ""
}
}
return ""
}
func directStreamHeaderFingerprint(header http.Header, name string) string {
return directStreamValidatorFingerprint(strings.Join(header.Values(name), "\x00"))
}
func directStreamValidatorFingerprint(validator string) string {
if strings.TrimSpace(validator) == "" {
return ""
}
digest := sha256.Sum256([]byte(validator))
return fmt.Sprintf("%x", digest[:8])
}
+266 -8
View File
@@ -1,8 +1,11 @@
package playback
import (
"bytes"
"encoding/json"
"fmt"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"os"
@@ -30,12 +33,15 @@ func TestServeDirectPlayHTTPContract(t *testing.T) {
t.Fatal(err)
}
serve := func(method, rangeHeader, ifRange, ifNoneMatch string) *httptest.ResponseRecorder {
serve := func(method, rangeHeader, ifMatch, ifRange, ifNoneMatch string) *httptest.ResponseRecorder {
t.Helper()
req := httptest.NewRequest(method, "/stream", nil)
if rangeHeader != "" {
req.Header.Set("Range", rangeHeader)
}
if ifMatch != "" {
req.Header.Set("If-Match", ifMatch)
}
if ifRange != "" {
req.Header.Set("If-Range", ifRange)
}
@@ -49,7 +55,7 @@ func TestServeDirectPlayHTTPContract(t *testing.T) {
return rr
}
full := serve(http.MethodGet, "", "", "")
full := serve(http.MethodGet, "", "", "", "")
if full.Code != http.StatusOK {
t.Fatalf("full status = %d, want 200", full.Code)
}
@@ -72,7 +78,7 @@ func TestServeDirectPlayHTTPContract(t *testing.T) {
}
t.Run("HEAD", func(t *testing.T) {
rr := serve(http.MethodHead, "", "", "")
rr := serve(http.MethodHead, "", "", "", "")
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200", rr.Code)
}
@@ -144,7 +150,7 @@ func TestServeDirectPlayHTTPContract(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resumesBefore := counterValue(t, directStreamRangeResumes)
rr := serve(http.MethodGet, tt.rangeHeader, "", "")
rr := serve(http.MethodGet, tt.rangeHeader, "", "", "")
if rr.Code != tt.wantStatus {
t.Fatalf("status = %d, want %d; body = %q", rr.Code, tt.wantStatus, rr.Body.String())
}
@@ -169,7 +175,7 @@ func TestServeDirectPlayHTTPContract(t *testing.T) {
if !validatorRequired {
t.Skip("platform does not expose a durable file revision")
}
rr := serve(http.MethodGet, "bytes=7-", etag, "")
rr := serve(http.MethodGet, "bytes=7-", "", etag, "")
if rr.Code != http.StatusPartialContent {
t.Fatalf("status = %d, want 206", rr.Code)
}
@@ -179,7 +185,7 @@ func TestServeDirectPlayHTTPContract(t *testing.T) {
})
t.Run("stale If-Range", func(t *testing.T) {
rr := serve(http.MethodGet, "bytes=7-", "\"stale\"", "")
rr := serve(http.MethodGet, "bytes=7-", "", "\"stale\"", "")
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200", rr.Code)
}
@@ -188,11 +194,34 @@ func TestServeDirectPlayHTTPContract(t *testing.T) {
}
})
t.Run("bounded range with matching If-Match", func(t *testing.T) {
if !validatorRequired {
t.Skip("platform does not expose a durable file revision")
}
rr := serve(http.MethodGet, "bytes=7-11", etag, "", "")
if rr.Code != http.StatusPartialContent {
t.Fatalf("status = %d, want 206", rr.Code)
}
if body := rr.Body.String(); body != content[7:12] {
t.Fatalf("body = %q, want %q", body, content[7:12])
}
})
t.Run("bounded range with stale If-Match", func(t *testing.T) {
rr := serve(http.MethodGet, "bytes=7-11", "\"stale\"", "", "")
if rr.Code != http.StatusPreconditionFailed {
t.Fatalf("status = %d, want 412", rr.Code)
}
if rr.Body.Len() != 0 {
t.Fatalf("body length = %d, want 0", rr.Body.Len())
}
})
t.Run("If-None-Match", func(t *testing.T) {
if !validatorRequired {
t.Skip("platform does not expose a durable file revision")
}
rr := serve(http.MethodGet, "", "", etag)
rr := serve(http.MethodGet, "", "", "", etag)
if rr.Code != http.StatusNotModified {
t.Fatalf("status = %d, want 304", rr.Code)
}
@@ -202,7 +231,219 @@ func TestServeDirectPlayHTTPContract(t *testing.T) {
})
}
func TestServeDirectPlayChangedEntityRejectsOldIfRange(t *testing.T) {
func TestDirectStreamConditionalResult(t *testing.T) {
tests := []struct {
name string
status int
hadIfMatch bool
hadIfRange bool
ifRangeResult string
want string
}{
{name: "unconditional", status: http.StatusOK, want: directStreamConditionalNone},
{name: "If-Match passed", status: http.StatusPartialContent, hadIfMatch: true, want: directStreamConditionalIfMatchPassed},
{name: "If-Match failed", status: http.StatusPreconditionFailed, hadIfMatch: true, want: directStreamConditionalIfMatchFailed},
{name: "If-Range matched", status: http.StatusPartialContent, hadIfRange: true, ifRangeResult: directStreamConditionalIfRangeMatched, want: directStreamConditionalIfRangeMatched},
{name: "If-Range mismatched", status: http.StatusOK, hadIfRange: true, ifRangeResult: directStreamConditionalIfRangeMismatched, want: directStreamConditionalIfRangeMismatched},
{name: "If-Range not evaluated after 304", status: http.StatusNotModified, hadIfRange: true, ifRangeResult: directStreamConditionalIfRangeMatched, want: directStreamConditionalIfRangeNotEvaluated},
{name: "If-Range not evaluated after 412", status: http.StatusPreconditionFailed, hadIfRange: true, ifRangeResult: directStreamConditionalIfRangeMismatched, want: directStreamConditionalIfRangeNotEvaluated},
{name: "If-Range without Range", status: http.StatusOK, hadIfRange: true, want: directStreamConditionalIfRangeNotEvaluated},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := directStreamConditionalResult(tt.status, tt.hadIfMatch, tt.hadIfRange, tt.ifRangeResult); got != tt.want {
t.Fatalf("conditional result = %q, want %q", got, tt.want)
}
})
}
}
func TestDirectStreamIfRangeResult(t *testing.T) {
const currentETag = `"current"`
modtime := time.Date(2026, time.August, 11, 12, 0, 0, 0, time.UTC)
tests := []struct {
name string
method string
rangeHead string
validator string
modtime time.Time
want string
}{
{name: "matching ETag", method: http.MethodGet, rangeHead: "bytes=2-4", validator: currentETag, modtime: modtime, want: directStreamConditionalIfRangeMatched},
{name: "matching ETag with whitespace", method: http.MethodGet, rangeHead: "bytes=2-4", validator: ` "current" `, modtime: modtime, want: directStreamConditionalIfRangeMatched},
{name: "stale ETag", method: http.MethodGet, rangeHead: "bytes=2-4", validator: `"stale"`, modtime: modtime, want: directStreamConditionalIfRangeMismatched},
{name: "weak ETag", method: http.MethodGet, rangeHead: "bytes=2-4", validator: `W/"current"`, modtime: modtime, want: directStreamConditionalIfRangeMismatched},
{name: "matching date", method: http.MethodHead, rangeHead: "bytes=2-4", validator: modtime.Format(http.TimeFormat), modtime: modtime, want: directStreamConditionalIfRangeMatched},
{name: "stale date", method: http.MethodGet, rangeHead: "bytes=2-4", validator: modtime.Add(-time.Second).Format(http.TimeFormat), modtime: modtime, want: directStreamConditionalIfRangeMismatched},
{name: "missing range", method: http.MethodGet, validator: currentETag, modtime: modtime},
{name: "empty validator", method: http.MethodGet, rangeHead: "bytes=2-4", modtime: modtime},
{name: "unsupported method", method: http.MethodPost, rangeHead: "bytes=2-4", validator: currentETag, modtime: modtime},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req := httptest.NewRequest(tt.method, "/stream", nil)
if tt.rangeHead != "" {
req.Header.Set("Range", tt.rangeHead)
}
if tt.validator != "" {
req.Header.Set("If-Range", tt.validator)
}
if got := directStreamIfRangeResult(req, currentETag, tt.modtime); got != tt.want {
t.Fatalf("If-Range result = %q, want %q", got, tt.want)
}
})
}
t.Run("uses first If-Range header", func(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/stream", nil)
req.Header.Set("Range", "bytes=2-4")
req.Header["If-Range"] = []string{currentETag, `"stale"`}
if got := directStreamIfRangeResult(req, currentETag, modtime); got != directStreamConditionalIfRangeMatched {
t.Fatalf("If-Range result = %q, want %q", got, directStreamConditionalIfRangeMatched)
}
})
}
func TestDirectStreamValidatorFingerprint(t *testing.T) {
const validator = "\"private-validator\""
fingerprint := directStreamValidatorFingerprint(validator)
if len(fingerprint) != 16 {
t.Fatalf("fingerprint length = %d, want 16", len(fingerprint))
}
if strings.Contains(fingerprint, "private") || strings.Contains(fingerprint, "validator") {
t.Fatalf("fingerprint leaked validator text: %q", fingerprint)
}
if got := directStreamValidatorFingerprint(validator); got != fingerprint {
t.Fatalf("fingerprint = %q, want stable %q", got, fingerprint)
}
if got := directStreamValidatorFingerprint("\"different\""); got == fingerprint {
t.Fatalf("different validator produced the same fingerprint %q", got)
}
if got := directStreamValidatorFingerprint(" "); got != "" {
t.Fatalf("blank validator fingerprint = %q, want omitted", got)
}
}
func TestServeDirectPlayConditionalDiagnostics(t *testing.T) {
if !platformRequiresDirectPlayValidator() {
t.Skip("platform does not expose a durable file revision")
}
filePath := filepath.Join(t.TempDir(), "private-fixture.mp4")
if err := os.WriteFile(filePath, []byte("0123456789"), 0o600); err != nil {
t.Fatal(err)
}
var logs bytes.Buffer
previousLogger := slog.Default()
slog.SetDefault(slog.New(slog.NewJSONHandler(&logs, nil)))
t.Cleanup(func() { slog.SetDefault(previousLogger) })
initial := httptest.NewRecorder()
if err := ServeDirectPlay(initial, httptest.NewRequest(http.MethodGet, "/stream", nil), filePath); err != nil {
t.Fatal(err)
}
currentETag := initial.Header().Get("ETag")
if currentETag == "" {
t.Fatal("initial response omitted ETag")
}
tests := []struct {
name string
rangeHeader string
headerName string
validator string
wantStatus int
wantResult string
fingerprintName string
}{
{
name: "If-Match mismatch",
rangeHeader: "bytes=2-4",
headerName: "If-Match",
validator: "\"stale-private-if-match\"",
wantStatus: http.StatusPreconditionFailed,
wantResult: directStreamConditionalIfMatchFailed,
fingerprintName: "if_match_fingerprint",
},
{
name: "If-Range mismatch",
rangeHeader: "bytes=2-4",
headerName: "If-Range",
validator: "\"stale-private-if-range\"",
wantStatus: http.StatusOK,
wantResult: directStreamConditionalIfRangeMismatched,
fingerprintName: "if_range_fingerprint",
},
{
name: "If-Range match with unsatisfiable range",
rangeHeader: "bytes=999-1000",
headerName: "If-Range",
validator: currentETag,
wantStatus: http.StatusRequestedRangeNotSatisfiable,
wantResult: directStreamConditionalIfRangeMatched,
fingerprintName: "if_range_fingerprint",
},
{
name: "If-Range match with ignored aggregate ranges",
rangeHeader: "bytes=0-9,0-9",
headerName: "If-Range",
validator: currentETag,
wantStatus: http.StatusOK,
wantResult: directStreamConditionalIfRangeMatched,
fingerprintName: "if_range_fingerprint",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
logs.Reset()
req := httptest.NewRequest(http.MethodGet, "/stream", nil)
req.Header.Set("Range", tt.rangeHeader)
req.Header.Set(tt.headerName, tt.validator)
rr := httptest.NewRecorder()
if err := ServeDirectPlay(rr, req, filePath); err != nil {
t.Fatal(err)
}
if rr.Code != tt.wantStatus {
t.Fatalf("status = %d, want %d", rr.Code, tt.wantStatus)
}
var record map[string]any
if err := json.Unmarshal(logs.Bytes(), &record); err != nil {
t.Fatalf("decode structured log %q: %v", logs.String(), err)
}
if got := record["conditional_result"]; got != tt.wantResult {
t.Fatalf("conditional_result = %v, want %q", got, tt.wantResult)
}
if got := record["had_if_match"]; got != (tt.headerName == "If-Match") {
t.Fatalf("had_if_match = %v, want %v", got, tt.headerName == "If-Match")
}
if got := record[tt.fingerprintName]; got != directStreamValidatorFingerprint(tt.validator) {
t.Fatalf("%s = %v, want request fingerprint", tt.fingerprintName, got)
}
responseETag := rr.Header().Get("ETag")
if tt.wantStatus == http.StatusRequestedRangeNotSatisfiable {
// ServeContent strips ETag from its 416 response, but the end log
// still fingerprints the validator used for If-Range evaluation.
responseETag = currentETag
}
if got := record["etag_fingerprint"]; got != directStreamValidatorFingerprint(responseETag) {
t.Fatalf("etag_fingerprint = %v, want response ETag fingerprint", got)
}
if strings.Contains(logs.String(), strings.Trim(tt.validator, "\"")) {
t.Fatalf("structured log leaked raw request validator: %s", logs.String())
}
if strings.Contains(logs.String(), strings.Trim(responseETag, "\"")) {
t.Fatalf("structured log leaked raw response validator: %s", logs.String())
}
if strings.Contains(logs.String(), filePath) {
t.Fatalf("structured log leaked media path: %s", logs.String())
}
})
}
}
func TestServeDirectPlayChangedEntityRejectsOldValidators(t *testing.T) {
if !platformRequiresDirectPlayValidator() {
t.Skip("platform does not expose a durable file revision")
}
@@ -275,6 +516,23 @@ func TestServeDirectPlayChangedEntityRejectsOldIfRange(t *testing.T) {
if newETag := rr.Header().Get("ETag"); newETag == oldETag {
t.Fatalf("ETag did not change after replacement: %q", newETag)
}
ifMatchRequest := httptest.NewRequest(http.MethodGet, "/stream", nil)
ifMatchRequest.Header.Set("Range", "bytes=5-8")
ifMatchRequest.Header.Set("If-Match", oldETag)
ifMatchResponse := httptest.NewRecorder()
if err := ServeDirectPlay(ifMatchResponse, ifMatchRequest, filePath); err != nil {
t.Fatal(err)
}
if ifMatchResponse.Code != http.StatusPreconditionFailed {
t.Fatalf("If-Match status = %d, want 412", ifMatchResponse.Code)
}
if ifMatchResponse.Body.Len() != 0 {
t.Fatalf("If-Match body length = %d, want 0", ifMatchResponse.Body.Len())
}
if newETag := ifMatchResponse.Header().Get("ETag"); newETag == oldETag {
t.Fatalf("If-Match response did not expose the replacement ETag: %q", newETag)
}
}
func TestDirectPlayEntityTagOmitsUnsupportedRevision(t *testing.T) {