jsip
  1. jsip
  2. JSIP-153

Incomplete error reporting in StringMsgParser.java

    Details

    • Type: Improvement Improvement
    • Status: Reopened
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: current
    • Fix Version/s: milestone 1
    • Component/s: www
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: PC

    • Issuezilla Id:
      153

      Description

      When parsing malformed incoming SIP messages via UDPMessageChannel.java, error
      reporting like the following results:
      ----------
      2008-04-17 09:05:00,187 INFO [STDOUT] 09:05:00,171 ERROR [38_5060]
      gov.nist.javax.sip.stack.UDPMessageChannel.handleException(UDPMessageChannel.java:579)
      [BAD MESSAGE!]

      2008-04-17 09:05:00,187 INFO [STDOUT] 09:05:00,187 ERROR [38_5060]
      gov.nist.javax.sip.stack.UDPMessageChannel.handleException(UDPMessageChannel.java:580)
      [null]
      ----------

      ...so it is not clear what actual message caused the problem.
      The "null" is there because StringMsgParser.java (v 1.23) does not fill the
      field "rawStringMessage" when "parseSIPMessage(byte[])" is called.
      It is only filled when "parseSIPMessage(String)" is called.

      Why this is so I haven't a clue

      Why the 2 routines are different I don't understand either. They look like
      duplicated code to me. Hence the one could just call the other with its'
      argument cast to the proper type.
      If that is so, the comment above both functions is wrong as well...

      Can anyone explian/fix this, please?

      TIA,
      Tom.

      1. jsipdebug.log
        152 kB
        uijltje
      2. jsipserver.log
        12 kB
        uijltje

        Activity

        Hide
        mranga added a comment -

        More info needed:

        – SIP Message that caused failure
        – Stack trace
        – Debug Logs of presumed failure

        Show
        mranga added a comment - More info needed: – SIP Message that caused failure – Stack trace – Debug Logs of presumed failure
        Hide
        uijltje added a comment -

        Created an attachment (id=51)
        SIP message flow (server log)

        Show
        uijltje added a comment - Created an attachment (id=51) SIP message flow (server log)
        Hide
        uijltje added a comment -

        Created an attachment (id=52)
        Debugging output of supplied message-flow.

        Show
        uijltje added a comment - Created an attachment (id=52) Debugging output of supplied message-flow.
        Hide
        uijltje added a comment -

        Apologies for the terseness.
        See attachments for SERVER_LOG and DEBUG_LOG output at DEBUG level.

        As can be seen, the parser barfs at the "501 Not implemented" response that
        contains wrong CSeq (and Call-ID) fields...

        Show
        uijltje added a comment - Apologies for the terseness. See attachments for SERVER_LOG and DEBUG_LOG output at DEBUG level. As can be seen, the parser barfs at the "501 Not implemented" response that contains wrong CSeq (and Call-ID) fields...
        Hide
        mranga added a comment -

        There is no way to report an error back to the sender of a malformed response
        message. What would you like to see done? Better logging?

        Show
        mranga added a comment - There is no way to report an error back to the sender of a malformed response message. What would you like to see done? Better logging?
        Hide
        uijltje added a comment -

        It was never the intention to report the error back to sender.
        It's just that the error-handling puzzles me.

        First off, one gets a cryptic and completely unrelated message like "BAD
        MESSAGE" and "null" from UDP-handling when all that is wrong is that a
        Cseq-field in a response message is empty. But you don't get to see that (not
        when your loglevel is above DEBUG anyway). This results in a long search for the
        cause, a waste of time.

        So I started to make a patch to get rid of the UDP errors:

        ----------
        diff -u -r1.23 StringMsgParser.java
        — src/gov/nist/javax/sip/parser/StringMsgParser.java 4 Nov 2007 23:21:16
        -0000 1.23
        +++ src/gov/nist/javax/sip/parser/StringMsgParser.java 22 May 2008 07:43:08 -0000
        @@ -131,6 +131,8 @@

        int i = 0;

        + rawStringMessage = new String(msgBuffer);
        +
        // Squeeze out any leading control character.
        try {
        while (msgBuffer[i] < 0x20)
        ----------

        after which I intended to catch the parse-exception message and neatly log that
        at error-level so that you could immediately see that the empty Cseq would be
        the problem.

        But then I got sort of lost within the source.
        Trying to figure out why "rawStringMessage" wasn't filled in one case but was
        in the other.
        Also, I noticed that both parseSIPMessage() methods were almost duplicates.

        At that point I decided to leave it to the experts as I could not oversee what
        would break in case I made changes.

        So, ultimately, I indeed would like:

        • improved error logging on this parse-error
        • get rid of the UDP-exception
        • know why "rawStringMessage" is filled in 1 case but not the other
        • get rid of the duplicate code, it obscufates things.

        Hope this helps.

        Cheers,
        Tom.

        Show
        uijltje added a comment - It was never the intention to report the error back to sender. It's just that the error-handling puzzles me. First off, one gets a cryptic and completely unrelated message like "BAD MESSAGE" and "null" from UDP-handling when all that is wrong is that a Cseq-field in a response message is empty. But you don't get to see that (not when your loglevel is above DEBUG anyway). This results in a long search for the cause, a waste of time. So I started to make a patch to get rid of the UDP errors: ---------- diff -u -r1.23 StringMsgParser.java — src/gov/nist/javax/sip/parser/StringMsgParser.java 4 Nov 2007 23:21:16 -0000 1.23 +++ src/gov/nist/javax/sip/parser/StringMsgParser.java 22 May 2008 07:43:08 -0000 @@ -131,6 +131,8 @@ int i = 0; + rawStringMessage = new String(msgBuffer); + // Squeeze out any leading control character. try { while (msgBuffer [i] < 0x20) ---------- after which I intended to catch the parse-exception message and neatly log that at error-level so that you could immediately see that the empty Cseq would be the problem. But then I got sort of lost within the source. Trying to figure out why "rawStringMessage" wasn't filled in one case but was in the other. Also, I noticed that both parseSIPMessage() methods were almost duplicates. At that point I decided to leave it to the experts as I could not oversee what would break in case I made changes. So, ultimately, I indeed would like: improved error logging on this parse-error get rid of the UDP-exception know why "rawStringMessage" is filled in 1 case but not the other get rid of the duplicate code, it obscufates things. Hope this helps. Cheers, Tom.

          People

          • Assignee:
            jsip-issues
            Reporter:
            uijltje
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: