Repository: bluenviron/gortsplib
Pull request #652 (2024-12-08 16:57:35)

server: fix crash with invalid SETUP request (bluenviron/mediamtx#4025)
Fixes bluenviron/mediamtx#4025

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/bluenviron/gortsplib/pull/652?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 82.40%. Comparing base [(`d7efbd0`)](https://app.codecov.io/gh/bluenviron/gortsplib/commit/d7efbd0a482b442039f9e2e741650e7357721911?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) to head [(`b256c8a`)](https://app.codecov.io/gh/bluenviron/gortsplib/commit/b256c8ab28220905f919ec1142c2ce1fbd2107e1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron).
> Report is 3 commits behind head on main.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #652      +/-   ##
==========================================
+ Coverage   82.31%   82.40%   +0.08%     
==========================================
  Files         116      116              
  Lines       11784    11793       +9     
==========================================
+ Hits         9700     9718      +18     
+ Misses       1696     1689       -7     
+ Partials      388      386       -2     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/bluenviron/gortsplib/pull/652?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron).


Patch:
diff --git a/pkg/liberrors/server.go b/pkg/liberrors/server.go
index 22ab635b..eb358851 100644
--- a/pkg/liberrors/server.go
+++ b/pkg/liberrors/server.go
@@ -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"
 }
diff --git a/server_session.go b/server_session.go
index 5f90a1ed..1ca8807d 100644
--- a/server_session.go
+++ b/server_session.go
@@ -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


Test patch:
diff --git a/server_play_test.go b/server_play_test.go
index 5305ae54..fb07a70f 100644
--- a/server_play_test.go
+++ b/server_play_test.go
@@ -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",
@@ -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",
@@ -235,7 +235,7 @@ func TestServerPlayPath(t *testing.T) {
 			"/test/stream",
 		},
 		{
-			"no slash",
+			"no path, no slash",
 			"rtsp://localhost:8554[control]",
 			"rtsp://localhost:8554",
 			"",
@@ -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
@@ -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) {
@@ -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")
 						}
@@ -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)
 
@@ -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)