[jira] [Reopened] (IO-871) IOUtils.contentEquals is incorrect when InputStream.available under-reports

["Éamonn McManus (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/IO-871?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Éamonn McManus reopened IO-871:
-------------------------------

This issue may have been resolved, but I'm reopening it until I can verify. (This involves a long test run that may not complete before tomorrow.)

I accidentally continued the discussion on IO-866. This is a copy of the exchange there between [~ggregory] and me.

"""
[~eamonnmcmanus] added a comment - 2 days ago
Unfortunately the [recent fix|https://github.com/apache/commons-io/commit/7815650ada4f27dc72c8c470d626481282b13dab] does not work. Google's tests reveal a failure, which I have boiled down to this repro:
{code:java}
        byte[] prefix = new byte[32];
        byte[] suffix = new byte[2];
        byte[] fileContents = "someTexts".getBytes();
        Files.write(testFile.toPath(), fileContents);
        byte[] expected = new byte[prefix.length + fileContents.length + suffix.length];
        System.arraycopy(prefix, 0, expected, 0, prefix.length);
        System.arraycopy(fileContents, 0, expected, prefix.length, fileContents.length);
        System.arraycopy(suffix, 0, expected, prefix.length + fileContents.length, suffix.length);
        assertTrue(IOUtils.contentEquals(
                new ByteArrayInputStream(expected),
                new SequenceInputStream(
                    Collections.enumeration(
                        Arrays.asList(
                            new ByteArrayInputStream(prefix),
                            new FileInputStream(testFile),
                            new ByteArrayInputStream(suffix)))))); {code}
The assertion passes if I replace the implementation of IOUtils.contentEquals with a simple loop that reads one byte from each stream until finding a mismatch or EOF, but it fails with the current snapshot implementation.

This is just my opinion, but the logic of the new [FileChannels.contentEquals|https://github.com/apache/commons-io/blob/c2e18c6d02f07c7466b29caeb5ac94aa04c4957d/src/main/java/org/apache/commons/io/channels/FileChannels.java#L61] method seems ferociously complicated and I wonder whether a simpler if less efficient approach might be better. Perhaps anyway the established IOUtils.contentEquals method could revert to its [old implementation|https://github.com/apache/commons-io/blob/1d2019f2a32fc5388e955c5a3a7fa4f236538d4a/src/main/java/org/apache/commons/io/IOUtils.java#L906]?

[~ggregory] added a comment - 2 days ago
Hello [~eamonnmcmanus] 

Thank you for the additional feedback.

We can revert to the old implementation if need be, but this does not solve the issue of fixing {{{}FileChannels.contentEquals(){}}}.

If you can find a better way to implement {{{}FileChannels.contentEquals(){}}}, feel free to provide it :)

So for now, I'll incorporate this new test and see how to fix that method.

 
 
[~ggregory] added a comment - Yesterday
Hello [~eamonnmcmanus] 

I fixed your most recent use case. Please test git master or a build from our snapshot repository [https://repository.apache.org/content/repositories/snapshots]

"""

> IOUtils.contentEquals is incorrect when InputStream.available under-reports
> ---------------------------------------------------------------------------
>
>                 Key: IO-871
>                 URL: https://issues.apache.org/jira/browse/IO-871
>             Project: Commons IO
>          Issue Type: Bug
>          Components: Utilities
>    Affects Versions: 2.19.0
>            Reporter: Éamonn McManus
>            Assignee: Gary D. Gregory
>            Priority: Minor
>             Fix For: 2.19.0
>
>
> A [recent change|https://github.com/apache/commons-io/commit/c832eb20796b25d8ee2437a8663616b1112cd7bd] to IOUtils.contentEquals(InputStream,InputStream) introduced a bug when at least one of the InputStream instances under-reports the number of available bytes. That is completely allowed by the contract of InputStream. I think the underlying problem may be [this line|https://github.com/apache/commons-io/blob/0d41d2fd958f28a0c4eb21afb6f34be8efa0e4fb/src/main/java/org/apache/commons/io/channels/FileChannels.java#L94] in FileChannels. There's no requirement for a ReadableByteChannel.read to return all available bytes, and therefore it is possible for one of the two reads being compared to be shorter than the other, even though the contents of the two channels are in fact equal.
> This problem showed up when importing the latest source snapshot to Google's internal repository and running all of Google's tests against it. Here is a simple repro:
> {code:java}
>     assertThat(
>             IOUtils.contentEquals(
>                 new ByteArrayInputStream("ab".getBytes()),
>                 new SequenceInputStream(
>                     new ByteArrayInputStream("a".getBytes()),
>                     new ByteArrayInputStream("b".getBytes()))))
>         .isTrue(); {code}
> The assertion passes before this change and fails after it.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)