Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
813e679f93 | ||
|
|
b9643f5fb7 | ||
|
|
cace7d179d | ||
|
|
550764d43c |
+13
-4
@@ -1466,10 +1466,19 @@ an hour of Swedish *was* an unknown here; it is now measured, see decision 8.)
|
|||||||
as busy beyond that. Together with the 20-job queue and the running job that bounds the spool
|
as busy beyond that. Together with the 20-job queue and the running job that bounds the spool
|
||||||
at about (4 + 20 + 1) x 200 MiB = 5 GiB of the 6 Gi `emptyDir` (infra): raising any of those or
|
at about (4 + 20 + 1) x 200 MiB = 5 GiB of the 6 Gi `emptyDir` (infra): raising any of those or
|
||||||
`TAPIR_UPLOAD_MAX_BYTES` must revisit that sum (pinned by `TestNewIntake_Defaults...`, not
|
`TAPIR_UPLOAD_MAX_BYTES` must revisit that sum (pinned by `TestNewIntake_Defaults...`, not
|
||||||
enforced against infra). **A slow-drip body holds its slot without a time bound**: the server
|
enforced against infra). **A body is bounded by its silence, not its length (#56, amended
|
||||||
sets `ReadHeaderTimeout` only, because a `ReadTimeout` would cut legitimate 200 MiB uploads on
|
2026-10-04)**: a `ReadTimeout` would cut legitimate 200 MiB uploads on slow links, so instead
|
||||||
slow links; two slow bodies take a user's two slots and two such users make everyone's upload
|
every read of the upload body re-arms the connection's read deadline 60 s ahead
|
||||||
busy. It needs a valid session; recorded as a limitation with a follow-up issue.
|
(`App.UploadIdleTimeout`). A body that stops delivering bytes fails its read, Accept returns
|
||||||
|
and the slot is released; one that keeps moving is never cut. The handler never clears the
|
||||||
|
deadline: an armed deadline also bounds net/http's drain of an unread remainder before it
|
||||||
|
replies, and net/http resets it itself before its background read. A deadline still armed
|
||||||
|
while the file is probed (the multipart part can end before the body's last bytes arrive)
|
||||||
|
costs at most keep-alive reuse of that connection, not the upload. The 60 s default is the
|
||||||
|
deployed value (cmd/tapir does not set it) and is pinned by a test. It needs `http.ResponseController` to reach the connection, so the
|
||||||
|
metrics middleware's writer implements `Unwrap`; where a deadline cannot be set the handler
|
||||||
|
logs `upload idle bound unavailable` rather than silently running unbounded. Still not
|
||||||
|
bounded: a body that drips one byte per 59 s holds its slot for as long as it keeps that up.
|
||||||
|
|
||||||
8. **Chat reuses `chat.Service`; it is not forked.** The uploaded-transcript chat handlers
|
8. **Chat reuses `chat.Service`; it is not forked.** The uploaded-transcript chat handlers
|
||||||
look the row up through the RLS-scoped store, so a non-owner gets 404 by the same mechanism
|
look the row up through the RLS-scoped store, so a non-owner gets 404 by the same mechanism
|
||||||
|
|||||||
@@ -33,9 +33,9 @@ require (
|
|||||||
github.com/rogpeppe/go-internal v1.15.0 // indirect
|
github.com/rogpeppe/go-internal v1.15.0 // indirect
|
||||||
github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect
|
github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect
|
||||||
go.yaml.in/yaml/v2 v2.4.2 // indirect
|
go.yaml.in/yaml/v2 v2.4.2 // indirect
|
||||||
golang.org/x/sync v0.18.0 // indirect
|
golang.org/x/sync v0.21.0 // indirect
|
||||||
golang.org/x/sys v0.41.0 // indirect
|
golang.org/x/sys v0.41.0 // indirect
|
||||||
golang.org/x/text v0.31.0 // indirect
|
golang.org/x/text v0.39.0 // indirect
|
||||||
google.golang.org/protobuf v1.36.8 // indirect
|
google.golang.org/protobuf v1.36.8 // indirect
|
||||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
gopkg.in/yaml.v3 v3.0.1 // indirect
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -113,12 +113,12 @@ go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI=
|
|||||||
go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU=
|
go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU=
|
||||||
golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs=
|
golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs=
|
||||||
golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q=
|
golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q=
|
||||||
golang.org/x/sync v0.18.0 h1:kr88TuHDroi+UVf+0hZnirlk8o8T+4MrK6mr60WkH/I=
|
golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM=
|
||||||
golang.org/x/sync v0.18.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
|
golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
|
||||||
golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k=
|
golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k=
|
||||||
golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
|
golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
|
||||||
golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM=
|
golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus=
|
||||||
golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM=
|
golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM=
|
||||||
golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U=
|
golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U=
|
||||||
golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno=
|
golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno=
|
||||||
google.golang.org/protobuf v1.36.8 h1:xHScyCOEuuwZEc6UtSOvPbAT4zRh0xcNRYekJwfqyMc=
|
google.golang.org/protobuf v1.36.8 h1:xHScyCOEuuwZEc6UtSOvPbAT4zRh0xcNRYekJwfqyMc=
|
||||||
|
|||||||
@@ -125,6 +125,11 @@ type statusWriter struct {
|
|||||||
wroteHeader bool
|
wroteHeader bool
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Unwrap exposes the underlying writer to http.ResponseController, so a handler behind this
|
||||||
|
// middleware can still set a connection read deadline (#56: without it the upload idle bound
|
||||||
|
// was silently unavailable in production).
|
||||||
|
func (s *statusWriter) Unwrap() http.ResponseWriter { return s.ResponseWriter }
|
||||||
|
|
||||||
func (s *statusWriter) WriteHeader(code int) {
|
func (s *statusWriter) WriteHeader(code int) {
|
||||||
if !s.wroteHeader {
|
if !s.wroteHeader {
|
||||||
s.code = code
|
s.code = code
|
||||||
|
|||||||
@@ -0,0 +1,11 @@
|
|||||||
|
package web
|
||||||
|
|
||||||
|
import "time"
|
||||||
|
|
||||||
|
// DefaultUploadIdleTimeout exposes the production idle bound (#56) to the black-box tests:
|
||||||
|
// cmd/tapir never sets App.UploadIdleTimeout, so this constant IS the deployed value.
|
||||||
|
const DefaultUploadIdleTimeout = defaultUploadIdleTimeout
|
||||||
|
|
||||||
|
// UploadIdleTimeoutForTest exposes the bound an App resolves to (#56), so a test can pin the
|
||||||
|
// value the zero-value App -- production's -- actually uses rather than only the constant.
|
||||||
|
func (a *App) UploadIdleTimeoutForTest() time.Duration { return a.uploadIdleTimeout() }
|
||||||
@@ -116,6 +116,11 @@ type App struct {
|
|||||||
// front rather than discovered by failing.
|
// front rather than discovered by failing.
|
||||||
Uploads Uploader
|
Uploads Uploader
|
||||||
UploadLimits uploads.Limits
|
UploadLimits uploads.Limits
|
||||||
|
// UploadIdleTimeout is how long an upload body may go without delivering a byte
|
||||||
|
// before the connection read is cut off and its receive slot freed (#56). Zero
|
||||||
|
// means defaultUploadIdleTimeout. It bounds silence, not the whole body, so a
|
||||||
|
// large upload on a slow but moving link still completes.
|
||||||
|
UploadIdleTimeout time.Duration
|
||||||
// UploadChatMaxChars is the transcript budget for chat over an uploaded
|
// UploadChatMaxChars is the transcript budget for chat over an uploaded
|
||||||
// recording. The caption default (18,000 chars, ~22 minutes of speech) would
|
// recording. The caption default (18,000 chars, ~22 minutes of speech) would
|
||||||
// keep only the first third of an hour-long meeting.
|
// keep only the first third of an hour-long meeting.
|
||||||
|
|||||||
@@ -0,0 +1,61 @@
|
|||||||
|
package oidc_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"errors"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"git.d-ma.be/mathias/tapir/internal/web/oidc"
|
||||||
|
)
|
||||||
|
|
||||||
|
func testConfig(issuer string) oidc.Config {
|
||||||
|
return oidc.Config{
|
||||||
|
Issuer: issuer,
|
||||||
|
ClientID: testClientID,
|
||||||
|
ClientSecret: "test-client-secret",
|
||||||
|
RedirectURL: "http://tapir.test/auth/callback",
|
||||||
|
SessionSecret: "test-session-secret-please-change",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// #59: a discovery 503 at start-up is retried instead of failing New.
|
||||||
|
func TestNewRetriesDiscoveryAfter503UntilIssuerAnswers(t *testing.T) {
|
||||||
|
f := newFakeIssuer(t)
|
||||||
|
f.unavailableFor = 2
|
||||||
|
|
||||||
|
auth, err := oidc.New(context.Background(), testConfig(f.server.URL),
|
||||||
|
oidc.WithDiscoveryRetry(5, time.Millisecond))
|
||||||
|
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NotNil(t, auth)
|
||||||
|
require.Equal(t, int32(3), f.discoveryHits.Load())
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestNewReturnsDiscoveryErrorAfterAttemptsExhausted(t *testing.T) {
|
||||||
|
f := newFakeIssuer(t)
|
||||||
|
f.unavailableFor = 1000
|
||||||
|
|
||||||
|
_, err := oidc.New(context.Background(), testConfig(f.server.URL),
|
||||||
|
oidc.WithDiscoveryRetry(3, time.Millisecond))
|
||||||
|
|
||||||
|
require.ErrorContains(t, err, "503")
|
||||||
|
require.Equal(t, int32(3), f.discoveryHits.Load())
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestNewStopsDiscoveryBackoffWhenContextCancelled(t *testing.T) {
|
||||||
|
f := newFakeIssuer(t)
|
||||||
|
f.unavailableFor = 1000
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
start := time.Now()
|
||||||
|
_, err := oidc.New(ctx, testConfig(f.server.URL),
|
||||||
|
oidc.WithDiscoveryRetry(5, time.Hour))
|
||||||
|
|
||||||
|
require.True(t, errors.Is(err, context.DeadlineExceeded), "err = %v", err)
|
||||||
|
require.Less(t, time.Since(start), 5*time.Second)
|
||||||
|
require.Equal(t, int32(1), f.discoveryHits.Load())
|
||||||
|
}
|
||||||
+70
-20
@@ -57,6 +57,10 @@ const (
|
|||||||
pendingTTL = 10 * time.Minute
|
pendingTTL = 10 * time.Minute
|
||||||
sessionCookie = "tapir_session"
|
sessionCookie = "tapir_session"
|
||||||
loginPath = "/auth/login"
|
loginPath = "/auth/login"
|
||||||
|
|
||||||
|
// Discovery retry (#59): 5 tries, 2+4+8+16 s back-off ≈ 30 s, then fail.
|
||||||
|
defaultDiscoveryAttempts = 5
|
||||||
|
defaultDiscoveryDelay = 2 * time.Second
|
||||||
)
|
)
|
||||||
|
|
||||||
// DexAuth implements web.Auth against Dex.
|
// DexAuth implements web.Auth against Dex.
|
||||||
@@ -70,6 +74,9 @@ type DexAuth struct {
|
|||||||
sessionTTL time.Duration
|
sessionTTL time.Duration
|
||||||
now func() time.Time
|
now func() time.Time
|
||||||
insecure bool // tests only: drop the Secure attribute so cookies flow over http
|
insecure bool // tests only: drop the Secure attribute so cookies flow over http
|
||||||
|
|
||||||
|
discoveryAttempts int
|
||||||
|
discoveryDelay time.Duration
|
||||||
}
|
}
|
||||||
|
|
||||||
var _ web.Auth = (*DexAuth)(nil)
|
var _ web.Auth = (*DexAuth)(nil)
|
||||||
@@ -96,14 +103,55 @@ func WithSessionTTL(ttl time.Duration) Option {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// WithDiscoveryRetry overrides the discovery attempt count and the initial
|
||||||
|
// back-off delay (doubled after each failed attempt).
|
||||||
|
func WithDiscoveryRetry(attempts int, delay time.Duration) Option {
|
||||||
|
return func(d *DexAuth) {
|
||||||
|
if attempts > 0 {
|
||||||
|
d.discoveryAttempts = attempts
|
||||||
|
}
|
||||||
|
if delay > 0 {
|
||||||
|
d.discoveryDelay = delay
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// WithInsecureCookies drops the Secure cookie attribute. TEST ONLY — it lets
|
// WithInsecureCookies drops the Secure cookie attribute. TEST ONLY — it lets
|
||||||
// the session cookie travel over plain http; never use it in production.
|
// the session cookie travel over plain http; never use it in production.
|
||||||
func WithInsecureCookies() Option {
|
func WithInsecureCookies() Option {
|
||||||
return func(d *DexAuth) { d.insecure = true }
|
return func(d *DexAuth) { d.insecure = true }
|
||||||
}
|
}
|
||||||
|
|
||||||
// New discovers the issuer and returns a configured DexAuth. ctx bounds the
|
// discover runs OIDC discovery up to attempts times, doubling delay between
|
||||||
// discovery request only.
|
// tries (#59: an issuer briefly unavailable behind its proxy answered 503 and
|
||||||
|
// serve exited). A cancelled ctx ends the back-off immediately with ctx's error.
|
||||||
|
func discover(ctx context.Context, issuer string, attempts int, delay time.Duration) (*oidc.Provider, error) {
|
||||||
|
var lastErr error
|
||||||
|
for i := range attempts {
|
||||||
|
if i > 0 {
|
||||||
|
t := time.NewTimer(delay)
|
||||||
|
select {
|
||||||
|
case <-ctx.Done():
|
||||||
|
t.Stop()
|
||||||
|
return nil, fmt.Errorf("after %d attempts (last: %v): %w", i, lastErr, ctx.Err())
|
||||||
|
case <-t.C:
|
||||||
|
}
|
||||||
|
delay *= 2
|
||||||
|
}
|
||||||
|
p, err := oidc.NewProvider(ctx, issuer)
|
||||||
|
if err == nil {
|
||||||
|
return p, nil
|
||||||
|
}
|
||||||
|
lastErr = err
|
||||||
|
if ctx.Err() != nil {
|
||||||
|
return nil, fmt.Errorf("after %d attempts: %w", i+1, ctx.Err())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return nil, fmt.Errorf("after %d attempts: %w", attempts, lastErr)
|
||||||
|
}
|
||||||
|
|
||||||
|
// New discovers the issuer and returns a configured DexAuth. ctx bounds discovery,
|
||||||
|
// including its retry back-off.
|
||||||
func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
||||||
for name, val := range map[string]string{
|
for name, val := range map[string]string{
|
||||||
"issuer": cfg.Issuer,
|
"issuer": cfg.Issuer,
|
||||||
@@ -117,29 +165,31 @@ func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
provider, err := oidc.NewProvider(ctx, cfg.Issuer)
|
|
||||||
if err != nil {
|
|
||||||
return nil, fmt.Errorf("oidc: discover issuer: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
d := &DexAuth{
|
d := &DexAuth{
|
||||||
cfg: cfg,
|
cfg: cfg,
|
||||||
verifier: provider.Verifier(&oidc.Config{ClientID: cfg.ClientID}),
|
pending: newPendingStore(),
|
||||||
oauth: &oauth2.Config{
|
secret: []byte(cfg.SessionSecret),
|
||||||
ClientID: cfg.ClientID,
|
sessionTTL: defaultSessionTTL,
|
||||||
ClientSecret: cfg.ClientSecret,
|
now: time.Now,
|
||||||
Endpoint: provider.Endpoint(),
|
discoveryAttempts: defaultDiscoveryAttempts,
|
||||||
RedirectURL: cfg.RedirectURL,
|
discoveryDelay: defaultDiscoveryDelay,
|
||||||
Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
|
|
||||||
},
|
|
||||||
pending: newPendingStore(),
|
|
||||||
secret: []byte(cfg.SessionSecret),
|
|
||||||
sessionTTL: defaultSessionTTL,
|
|
||||||
now: time.Now,
|
|
||||||
}
|
}
|
||||||
for _, o := range opts {
|
for _, o := range opts {
|
||||||
o(d)
|
o(d)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
provider, err := discover(ctx, cfg.Issuer, d.discoveryAttempts, d.discoveryDelay)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("oidc: discover issuer: %w", err)
|
||||||
|
}
|
||||||
|
d.verifier = provider.Verifier(&oidc.Config{ClientID: cfg.ClientID})
|
||||||
|
d.oauth = &oauth2.Config{
|
||||||
|
ClientID: cfg.ClientID,
|
||||||
|
ClientSecret: cfg.ClientSecret,
|
||||||
|
Endpoint: provider.Endpoint(),
|
||||||
|
RedirectURL: cfg.RedirectURL,
|
||||||
|
Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
|
||||||
|
}
|
||||||
return d, nil
|
return d, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"net/url"
|
"net/url"
|
||||||
|
"sync/atomic"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -34,6 +35,11 @@ type fakeIssuer struct {
|
|||||||
sub string
|
sub string
|
||||||
email string
|
email string
|
||||||
nonce string
|
nonce string
|
||||||
|
|
||||||
|
// discoveryHits counts requests to the discovery document; the first
|
||||||
|
// unavailableFor of them answer 503 (issuer briefly down behind its proxy).
|
||||||
|
discoveryHits atomic.Int32
|
||||||
|
unavailableFor int32
|
||||||
}
|
}
|
||||||
|
|
||||||
const testKID = "test-key"
|
const testKID = "test-key"
|
||||||
@@ -47,6 +53,10 @@ func newFakeIssuer(t *testing.T) *fakeIssuer {
|
|||||||
|
|
||||||
mux := http.NewServeMux()
|
mux := http.NewServeMux()
|
||||||
mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) {
|
mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) {
|
||||||
|
if f.discoveryHits.Add(1) <= f.unavailableFor {
|
||||||
|
http.Error(w, "no available server", http.StatusServiceUnavailable)
|
||||||
|
return
|
||||||
|
}
|
||||||
writeJSON(t, w, map[string]any{
|
writeJSON(t, w, map[string]any{
|
||||||
"issuer": f.server.URL,
|
"issuer": f.server.URL,
|
||||||
"authorization_endpoint": f.server.URL + "/authorize",
|
"authorization_endpoint": f.server.URL + "/authorize",
|
||||||
|
|||||||
+48
-1
@@ -84,7 +84,7 @@ func (a *App) handleUploadPost(w http.ResponseWriter, r *http.Request) {
|
|||||||
if !ok {
|
if !ok {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
r.Body = http.MaxBytesReader(w, r.Body, a.UploadLimits.MaxBytes+uploadOverhead)
|
r.Body = http.MaxBytesReader(w, a.idleBoundedBody(w, r.Body), a.UploadLimits.MaxBytes+uploadOverhead)
|
||||||
mr, err := r.MultipartReader()
|
mr, err := r.MultipartReader()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "Choose an audio file to upload.")
|
a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "Choose an audio file to upload.")
|
||||||
@@ -158,6 +158,53 @@ func (a *App) handleUploadPost(w http.ResponseWriter, r *http.Request) {
|
|||||||
a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "Choose an audio file to upload.")
|
a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "Choose an audio file to upload.")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// defaultUploadIdleTimeout is how long an upload body may deliver nothing before its
|
||||||
|
// connection read is cut off (#56). A stall this long is not a slow link but a dead or
|
||||||
|
// hostile one; a legitimate upload, however slow, keeps delivering bytes.
|
||||||
|
const defaultUploadIdleTimeout = 60 * time.Second
|
||||||
|
|
||||||
|
// uploadIdleTimeout is the idle bound this App applies: UploadIdleTimeout when set, else
|
||||||
|
// defaultUploadIdleTimeout. cmd/tapir leaves it unset, so production runs the default.
|
||||||
|
func (a *App) uploadIdleTimeout() time.Duration {
|
||||||
|
if a.UploadIdleTimeout <= 0 {
|
||||||
|
return defaultUploadIdleTimeout
|
||||||
|
}
|
||||||
|
return a.UploadIdleTimeout
|
||||||
|
}
|
||||||
|
|
||||||
|
// idleBoundedBody bounds the SILENCE in an upload body, not its total duration (#56): every
|
||||||
|
// read first pushes the connection's read deadline idle into the future, so a body that
|
||||||
|
// keeps moving is never cut, and one that stops fails its read with a timeout. That read
|
||||||
|
// error reaches Accept as a client read error, so Accept returns and its receive slot is
|
||||||
|
// released. A whole-request ReadTimeout would instead cut a large upload on a slow link.
|
||||||
|
//
|
||||||
|
// If the writer cannot set a read deadline (a wrapper without Unwrap, or a test recorder)
|
||||||
|
// the body is returned unbounded and that is logged: a control that silently does not
|
||||||
|
// apply is worse than one that says so.
|
||||||
|
func (a *App) idleBoundedBody(w http.ResponseWriter, body io.ReadCloser) io.ReadCloser {
|
||||||
|
idle := a.uploadIdleTimeout()
|
||||||
|
rc := http.NewResponseController(w)
|
||||||
|
if err := rc.SetReadDeadline(time.Now().Add(idle)); err != nil {
|
||||||
|
a.logger().Warn("upload idle bound unavailable", "err", err)
|
||||||
|
return body
|
||||||
|
}
|
||||||
|
return &idleDeadlineBody{ReadCloser: body, rc: rc, idle: idle}
|
||||||
|
}
|
||||||
|
|
||||||
|
type idleDeadlineBody struct {
|
||||||
|
io.ReadCloser
|
||||||
|
rc *http.ResponseController
|
||||||
|
idle time.Duration
|
||||||
|
}
|
||||||
|
|
||||||
|
func (b *idleDeadlineBody) Read(p []byte) (int, error) {
|
||||||
|
_ = b.rc.SetReadDeadline(time.Now().Add(b.idle))
|
||||||
|
// Never cleared here: net/http clears the read deadline itself before its background
|
||||||
|
// read once the handler has the body, and an armed deadline after an error also bounds
|
||||||
|
// net/http's drain of an unread remainder before it replies.
|
||||||
|
return b.ReadCloser.Read(p)
|
||||||
|
}
|
||||||
|
|
||||||
// abandoned reports, logs at INFO and answers 499, because the client has already gone: its request
|
// abandoned reports, logs at INFO and answers 499, because the client has already gone: its request
|
||||||
// context is cancelled, so there is nobody to answer and rendering a page would run its
|
// context is cancelled, so there is nobody to answer and rendering a page would run its
|
||||||
// database read on a dead context and log a false ERROR. A hang-up mid-upload is routine.
|
// database read on a dead context and log a false ERROR. A hang-up mid-upload is routine.
|
||||||
|
|||||||
@@ -0,0 +1,161 @@
|
|||||||
|
package web_test
|
||||||
|
|
||||||
|
// #56: a request body that stops arriving must not hold a receive slot forever. The bound is a
|
||||||
|
// read deadline on the connection, re-armed on every read, so it only exercises over a real
|
||||||
|
// socket and through the production handler stack (metrics.HTTPMiddleware wraps the writer).
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bufio"
|
||||||
|
"bytes"
|
||||||
|
"errors"
|
||||||
|
"fmt"
|
||||||
|
"log/slog"
|
||||||
|
"mime/multipart"
|
||||||
|
"net"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"git.d-ma.be/mathias/tapir/internal/metrics"
|
||||||
|
"git.d-ma.be/mathias/tapir/internal/uploads"
|
||||||
|
"git.d-ma.be/mathias/tapir/internal/web"
|
||||||
|
)
|
||||||
|
|
||||||
|
func multipartBody(t *testing.T, content []byte) (body []byte, contentType string) {
|
||||||
|
t.Helper()
|
||||||
|
var buf bytes.Buffer
|
||||||
|
mw := multipart.NewWriter(&buf)
|
||||||
|
fw, err := mw.CreateFormFile("file", "a.m4a")
|
||||||
|
require.NoError(t, err)
|
||||||
|
_, err = fw.Write(content)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NoError(t, mw.Close())
|
||||||
|
return buf.Bytes(), mw.FormDataContentType()
|
||||||
|
}
|
||||||
|
|
||||||
|
// rawUpload opens a connection and sends the request headers; the caller feeds the body.
|
||||||
|
func rawUpload(t *testing.T, addr, contentType string, length int) net.Conn {
|
||||||
|
t.Helper()
|
||||||
|
c, err := net.Dial("tcp", addr)
|
||||||
|
require.NoError(t, err)
|
||||||
|
t.Cleanup(func() { _ = c.Close() })
|
||||||
|
_, err = fmt.Fprintf(c, "POST /uploads HTTP/1.1\r\nHost: x\r\nContent-Type: %s\r\nContent-Length: %d\r\n\r\n", contentType, length)
|
||||||
|
require.NoError(t, err)
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
|
// serverEnded reports whether the server ended the request (answered it or closed the
|
||||||
|
// connection) before wait elapsed, rather than leaving the client waiting.
|
||||||
|
func serverEnded(t *testing.T, c net.Conn, wait time.Duration) bool {
|
||||||
|
t.Helper()
|
||||||
|
require.NoError(t, c.SetReadDeadline(time.Now().Add(wait)))
|
||||||
|
_, err := http.ReadResponse(bufio.NewReader(c), nil)
|
||||||
|
var ne net.Error
|
||||||
|
return !errors.As(err, &ne) || !ne.Timeout()
|
||||||
|
}
|
||||||
|
|
||||||
|
// readStatus returns the response status code, or 0 if none arrived before wait elapsed.
|
||||||
|
func readStatus(t *testing.T, c net.Conn, wait time.Duration) int {
|
||||||
|
t.Helper()
|
||||||
|
require.NoError(t, c.SetReadDeadline(time.Now().Add(wait)))
|
||||||
|
resp, err := http.ReadResponse(bufio.NewReader(c), nil)
|
||||||
|
if err != nil {
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
_ = resp.Body.Close()
|
||||||
|
return resp.StatusCode
|
||||||
|
}
|
||||||
|
|
||||||
|
func newIdleBoundServer(t *testing.T, idle time.Duration) (addr string) {
|
||||||
|
t.Helper()
|
||||||
|
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||||
|
app.UploadIdleTimeout = idle
|
||||||
|
app.Uploads.(*uploads.Intake).MaxReceivesPerUser = 1
|
||||||
|
srv := httptest.NewServer(metrics.HTTPMiddleware(app.Router()))
|
||||||
|
t.Cleanup(srv.Close)
|
||||||
|
return strings.TrimPrefix(srv.URL, "http://")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUploadPost_AConnectionReadDeadlineEndsABodyThatStallsAndReleasesItsReceiveSlot(t *testing.T) {
|
||||||
|
addr := newIdleBoundServer(t, 200*time.Millisecond)
|
||||||
|
body, ct := multipartBody(t, bytes.Repeat([]byte("a"), 8000))
|
||||||
|
|
||||||
|
stalled := rawUpload(t, addr, ct, len(body))
|
||||||
|
_, err := stalled.Write(body[:4000]) // half the body, then nothing more
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
require.True(t, serverEnded(t, stalled, 3*time.Second),
|
||||||
|
"the server never ended the stalled request: it still holds its slot")
|
||||||
|
|
||||||
|
next := rawUpload(t, addr, ct, len(body))
|
||||||
|
_, err = next.Write(body)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, http.StatusSeeOther, readStatus(t, next, 3*time.Second),
|
||||||
|
"the same user's next upload gets the slot the stalled one held")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUploadPost_TheReadDeadlineIsReArmedOnEveryReadSoASlowButMovingBodyIsAccepted(t *testing.T) {
|
||||||
|
addr := newIdleBoundServer(t, 300*time.Millisecond)
|
||||||
|
body, ct := multipartBody(t, bytes.Repeat([]byte("a"), 2000))
|
||||||
|
|
||||||
|
c := rawUpload(t, addr, ct, len(body))
|
||||||
|
// ~1.2 s in total, four times the idle bound, but never 300 ms without a byte.
|
||||||
|
const chunks = 20
|
||||||
|
step := len(body)/chunks + 1
|
||||||
|
for i := 0; i < len(body); i += step {
|
||||||
|
_, err := c.Write(body[i:min(i+step, len(body))])
|
||||||
|
require.NoError(t, err)
|
||||||
|
time.Sleep(60 * time.Millisecond)
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusSeeOther, readStatus(t, c, 3*time.Second))
|
||||||
|
}
|
||||||
|
|
||||||
|
// cmd/tapir never sets UploadIdleTimeout, so the default is the value production runs.
|
||||||
|
// Shorter and legitimate uploads on a slow link are cut; much longer and the bound is moot.
|
||||||
|
func TestDefaultUploadIdleTimeoutIsSixtySeconds(t *testing.T) {
|
||||||
|
require.Equal(t, 60*time.Second, web.DefaultUploadIdleTimeout)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The bound an App actually resolves to, not the constant: production's App leaves
|
||||||
|
// UploadIdleTimeout unset, so the zero value must resolve to exactly 60 s, and a set value
|
||||||
|
// must be used as given.
|
||||||
|
func TestAppUploadIdleTimeout_ResolvesUnsetToSixtySecondsAndHonoursASetValue(t *testing.T) {
|
||||||
|
require.Equal(t, 60*time.Second, (&web.App{}).UploadIdleTimeoutForTest())
|
||||||
|
require.Equal(t, 60*time.Second, (&web.App{UploadIdleTimeout: -time.Second}).UploadIdleTimeoutForTest())
|
||||||
|
require.Equal(t, 300*time.Millisecond, (&web.App{UploadIdleTimeout: 300 * time.Millisecond}).UploadIdleTimeoutForTest())
|
||||||
|
}
|
||||||
|
|
||||||
|
// With UploadIdleTimeout unset, a body with 100 ms gaps between reads is not cut (a resolved
|
||||||
|
// bound in milliseconds or nanoseconds would cut it). The exact value is pinned above.
|
||||||
|
func TestUploadPost_AnUnsetIdleTimeoutDoesNotCutABodyWithSubSecondGaps(t *testing.T) {
|
||||||
|
addr := newIdleBoundServer(t, 0)
|
||||||
|
body, ct := multipartBody(t, bytes.Repeat([]byte("a"), 1000))
|
||||||
|
|
||||||
|
c := rawUpload(t, addr, ct, len(body))
|
||||||
|
const chunks = 5
|
||||||
|
step := len(body)/chunks + 1
|
||||||
|
for i := 0; i < len(body); i += step {
|
||||||
|
time.Sleep(100 * time.Millisecond)
|
||||||
|
_, err := c.Write(body[i:min(i+step, len(body))])
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusSeeOther, readStatus(t, c, 3*time.Second))
|
||||||
|
}
|
||||||
|
|
||||||
|
// A writer that cannot set a read deadline leaves the body unbounded; that must be said in
|
||||||
|
// the log, not happen silently. The in-process recorder is such a writer.
|
||||||
|
func TestUploadPost_AWriterWithoutReadDeadlineSupportLogsThatTheIdleBoundIsUnavailable(t *testing.T) {
|
||||||
|
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||||
|
buf := &lockedBuf{}
|
||||||
|
app.Log = slog.New(slog.NewTextHandler(buf, nil))
|
||||||
|
|
||||||
|
do(t, app, uploadReq(t, "t", "a.m4a", []byte("x")))
|
||||||
|
|
||||||
|
require.Contains(t, buf.String(), "upload idle bound unavailable")
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user