commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Martin Oberhuber (JIRA)" <j...@apache.org>
Subject [jira] Commented: (NET-89) [net] TelnetClient broken for binary transmissions + solution
Date Fri, 30 Jan 2009 14:49:59 GMT

    [ https://issues.apache.org/jira/browse/NET-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12668886#action_12668886
] 

Martin Oberhuber commented on NET-89:
-------------------------------------

As an alternative, I believe we could do this -- though I'm not so fond of this since it also
requires a functional change: for this API to work, the conditional conversion from _input_
to __input by applying the FromNetASCIIInputStream or not must be made *after* applying the
ReaderThread, and *not before* as it's done now. May be a marginal change, but may also change
functionality. Also note, that lazy creation of the FromNetASCIIInputStream / ToNetASCIIOutputStream
in the getInputStream() / getOutputStream() methods would be preferred over unconditional
creation of the Streams in the __connect method.

On the positive side, mixing the Streams (i.e. using either the binary or the NetASCII variant)
may be possible when flush() is called when done with one variant. At this point, however,
I'm not exactly sure how this could work out, so the Javadocs below discourage such usage.

Finally, another idea might be deprecating getInputStream() / getOutputStream() altogether
and replace it with having the client do the ToNetASCIIInputStream() etc conversion on the
client side after calling getBinaryOutputStream().

{code:title=TelnetClient.java|borderStyle=solid}
    /***
     * Returns the unconverted, binary telnet connection output stream
     * (without automatic NetASCII conversion to CRLF line endings).
     * Clients should use either the stream returned by this method, or
     * the stream returned from {@link #getOutputStream()}, but not mix
     * using the Streams.  You should not close the
     * stream when you finish with it.  Rather, you should call
     * {@link #disconnect  disconnect }.
     * <p>
     * This API is available in Commons Net 1.5 and 2.1, but not in version 2.0
     * <p>
     * @return The unconverted, binary telnet connection output stream.
     * @since 1.5
     * @since 2.1
     ***/
    public OutputStream getBinaryOutputStream()
    {
        return __binaryOutput;
    }

    /***
     * Returns the unconverted, binary telnet connection input stream
     * (without automatic NetASCII conversion from CRLF line endings).
     * Clients should use either the stream returned by this method, or
     * the stream returned from {@link #getInputStream()}, but not mix
     * using the Streams.  You should not close the
     * stream when you finish with it.  Rather, you should call
     * {@link #disconnect  disconnect }.
     * <p>
     * This API is available in Commons Net 1.5 and 2.1, but not in version 2.0
     * <p>
     * @return The unconverted, binary telnet connection input stream.
     * @since 1.5
     * @since 2.1
     ***/
    public InputStream getBinaryInputStream()
    {
        return __binaryInput;
    }
{code}

> [net] TelnetClient broken for binary transmissions + solution
> -------------------------------------------------------------
>
>                 Key: NET-89
>                 URL: https://issues.apache.org/jira/browse/NET-89
>             Project: Commons Net
>          Issue Type: Bug
>         Environment: Operating System: All
> Platform: All
>            Reporter: Colin Surprenant
>
> TelnetClient does not handle correctly binary transmissions in two places:
> First in TelnetClient#_connectAction_() the telnet input and output streams are
> wrapped in the NetASCII streams to handle net vs platform line separator
> conversion which breaks the binary data. My quick solution was to simply remove
> those two wrapping streams. A more general solution might be to provide access
> to the unfilterer stream with methods like getUnfilteredInputStream and
> getUnfilteredOutputStream or to dynamically stop the NetASCII stream from
> 'corrupting' the stream when a TelnetOption.BINARY option is negotiated.
> Also, in TelnetInoutStream#__read() there is a bug in the __receiveState
> handling for the _STATE_IAC state. When a second consecutive IAC (0x255) is
> received to encode the single 0x255 character, read does not return 0x255 but
> instead move on to reading the next char in the stream.
> The current code reads:
> case _STATE_IAC:
>     switch (ch)
>     {
>     case TelnetCommand.WILL:
>         __receiveState = _STATE_WILL;
>         continue;
>     case TelnetCommand.WONT:
>         __receiveState = _STATE_WONT;
>         continue;
>     case TelnetCommand.DO:
>         __receiveState = _STATE_DO;
>         continue;
>     case TelnetCommand.DONT:
>         __receiveState = _STATE_DONT;
>         continue;
>     /* TERMINAL-TYPE option (start)*/
>     case TelnetCommand.SB:
>         __suboption_count = 0;
>         __receiveState = _STATE_SB;
>         continue;
>     /* TERMINAL-TYPE option (end)*/
>     case TelnetCommand.IAC:
>         __receiveState = _STATE_DATA;
>         break;
>     default:
>         break;
>     }
>     __receiveState = _STATE_DATA;
>     continue;
> case _STATE_WILL:
> but it should be:
> case _STATE_IAC:
>     switch (ch)
>     {
>     case TelnetCommand.WILL:
>         __receiveState = _STATE_WILL;
>         continue;
>     case TelnetCommand.WONT:
>         __receiveState = _STATE_WONT;
>         continue;
>     case TelnetCommand.DO:
>         __receiveState = _STATE_DO;
>         continue;
>     case TelnetCommand.DONT:
>         __receiveState = _STATE_DONT;
>         continue;
>     /* TERMINAL-TYPE option (start)*/
>     case TelnetCommand.SB:
>         __suboption_count = 0;
>         __receiveState = _STATE_SB;
>         continue;
>     /* TERMINAL-TYPE option (end)*/
>     case TelnetCommand.IAC:
>         __receiveState = _STATE_DATA;
>         break; // exit to enclosing switch to return from read
>     default:
>         __receiveState = _STATE_DATA;           
>         continue; // move on the next char
>     }
>     break; // exit and return from read
> case _STATE_WILL:
> I'll provide patches for this.
> Colin.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message