diff --git a/internal/utils/utils.go b/internal/utils/utils.go index c031c0e59..fe32a4143 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -69,11 +69,12 @@ func ApiCORS(ctx context.Context, w http.ResponseWriter, r *http.Request) bool { // ParseBody read the body from r, and unmarshal JSON to v. func ParseBody(r io.ReadCloser, v interface{}) error { + defer r.Close() + b, err := io.ReadAll(r) if err != nil { return errors.Wrapf(err, "read body") } - defer r.Close() if len(b) == 0 { return nil @@ -97,7 +98,7 @@ func BuildStreamURL(r string) (string, error) { defaultVhost := !strings.Contains(u.Hostname(), ".") // If hostname is actually an IP address, it's __defaultVhost__. - if ip := net.ParseIP(u.Hostname()); ip.To4() != nil { + if ip := net.ParseIP(u.Hostname()); ip != nil && ip.To4() != nil { defaultVhost = true } diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index bab316628..e67b7e12b 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -121,6 +121,19 @@ func TestParseBody_ReadError(t *testing.T) { } } +// TestParseBody_CloseCalledOnReadError verifies that r.Close() is called even +// when ReadAll fails - this prevents the resource leak fixed by moving defer +// to the top of the function. +func TestParseBody_CloseCalledOnReadError(t *testing.T) { + rc := &errReadCloser{} + if err := ParseBody(rc, &struct{}{}); err == nil { + t.Fatal("want error") + } + if !rc.closed { + t.Fatal("Close() was not called on read error - resource leak") + } +} + func TestParseBody_UnmarshalError(t *testing.T) { var v struct{ Name string } err := ParseBody(io.NopCloser(strings.NewReader("not json")), &v) @@ -136,11 +149,20 @@ func TestBuildStreamURL(t *testing.T) { cases := []struct { in, want string }{ + // Domain names with dots use hostname as vhost. + // This case would panic with nil pointer dereference before the fix + // because net.ParseIP("example.com") returns nil and nil.To4() panics. {"rtmp://example.com/live/stream", "example.com/live/stream"}, {"rtmp://example.com:1935/live/stream", "example.com/live/stream"}, + // IPv4 addresses use defaultVhost. {"rtmp://127.0.0.1/live/stream", "__defaultVhost__/live/stream"}, + // Hostnames without dots use defaultVhost. {"rtmp://localhost/live/stream", "__defaultVhost__/live/stream"}, {"rtmp://localhost:1935/live/stream", "__defaultVhost__/live/stream"}, + // IPv6 addresses: net.ParseIP returns non-nil but To4() returns nil, + // but they still get defaultVhost because they contain no dots. + {"rtmp://[::1]/live/stream", "__defaultVhost__/live/stream"}, + {"rtmp://[2001:db8::1]:1935/live/stream", "__defaultVhost__/live/stream"}, } for _, c := range cases { got, err := BuildStreamURL(c.in)