Commons Net
  1. Commons Net
  2. NET-73

[net][PATCH] TelnetInputStream.read hangs when socket data ends in a command sequence

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 1.5, 2.0
    • Component/s: None
    • Labels:
      None
    • Environment:

      Operating System: other
      Platform: All

      Description

      Background: If one calls TelnetInputStream.read() in single-threaded mode (no
      reader thread) and there is no data immediately available, the call blocks on
      a socket read. When data starts to arrive, the stream adds all the available
      bytes to its internal queue before returning the first one to the caller. To
      do this, it calls __read() in a loop for as long as there are bytes available.
      The __read() method returns the first byte of "user data" from the socket. If
      __read() encounters a Telnet command sequence (IAC, WILL, WONT, DO, DONT,
      etc.), it handles the negotiation transparently and then returns the first
      byte of user data.

      In most cases, this works fine, but a problem arises if a chunk of data from
      the remote host ends in a Telnet command sequence. When that happens, the
      TelnetInputStream.read() method hangs, even though it may have already
      acquired some user data. This is because it calls __read() in a loop as long
      as super.available() returns true. But if the remaining data from the socket
      consists entirely of Telnet commands, __read() will process those AND THEN
      BLOCK waiting for user data.

      Just checking super.available() is not sufficient. We should continue the loop
      only if there are bytes of USER DATA still available from the socket. Not
      doing this can cause the client to wait indefinitely.

        Activity

        Hide
        Daniel Savarese added a comment -

        Can you submit a patch that corrects the error? All I can offer you in return
        is our thanks and the addition of your name to the list of contributors.

        Show
        Daniel Savarese added a comment - Can you submit a patch that corrects the error? All I can offer you in return is our thanks and the addition of your name to the list of contributors.
        Hide
        Rob Hasselbaum added a comment -

        I am working on a patch now. If someone can commit my earlier patch for bug
        38688, we can avoid a messy merge later, as this impacts some of the same code.

        Show
        Rob Hasselbaum added a comment - I am working on a patch now. If someone can commit my earlier patch for bug 38688, we can avoid a messy merge later, as this impacts some of the same code.
        Hide
        Daniel Savarese added a comment -

        Patch for COM-2776 applied. Thanks!

        Show
        Daniel Savarese added a comment - Patch for COM-2776 applied. Thanks!
        Hide
        Rob Hasselbaum added a comment -

        Created an attachment (id=18174)
        Fix read hang after command sequence

        This patch prevents the read hang that occurs in single-threaded mode when a
        remote host response ends in a Telnet command sequence.

        I have added a boolean flag to TelnetInputStream.__read that tells it whether
        or not to block on a socket read if there is no user data available in the
        superclass buffer. It is set to true for the first iteration through the loop
        in read(), which is the only time we need to wait. Subsequent iterations only
        serve to eagerly fill the queue, so we don't need to wait, and the argument is
        set to false.

        If the argument is false and there is no data available, __read() returns a
        value of -2 that prevents the caller from using the data. I didn't use an
        exception because of overhead concern. (We could hit this condition often.)

        If the reader thread is enabled, run() calls __read() with the argument set to
        true, which is equivalent to the old behavior.

        Show
        Rob Hasselbaum added a comment - Created an attachment (id=18174) Fix read hang after command sequence This patch prevents the read hang that occurs in single-threaded mode when a remote host response ends in a Telnet command sequence. I have added a boolean flag to TelnetInputStream.__read that tells it whether or not to block on a socket read if there is no user data available in the superclass buffer. It is set to true for the first iteration through the loop in read(), which is the only time we need to wait. Subsequent iterations only serve to eagerly fill the queue, so we don't need to wait, and the argument is set to false. If the argument is false and there is no data available, __read() returns a value of -2 that prevents the caller from using the data. I didn't use an exception because of overhead concern. (We could hit this condition often.) If the reader thread is enabled, run() calls __read() with the argument set to true, which is equivalent to the old behavior.
        Hide
        Rory Winston added a comment -

        This has been fixed.

        Show
        Rory Winston added a comment - This has been fixed.
        Hide
        Sebb added a comment -

        The fix has been applied to trunk - surely it should also be applied to the NET_2_0 branch before closing this bug?

        Show
        Sebb added a comment - The fix has been applied to trunk - surely it should also be applied to the NET_2_0 branch before closing this bug?
        Hide
        Martin Oberhuber added a comment -

        Darn, it seems to be missing on the NET_2_0 branch indeed... and this has been detected before, see NET-179 : https://issues.apache.org/jira/browse/NET-179 Looks like that one needs to get re-opened?

        Show
        Martin Oberhuber added a comment - Darn, it seems to be missing on the NET_2_0 branch indeed... and this has been detected before, see NET-179 : https://issues.apache.org/jira/browse/NET-179 Looks like that one needs to get re-opened?
        Hide
        muralidhar polavarapu added a comment -

        I am facing same issue with commons-net-3.0.1 jar. TelentInputStream.read hangs and thorows SocketTimeoutException

        Using ERS on linux machine.

        Stack trace:

        java.net.SocketTimeoutException: Read timed out
        at java.net.SocketInputStream.socketRead0(Native Method)
        at java.net.SocketInputStream.read(SocketInputStream.java:129)
        at java.io.BufferedInputStream.read1(BufferedInputStream.java:256)
        at java.io.BufferedInputStream.read(BufferedInputStream.java:317)
        at java.io.BufferedInputStream.fill(BufferedInputStream.java:218)
        at java.io.BufferedInputStream.read(BufferedInputStream.java:237)
        at org.apache.commons.net.telnet.TelnetInputStream.__read(TelnetInputStream.java:136)
        at org.apache.commons.net.telnet.TelnetInputStream.run(TelnetInputStream.java:591)
        at java.lang.Thread.run(Thread.java:619)

        Show
        muralidhar polavarapu added a comment - I am facing same issue with commons-net-3.0.1 jar. TelentInputStream.read hangs and thorows SocketTimeoutException Using ERS on linux machine. Stack trace: java.net.SocketTimeoutException: Read timed out at java.net.SocketInputStream.socketRead0(Native Method) at java.net.SocketInputStream.read(SocketInputStream.java:129) at java.io.BufferedInputStream.read1(BufferedInputStream.java:256) at java.io.BufferedInputStream.read(BufferedInputStream.java:317) at java.io.BufferedInputStream.fill(BufferedInputStream.java:218) at java.io.BufferedInputStream.read(BufferedInputStream.java:237) at org.apache.commons.net.telnet.TelnetInputStream.__read(TelnetInputStream.java:136) at org.apache.commons.net.telnet.TelnetInputStream.run(TelnetInputStream.java:591) at java.lang.Thread.run(Thread.java:619)
        Hide
        Sebb added a comment -

        The 2.x (and 3.x) line of code was patched in r633798, and I've checked that the same patch is also in the current 3.0.1 code.

        ==

        I think the stack trace shows that the code is waiting to close a previous input stream as part of a connect.

        This suggests that the problem may lie in the way the Telnet classes are being used.
        Can you provide sample code that demonstrates the problem?

        Show
        Sebb added a comment - The 2.x (and 3.x) line of code was patched in r633798, and I've checked that the same patch is also in the current 3.0.1 code. == I think the stack trace shows that the code is waiting to close a previous input stream as part of a connect. This suggests that the problem may lie in the way the Telnet classes are being used. Can you provide sample code that demonstrates the problem?
        Hide
        Sebb added a comment -

        No response to request for reproducer so not a lot can do here.

        Show
        Sebb added a comment - No response to request for reproducer so not a lot can do here.

          People

          • Assignee:
            Unassigned
            Reporter:
            Rob Hasselbaum
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development