Skip to content

Commit

Permalink
server: fix crash with invalid SETUP request (bluenviron/mediamtx#4025)…
Browse files Browse the repository at this point in the history 
… (#652)
  • Loading branch information
@aler9
aler9 authored Dec 8, 2024
1 parent fb61e9a commit db334b3
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 39 deletions.
8 changes: 4 additions & 4 deletions pkg/liberrors/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,12 +261,12 @@ func (e ErrServerStreamClosed) Error() string {
return "stream is closed"
}

// ErrServerPathNoSlash is an error that can be returned by a server.
type ErrServerPathNoSlash struct{}
// ErrServerInvalidSetupPath is an error that can be returned by a server.
type ErrServerInvalidSetupPath struct{}

// Error implements the error interface.
func (ErrServerPathNoSlash) Error() string {
return "path of a SETUP request must end with a slash. " +
func (ErrServerInvalidSetupPath) Error() string {
return "invalid SETUP path. " +
"This typically happens when VLC fails a request, and then switches to an " +
"unsupported RTSP dialect"
}
113 changes: 80 additions & 33 deletions server_play_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,38 +174,38 @@ func TestServerPlayPath(t *testing.T) {
path string
}{
{
"normal",
"standard",
"rtsp://localhost:8554/teststream[control]",
"rtsp://localhost:8554/teststream/",
"/teststream",
},
{
"with query, ffmpeg format",
"no query, ffmpeg format",
"rtsp://localhost:8554/teststream?testing=123[control]",
"rtsp://localhost:8554/teststream/",
"/teststream",
},
{
"with query, gstreamer format",
"no query, gstreamer format",
"rtsp://localhost:8554/teststream[control]?testing=123",
"rtsp://localhost:8554/teststream/",
"/teststream",
},
{
// this is needed to support reading mpegts with ffmpeg
"without media id",
"no control",
"rtsp://localhost:8554/teststream/",
"rtsp://localhost:8554/teststream/",
"/teststream",
},
{
"without media id, query, ffmpeg",
"no control, query, ffmpeg",
"rtsp://localhost:8554/teststream?testing=123/",
"rtsp://localhost:8554/teststream/",
"/teststream",
},
{
"without media id, query, gstreamer",
"no control, query, gstreamer",
"rtsp://localhost:8554/teststream/?testing=123",
"rtsp://localhost:8554/teststream/",
"/teststream",
Expand All @@ -217,7 +217,7 @@ func TestServerPlayPath(t *testing.T) {
"/test/stream",
},
{
"subpath without media id",
"subpath, no control",
"rtsp://localhost:8554/test/stream/",
"rtsp://localhost:8554/test/stream/",
"/test/stream",
Expand All @@ -235,7 +235,7 @@ func TestServerPlayPath(t *testing.T) {
"/test/stream",
},
{
"no slash",
"no path, no slash",
"rtsp://localhost:8554[control]",
"rtsp://localhost:8554",
"",
Expand All @@ -246,6 +246,18 @@ func TestServerPlayPath(t *testing.T) {
"rtsp://localhost:8554//",
"/",
},
{
"no control, no path",
"rtsp://localhost:8554/",
"rtsp://localhost:8554/",
"/",
},
{
"no control, no path, no slash",
"rtsp://localhost:8554",
"rtsp://localhost:8554",
"",
},
} {
t.Run(ca.name, func(t *testing.T) {
var stream *ServerStream
Expand Down Expand Up @@ -316,9 +328,10 @@ func TestServerPlayPath(t *testing.T) {

func TestServerPlaySetupErrors(t *testing.T) {
for _, ca := range []string{
"invalid path",
"closed stream",
"different paths",
"double setup",
"closed stream",
"different protocols",
} {
t.Run(ca, func(t *testing.T) {
Expand All @@ -329,15 +342,19 @@ func TestServerPlaySetupErrors(t *testing.T) {
Handler: &testServerHandler{
onConnClose: func(ctx *ServerHandlerOnConnCloseCtx) {
switch ca {
case "invalid path":
require.EqualError(t, ctx.Error, "invalid SETUP path. "+
"This typically happens when VLC fails a request, and then switches to an unsupported RTSP dialect")

case "closed stream":
require.EqualError(t, ctx.Error, "stream is closed")

case "different paths":
require.EqualError(t, ctx.Error, "can't setup medias with different paths")

case "double setup":
require.EqualError(t, ctx.Error, "media has already been setup")

case "closed stream":
require.EqualError(t, ctx.Error, "stream is closed")

case "different protocols":
require.EqualError(t, ctx.Error, "can't setup medias with different protocols")
}
Expand Down Expand Up @@ -375,30 +392,64 @@ func TestServerPlaySetupErrors(t *testing.T) {
defer nconn.Close()
conn := conn.NewConn(nconn)

desc := doDescribe(t, conn)
var desc *description.Session
var th *headers.Transport
var res *base.Response

th := &headers.Transport{
Protocol: headers.TransportProtocolUDP,
Delivery: deliveryPtr(headers.TransportDeliveryUnicast),
Mode: transportModePtr(headers.TransportModePlay),
ClientPorts: &[2]int{35466, 35467},
}
switch ca {
case "invalid path":
th = &headers.Transport{
Protocol: headers.TransportProtocolUDP,
Delivery: deliveryPtr(headers.TransportDeliveryUnicast),
Mode: transportModePtr(headers.TransportModePlay),
ClientPorts: &[2]int{35466, 35467},
}

res, err := writeReqReadRes(conn, base.Request{
Method: base.Setup,
URL: mediaURL(t, desc.BaseURL, desc.Medias[0]),
Header: base.Header{
"CSeq": base.HeaderValue{"2"},
"Transport": th.Marshal(),
},
})
res, err = writeReqReadRes(conn, base.Request{
Method: base.Setup,
URL: &base.URL{
Scheme: "rtsp",
Path: "/invalid",
},
Header: base.Header{
"CSeq": base.HeaderValue{"2"},
"Transport": th.Marshal(),
},
})

if ca != "closed stream" {
require.NoError(t, err)
require.Equal(t, base.StatusOK, res.StatusCode)
require.Equal(t, base.StatusBadRequest, res.StatusCode)

default:
desc = doDescribe(t, conn)

th = &headers.Transport{
Protocol: headers.TransportProtocolUDP,
Delivery: deliveryPtr(headers.TransportDeliveryUnicast),
Mode: transportModePtr(headers.TransportModePlay),
ClientPorts: &[2]int{35466, 35467},
}

res, err = writeReqReadRes(conn, base.Request{
Method: base.Setup,
URL: mediaURL(t, desc.BaseURL, desc.Medias[0]),
Header: base.Header{
"CSeq": base.HeaderValue{"2"},
"Transport": th.Marshal(),
},
})

if ca != "closed stream" {
require.NoError(t, err)
require.Equal(t, base.StatusOK, res.StatusCode)
}
}

switch ca {
case "closed stream":
require.NoError(t, err)
require.Equal(t, base.StatusBadRequest, res.StatusCode)

case "different paths":
session := readSession(t, res)

Expand Down Expand Up @@ -433,10 +484,6 @@ func TestServerPlaySetupErrors(t *testing.T) {
require.NoError(t, err)
require.Equal(t, base.StatusBadRequest, res.StatusCode)

case "closed stream":
require.NoError(t, err)
require.Equal(t, base.StatusBadRequest, res.StatusCode)

case "different protocols":
session := readSession(t, res)

Expand Down
10 changes: 8 additions & 2 deletions server_session.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,17 @@ func getPathAndQueryAndTrackID(u *base.URL) (string, string, string, error) {
if strings.HasSuffix(u.RawQuery, "/") {
return u.Path, u.RawQuery[:len(u.RawQuery)-1], "0", nil
}
if strings.HasSuffix(u.Path[1:], "/") {
if len(u.Path) >= 1 && strings.HasSuffix(u.Path[1:], "/") {
return u.Path[:len(u.Path)-1], u.RawQuery, "0", nil
}

return "", "", "", liberrors.ErrServerPathNoSlash{}
// special case for empty path
if u.Path == "" || u.Path == "/" {
return u.Path, u.RawQuery, "0", nil
}

// no slash at the end of the path.
return "", "", "", liberrors.ErrServerInvalidSetupPath{}
}

// used for SETUP when recording
Expand Down

0 comments on commit db334b3

Please sign in to comment.