Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All SIP messages of a TCP stream are ignored if that TCP stream contains a partial SIP message #145

Open
ayonpals opened this issue Oct 19, 2024 · 4 comments

Comments

@ayonpals
Copy link

ayonpals commented Oct 19, 2024

If I pump SIP register messages with tools like SIPp, it is seen that Register handler is called less number of times than the actual SIP messages pumped. On further debugging it is seen that if a TCP stream contains a partial SIP message, all the other SIP messages in that TCP stream is ignored. Thats why the register handler is called less number of times.

The following changes solved the issue

diff --git a/sip/parser_stream.go b/sip/parser_stream.go
index 5e4545a..a5f36cc 100644
--- a/sip/parser_stream.go
+++ b/sip/parser_stream.go
@@ -166,7 +169,7 @@ func (p *ParserStream) ParseSIPStream(data []byte) (msgs []Message, err error) {
        case ErrParseLineNoCRLF, ErrParseReadBodyIncomplete:
            reader.Reset()
            reader.Write(unparsed)
-           return nil, ErrParseSipPartial
+           return msgs, ErrParseSipPartial
        }

        if err != nil {
 }

diff --git a/sip/transport_tcp.go b/sip/transport_tcp.go
index ed8660f..a765893 100644
--- a/sip/transport_tcp.go
+++ b/sip/transport_tcp.go
@@ -183,20 +183,20 @@ func (t *transportTCP) readConnection(conn *TCPConnection, laddr string, raddr s

 func (t *transportTCP) parseStream(par *ParserStream, data []byte, src string, handler MessageHandler) {
    msgs, err := par.ParseSIPStream(data)
-   if err == ErrParseSipPartial {
-       return
-   }

-   for _, msg := range msgs {
-       if err != nil {
-           t.log.Error().Err(err).Str("data", string(data)).Msg("failed to parse")
-           return
-       }
+    if err != nil && err != ErrParseSipPartial {
+        t.log.Error().Err(err).Str("data", string(data)).Msg("failed to parse")
+        return
+    }

+   for _, msg := range msgs {
        msg.SetTransport(t.Network())
        msg.SetSource(src)
        handler(msg)
    }
 }

@emiago
Copy link
Owner

emiago commented Oct 19, 2024

ah ok I see. I think it was due to large transfer buffer size as well. Therefore more messages can fit.
Thanks for reporting, I will consider your diff here is shared for patching.

@emiago
Copy link
Owner

emiago commented Oct 19, 2024

It needs probably test for all to be covered

@ayonpals
Copy link
Author

Looking into the code it looks like if parsing of any SIP message of a TCP stream fails, then also all the other SIP messages are ignored.
Is it possible to enhance the code to send "400 Bad Request" response to client only for the bad request and process the other messages normally?

@emiago
Copy link
Owner

emiago commented Oct 21, 2024

Hi ayonpals, thanks providing more context. Yes we need to deal this differentely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants