From 99855bd3a2136d07b8dc1a8d39cf3e96d70cf2ad Mon Sep 17 00:00:00 2001 From: aler9 <46489434+aler9@users.noreply.github.com> Date: Fri, 9 Dec 2022 12:45:51 +0100 Subject: [PATCH] allow invalid RTCP packets in both client and server (https://github.com/aler9/rtsp-simple-server/issues/1085) Invalid RTCP packets, in both server and client, do not trigger a fatal error anymore but they're just blocked. OnDecodeError() is called in order to emit a warning. --- client.go | 5 ++--- client_publish_test.go | 13 ++++++++++++- server_publish_test.go | 11 +++++++++++ server_read_test.go | 11 +++++++++++ serverconn.go | 6 ++++-- 5 files changed, 40 insertions(+), 6 deletions(-) diff --git a/client.go b/client.go index 03df4f7d..129297d6 100644 --- a/client.go +++ b/client.go @@ -825,8 +825,6 @@ func (c *Client) runReader() { packets, err := rtcp.Unmarshal(payload) if err != nil { - // some cameras send invalid RTCP packets. - // ignore them. c.OnDecodeError(err) return nil } @@ -852,7 +850,8 @@ func (c *Client) runReader() { packets, err := rtcp.Unmarshal(payload) if err != nil { - return err + c.OnDecodeError(err) + return nil } for _, pkt := range packets { diff --git a/client_publish_test.go b/client_publish_test.go index 70941713..b81ebce6 100644 --- a/client_publish_test.go +++ b/client_publish_test.go @@ -846,6 +846,7 @@ func TestClientPublishDecodeErrors(t *testing.T) { }{ {"udp", "rtcp invalid"}, {"udp", "rtcp too big"}, + {"tcp", "rtcp invalid"}, {"tcp", "rtcp too big"}, } { t.Run(ca.proto+" "+ca.name, func(t *testing.T) { @@ -944,7 +945,7 @@ func TestClientPublishDecodeErrors(t *testing.T) { }) require.NoError(t, err) - switch { + switch { //nolint:dupl case ca.proto == "udp" && ca.name == "rtcp invalid": l2.WriteTo([]byte{0x01, 0x02}, &net.UDPAddr{ IP: net.ParseIP("127.0.0.1"), @@ -957,6 +958,13 @@ func TestClientPublishDecodeErrors(t *testing.T) { Port: th.ClientPorts[1], }) + case ca.proto == "tcp" && ca.name == "rtcp invalid": + err = conn.WriteInterleavedFrame(&base.InterleavedFrame{ + Channel: 1, + Payload: []byte{0x01, 0x02}, + }, make([]byte, 2048)) + require.NoError(t, err) + case ca.proto == "tcp" && ca.name == "rtcp too big": err = conn.WriteInterleavedFrame(&base.InterleavedFrame{ Channel: 1, @@ -992,6 +1000,9 @@ func TestClientPublishDecodeErrors(t *testing.T) { case ca.proto == "udp" && ca.name == "rtcp too big": require.EqualError(t, err, "RTCP packet is too big to be read with UDP") + case ca.proto == "tcp" && ca.name == "rtcp invalid": + require.EqualError(t, err, "rtcp: packet too short") + case ca.proto == "tcp" && ca.name == "rtcp too big": require.EqualError(t, err, "RTCP packet size (2000) is greater than maximum allowed (1472)") } diff --git a/server_publish_test.go b/server_publish_test.go index 8da1ddb1..0e395d6f 100644 --- a/server_publish_test.go +++ b/server_publish_test.go @@ -1494,6 +1494,7 @@ func TestServerPublishDecodeErrors(t *testing.T) { {"udp", "rtp packets lost"}, {"udp", "rtp too big"}, {"udp", "rtcp too big"}, + {"tcp", "rtcp invalid"}, {"tcp", "rtcp too big"}, } { t.Run(ca.proto+" "+ca.name, func(t *testing.T) { @@ -1533,6 +1534,9 @@ func TestServerPublishDecodeErrors(t *testing.T) { case ca.proto == "udp" && ca.name == "rtcp too big": require.EqualError(t, ctx.Error, "RTCP packet is too big to be read with UDP") + case ca.proto == "tcp" && ca.name == "rtcp invalid": + require.EqualError(t, ctx.Error, "rtcp: packet too short") + case ca.proto == "tcp" && ca.name == "rtcp too big": require.EqualError(t, ctx.Error, "RTCP packet size (2000) is greater than maximum allowed (1472)") } @@ -1682,6 +1686,13 @@ func TestServerPublishDecodeErrors(t *testing.T) { Port: resTH.ServerPorts[1], }) + case ca.proto == "tcp" && ca.name == "rtcp invalid": + err = conn.WriteInterleavedFrame(&base.InterleavedFrame{ + Channel: 1, + Payload: []byte{0x01, 0x02}, + }, make([]byte, 2048)) + require.NoError(t, err) + case ca.proto == "tcp" && ca.name == "rtcp too big": err = conn.WriteInterleavedFrame(&base.InterleavedFrame{ Channel: 1, diff --git a/server_read_test.go b/server_read_test.go index 624dee2c..4d185944 100644 --- a/server_read_test.go +++ b/server_read_test.go @@ -724,6 +724,7 @@ func TestServerReadDecodeErrors(t *testing.T) { }{ {"udp", "rtcp invalid"}, {"udp", "rtcp too big"}, + {"tcp", "rtcp invalid"}, {"tcp", "rtcp too big"}, } { t.Run(ca.proto+" "+ca.name, func(t *testing.T) { @@ -759,6 +760,9 @@ func TestServerReadDecodeErrors(t *testing.T) { case ca.proto == "udp" && ca.name == "rtcp too big": require.EqualError(t, ctx.Error, "RTCP packet is too big to be read with UDP") + case ca.proto == "tcp" && ca.name == "rtcp invalid": + require.EqualError(t, ctx.Error, "rtcp: packet too short") + case ca.proto == "tcp" && ca.name == "rtcp too big": require.EqualError(t, ctx.Error, "RTCP packet size (2000) is greater than maximum allowed (1472)") } @@ -854,6 +858,13 @@ func TestServerReadDecodeErrors(t *testing.T) { Port: resTH.ServerPorts[1], }) + case ca.proto == "tcp" && ca.name == "rtcp invalid": + err = conn.WriteInterleavedFrame(&base.InterleavedFrame{ + Channel: 1, + Payload: []byte{0x01, 0x02}, + }, make([]byte, 2048)) + require.NoError(t, err) + case ca.proto == "tcp" && ca.name == "rtcp too big": err = conn.WriteInterleavedFrame(&base.InterleavedFrame{ Channel: 1, diff --git a/serverconn.go b/serverconn.go index e742eab3..be76210b 100644 --- a/serverconn.go +++ b/serverconn.go @@ -251,7 +251,8 @@ func (sc *ServerConn) readFuncTCP(readRequest chan readReq) error { packets, err := rtcp.Unmarshal(payload) if err != nil { - return err + onDecodeError(sc.session, err) + return nil } if h, ok := sc.s.Handler.(ServerHandlerOnPacketRTCP); ok { @@ -294,7 +295,8 @@ func (sc *ServerConn) readFuncTCP(readRequest chan readReq) error { packets, err := rtcp.Unmarshal(payload) if err != nil { - return err + onDecodeError(sc.session, err) + return nil } for _, pkt := range packets {