commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "James Pickering (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CODEC-130) Base64InputStream.skip skips underlying stream, not output
Date Tue, 27 Sep 2011 08:14:13 GMT

    [ https://issues.apache.org/jira/browse/CODEC-130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13115318#comment-13115318
] 

James Pickering commented on CODEC-130:
---------------------------------------

I think what you're saying is that the current code conforms to the FilterInputStream contract.
I don't think this is the case.

At a practical level, the current behaviour isn't what other FilterInputStream subclasses
do. InflaterInputStream, for example (from the JDK standard class library), skips over bytes
of uncompressed (i.e, output) data.

At a theoretical level, we should be honoring the InputStream contract. FilterInputStream
is a convenience subclass, to simplify InputStream coding (most FilterInputStream subclasses
have a 1-1 correspondence between input and output bytes, so FilterInputStream's behaviour
is reasonable). An InputStream is a stream of bytes that can be read, and consumers should
not need to know how it is implemented. If we continue with the current behaviour, we introduce
an implementation dependency, breaking OO encapsulation (indeed, I first noticed this bug
when some code that worked with other InputStreams broke with Base64InputStream).

I take your point that existing code that relies on the current behaviour could break as a
result of the fix, but I can't imagine any correct code relying on the current behaviour (it
produces nonsense data if skip requests are not multiples of 4, and aligned to 4 byte boundaries).
At worst, we'd break people's workarounds.
                
> Base64InputStream.skip skips underlying stream, not output
> ----------------------------------------------------------
>
>                 Key: CODEC-130
>                 URL: https://issues.apache.org/jira/browse/CODEC-130
>             Project: Commons Codec
>          Issue Type: Bug
>    Affects Versions: 1.5
>            Reporter: James Pickering
>            Priority: Minor
>         Attachments: base64snippet.java
>
>
> Base64InputStream.skip() skips within underlying stream, leading to unexpected behaviour.
> The following code will reproduce the issue:
> @Test
> public void testSkip() throws Throwable {
>     InputStream ins =
>             new ByteArrayInputStream("AAAA////".getBytes("ISO-8859-1"));//should decode
to {0, 0, 0, 255, 255, 255}
>     Base64InputStream instance = new Base64InputStream(ins);
>     assertEquals(3L, instance.skip(3L)); //should skip 3 decoded characters, or 4 encoded
characters
>     assertEquals(255, instance.read()); //Currently returns 3, as it is decoding "A/",
not "//" 
> }
> The following code, if added to Base64InputStream, or (BaseNCodecInputStream in the dev
build) would resolve the issue:
> @Override
> public long skip(long n) throws IOException {
>     //delegate to read()
>     long bytesRead = 0;
>     while ((bytesRead < n) && (read() != -1)) {
>         bytesRead++;
>     }
>     return bytesRead;
> }
> More efficient code may be possible.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message