Repository: bluenviron/gortsplib
Pull request #458 (2023-11-05 10:58:26)

fix nil url deref when handling invalid requests
This fixes the following panic when handling certain invalid requests:

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x50 pc=0x5601e7562614]
goroutine 206 [running]:
github.com/bluenviron/gortsplib/v4/pkg/url.(*URL).RTSPPathAndQuery(...)
        github.com/bluenviron/gortsplib/v4@v4.3.0/pkg/url/url.go:81
github.com/bluenviron/gortsplib/v4.(*ServerConn).handleRequestInner(0xc0001a8200, 0xc00064c7c0)
        github.com/bluenviron/gortsplib/v4@v4.3.0/server_conn.go:221 +0x174
github.com/bluenviron/gortsplib/v4.(*ServerConn).handleRequestOuter(0xc0001a8200, 0xc00064c7c0)
        github.com/bluenviron/gortsplib/v4@v4.3.0/server_conn.go:382 +0x67
github.com/bluenviron/gortsplib/v4.(*ServerConn).runInner(0xc0001a8200)
        github.com/bluenviron/gortsplib/v4@v4.3.0/server_conn.go:191 +0x172
github.com/bluenviron/gortsplib/v4.(*ServerConn).run(0xc0001a8200)
        github.com/bluenviron/gortsplib/v4@v4.3.0/server_conn.go:165 +0x348
created by github.com/bluenviron/gortsplib/v4.newServerConn in goroutine 7
        github.com/bluenviron/gortsplib/v4@v4.3.0/server_conn.go:109 +0x3d6
```

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/bluenviron/gortsplib/pull/458?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) Report
> Merging [#458](https://app.codecov.io/gh/bluenviron/gortsplib/pull/458?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) (e189695) into [main](https://app.codecov.io/gh/bluenviron/gortsplib/commit/0933bf997556ccd22bf62e1f7d25552d7525244d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) (0933bf9) will **decrease** coverage by `0.03%`.
> The diff coverage is `0.00%`.

```diff
@@            Coverage Diff             @@
##             main     #458      +/-   ##
==========================================
- Coverage   84.55%   84.52%   -0.03%     
==========================================
  Files         106      106              
  Lines       10921    10926       +5     
==========================================
+ Hits         9234     9235       +1     
- Misses       1291     1295       +4     
  Partials      396      396              
```


| [Files](https://app.codecov.io/gh/bluenviron/gortsplib/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) | Coverage Δ | |
|---|---|---|
| [server\_conn.go](https://app.codecov.io/gh/bluenviron/gortsplib/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron#diff-c2VydmVyX2Nvbm4uZ28=) | `89.58% <0.00%> (-1.36%)` | :arrow_down: |

... and [3 files with indirect coverage changes](https://app.codecov.io/gh/bluenviron/gortsplib/pull/458/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron)

:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in [Chrome](https://chrome.google.com/webstore/detail/codecov/gedikamndpbemklijjkncpnolildpbgo) or [Firefox](https://addons.mozilla.org/en-US/firefox/addon/codecov/) today!

aler9: Hello, thanks for reporting the issue, i moved the check a little above in order to cover all cases, and i added tests.

Patch:
diff --git a/server_conn.go b/server_conn.go
index eac113bf..35e65064 100644
--- a/server_conn.go
+++ b/server_conn.go
@@ -208,10 +208,15 @@ func (sc *ServerConn) handleRequestInner(req *base.Request) (*base.Response, err
 	if cseq, ok := req.Header["CSeq"]; !ok || len(cseq) != 1 {
 		return &base.Response{
 			StatusCode: base.StatusBadRequest,
-			Header:     base.Header{},
 		}, liberrors.ErrServerCSeqMissing{}
 	}
 
+	if req.Method != base.Options && req.URL == nil {
+		return &base.Response{
+			StatusCode: base.StatusBadRequest,
+		}, liberrors.ErrServerInvalidPath{}
+	}
+
 	sxID := getSessionID(req.Header)
 
 	var path string


Test patch:
diff --git a/server_test.go b/server_test.go
index e8445f3c..4f0a4234 100644
--- a/server_test.go
+++ b/server_test.go
@@ -311,7 +311,40 @@ func TestServerErrorCSeqMissing(t *testing.T) {
 	res, err := writeReqReadRes(conn, base.Request{
 		Method: base.Options,
 		URL:    mustParseURL("rtsp://localhost:8554/"),
-		Header: base.Header{},
+	})
+	require.NoError(t, err)
+	require.Equal(t, base.StatusBadRequest, res.StatusCode)
+
+	<-nconnClosed
+}
+
+func TestServerErrorNilURL(t *testing.T) {
+	nconnClosed := make(chan struct{})
+
+	s := &Server{
+		Handler: &testServerHandler{
+			onConnClose: func(ctx *ServerHandlerOnConnCloseCtx) {
+				require.EqualError(t, ctx.Error, "invalid path")
+				close(nconnClosed)
+			},
+		},
+		RTSPAddress: "localhost:8554",
+	}
+	err := s.Start()
+	require.NoError(t, err)
+	defer s.Close()
+
+	nconn, err := net.Dial("tcp", "localhost:8554")
+	require.NoError(t, err)
+	defer nconn.Close()
+	conn := conn.NewConn(nconn)
+
+	res, err := writeReqReadRes(conn, base.Request{
+		Method: base.Describe,
+		URL:    nil,
+		Header: base.Header{
+			"CSeq": base.HeaderValue{"1"},
+		},
 	})
 	require.NoError(t, err)
 	require.Equal(t, base.StatusBadRequest, res.StatusCode)