Harmony
  1. Harmony
  2. HARMONY-6637

[classlib] [archive] Inflater.inflate() short-circuits on zero-length request, finished() never true for a zero-length data source

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0M16
    • Component/s: Classlib
    • Labels:
      None
    • Estimated Complexity:
      Moderate

      Description

      There's code in the inflate() method of classlib/modules/archive/src/main/java/java/util/zip/Inflater.java (r926871 and earlier) which short-cuts the inflation if the parameter 'nbytes' is 0 - ie if the caller is actually requesting zero bytes:

      if (nbytes == 0)

      { return 0; --- Don't do any work: DON'T UPDATE THE *FINISHED* FLAG! }

      While it seems reasonable, it is in fact incompatible with the RI (which does attempt to process the deflated source data in the nbytes, and will update the finished() flag) - and so causes problems with a common inflation loop scenario, where the code loops to populate a destination byte-array of known size while inflater.finished() is not true. Even if the destination byte-array is known to be zero-length (e.g. we're deflating a zero-length file), the coder might still reasonably expect calling inflate() to set the finished() flag to true. However, unlike the RI, Harmony implementation does not update the finished() flag, which leads to an infinite loop or some other bad outcome.

      Bit short on time right now, will try to provide a better testcase and possibly a patch at a later date.

      best regards,
      Roberto Tyley

      1. Main.java
        1 kB
        Roberto Tyley

        Activity

        Roberto Tyley created issue -
        Hide
        Tim Ellison added a comment -

        Roberto, if you have a simple program demonstrating the problem that would be very helpful.

        Thanks.

        Show
        Tim Ellison added a comment - Roberto, if you have a simple program demonstrating the problem that would be very helpful. Thanks.
        Hide
        Roberto Tyley added a comment -

        This file demonstrates the difference between Harmony and the RI for java.util.zip.Inflater

        The RI output is:

        Inflating 10 bytes in a loop... [ 10 ] ...read total 10 bytes
        Inflating 5 bytes in a loop... [ 5 ] ...read total 5 bytes
        Inflating 0 bytes in a loop... [ 0 ] ...read total 0 bytes

        The Harmony output is:

        Inflating 10 bytes in a loop... [ 10 ] ...read total 10 bytes
        Inflating 5 bytes in a loop... [ 5 ] ...read total 5 bytes
        Inflating 0 bytes in a loop... [ 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 ... forever

        Show
        Roberto Tyley added a comment - This file demonstrates the difference between Harmony and the RI for java.util.zip.Inflater The RI output is: Inflating 10 bytes in a loop... [ 10 ] ...read total 10 bytes Inflating 5 bytes in a loop... [ 5 ] ...read total 5 bytes Inflating 0 bytes in a loop... [ 0 ] ...read total 0 bytes The Harmony output is: Inflating 10 bytes in a loop... [ 10 ] ...read total 10 bytes Inflating 5 bytes in a loop... [ 5 ] ...read total 5 bytes Inflating 0 bytes in a loop... [ 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 ... forever
        Roberto Tyley made changes -
        Field Original Value New Value
        Attachment Main.java [ 12455664 ]
        Hide
        Roberto Tyley added a comment -

        The infinite loop that occurs in Harmony is because inflater.finished() is never true - in the RI, the flag would get set to true even if you were asking for the flation of zero bytes, but in Harmony, if numBytes is zero, the flag will never get set.

        byte[] buffer=new byte[numBytes]; // the developer knows how big the data is going to be, that info is available to him through some other metadata
        int numRead=0;
        while (!inflater.finished())

        { // reasonable assumption by the developer here! int inflatedChunkSize=inflater.inflate(buffer, numRead, buffer.length-numRead); // 'buffer.length-numRead' is zero, harmony will not set the flag numRead+=inflatedChunkSize;; }

        ... consequently, an infinite loop :-/

        Show
        Roberto Tyley added a comment - The infinite loop that occurs in Harmony is because inflater.finished() is never true - in the RI, the flag would get set to true even if you were asking for the flation of zero bytes, but in Harmony, if numBytes is zero, the flag will never get set. byte[] buffer=new byte [numBytes] ; // the developer knows how big the data is going to be, that info is available to him through some other metadata int numRead=0; while (!inflater.finished()) { // reasonable assumption by the developer here! int inflatedChunkSize=inflater.inflate(buffer, numRead, buffer.length-numRead); // 'buffer.length-numRead' is zero, harmony will not set the flag numRead+=inflatedChunkSize;; } ... consequently, an infinite loop :-/
        Tim Ellison made changes -
        Assignee Tim Ellison [ tellison ]
        Hide
        Tim Ellison added a comment -

        Fix and regression test added to the ARCHIVE module at repo revision r1003513.

        Roberto, please check that it resolves the problem satisfactorily for you.

        Show
        Tim Ellison added a comment - Fix and regression test added to the ARCHIVE module at repo revision r1003513. Roberto, please check that it resolves the problem satisfactorily for you.
        Tim Ellison made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 5.0M16 [ 12315337 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in Harmony-1.5-head-linux-x86_64 #974 (See https://hudson.apache.org/hudson/job/Harmony-1.5-head-linux-x86_64/974/)
        Apply fix for HARMONY-6637 ([classlib] [archive] Inflater.inflate() short-circuits on zero-length request, finished() never true for a zero-length data source)

        Show
        Hudson added a comment - Integrated in Harmony-1.5-head-linux-x86_64 #974 (See https://hudson.apache.org/hudson/job/Harmony-1.5-head-linux-x86_64/974/ ) Apply fix for HARMONY-6637 ( [classlib] [archive] Inflater.inflate() short-circuits on zero-length request, finished() never true for a zero-length data source)
        Hide
        Hudson added a comment -

        Integrated in Harmony-select-1.5-head-linux-x86_64 #127 (See https://hudson.apache.org/hudson/job/Harmony-select-1.5-head-linux-x86_64/127/)
        Apply fix for HARMONY-6637 ([classlib] [archive] Inflater.inflate() short-circuits on zero-length request, finished() never true for a zero-length data source)

        Show
        Hudson added a comment - Integrated in Harmony-select-1.5-head-linux-x86_64 #127 (See https://hudson.apache.org/hudson/job/Harmony-select-1.5-head-linux-x86_64/127/ ) Apply fix for HARMONY-6637 ( [classlib] [archive] Inflater.inflate() short-circuits on zero-length request, finished() never true for a zero-length data source)
        Hide
        Roberto Tyley added a comment -

        Thanks, that's fantastic - I ran my codebase against the relevant Hudson snapshot builds (harmony-5.0-jdk-1003286 vs harmony-5.0-jdk-1003526 before/after the fix) and have confirmed that the new build resolves my problems.

        much appreciated,
        Roberto

        Show
        Roberto Tyley added a comment - Thanks, that's fantastic - I ran my codebase against the relevant Hudson snapshot builds (harmony-5.0-jdk-1003286 vs harmony-5.0-jdk-1003526 before/after the fix) and have confirmed that the new build resolves my problems. much appreciated, Roberto
        Hide
        Tim Ellison added a comment -

        Verified by Roberto.

        Show
        Tim Ellison added a comment - Verified by Roberto.
        Tim Ellison made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Roberto Tyley added a comment -

        Accepted downstream for Android honeycomb release:

        http://code.google.com/p/android/issues/detail?id=11755

        Show
        Roberto Tyley added a comment - Accepted downstream for Android honeycomb release: http://code.google.com/p/android/issues/detail?id=11755

          People

          • Assignee:
            Tim Ellison
            Reporter:
            Roberto Tyley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development