Commons Compress
  1. Commons Compress
  2. COMPRESS-270

TarArchiveInputStream fails to read PAX header from InputStream

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8
    • Fix Version/s: 1.8.1
    • Component/s: Archivers
    • Labels:

      Description

      We have a scenario with a "slow" InputStream and are facing IOExceptions with TarArchiveEntry#getNextTarEntry().

      If the InputStream does not deliver fast enough, TarArchiveEntry#parsePaxHeaders(InputStream i) fails at this location:

      TarArchiveInputStream.java
      // Get rest of entry
      byte[] rest = new byte[len - read];
      int got = i.read(rest);
      if (got != len - read){
      	throw new IOException("Failed to read "
      		+ "Paxheader. Expected "
      		+ (len - read)
      		+ " bytes, read "
      		+ got);
      }
      

      We would suggest to change the code to something like this:

      TarArchiveInputStream.java
      // Get rest of entry
      byte[] rest = new byte[len - read];
      int got = 0;
      while((ch = i.read()) != -1) {
      	rest[got] = (byte) ch;
      	got++;
      	if(got == len - read) {
      		break;
      	}
      }
      if (got != len - read){
      	throw new IOException("Failed to read "
      		+ "Paxheader. Expected "
      		+ (len - read)
      		+ " bytes, read "
      		+ got);
      }
      

      This would make sure, that it gets all bytes of the PAX header value.

        Activity

        Patrik Burkhalter created issue -
        Patrik Burkhalter made changes -
        Field Original Value New Value
        Description We have a scenario with a "slow" {{InputStream}} and are facing {{IOExceptions}} with {{TarArchiveEntry#getNextTarEntry()}}.

        If the {{InputStream}} does not deliver fast enough, {{TarArchiveEntry#parsePaxHeaders(InputStream i)}} fails at this location:

        {code:title=TarArchiveInputStream.java|borderStyle=solid}
        // Get rest of entry
        byte[] rest = new byte[len - read];
        int got = i.read(rest);
        if (got != len - read){
        throw new IOException("Failed to read "
        + "Paxheader. Expected "
        + (len - read)
        + " bytes, read "
        + got);
        }
        {code}

        We would suggest to change the code to something like this:

        {code:title=TarArchiveInputStream.java|borderStyle=solid}
         // Get rest of entry
        byte[] rest = new byte[len - read];
        byte[] buffer = new byte[len - read];
        int got = 0;
        int destPos = 0;
        while((got = i.read(buffer)) != -1) {
        System.arraycopy(buffer, 0, rest, destPos, got);
        destPos += got;
        }
        if (destPos != len - read){
        throw new IOException("Failed to read "
        + "Paxheader. Expected "
        + (len - read)
        + " bytes, read "
        + destPos);
        }
        {code}

        This would make sure, that it gets all bytes of the PAX header value.
        We have a scenario with a "slow" {{InputStream}} and are facing {{IOExceptions}} with {{TarArchiveEntry#getNextTarEntry()}}.

        If the {{InputStream}} does not deliver fast enough, {{TarArchiveEntry#parsePaxHeaders(InputStream i)}} fails at this location:

        {code:title=TarArchiveInputStream.java|borderStyle=solid}
        // Get rest of entry
        byte[] rest = new byte[len - read];
        int got = i.read(rest);
        if (got != len - read){
        throw new IOException("Failed to read "
        + "Paxheader. Expected "
        + (len - read)
        + " bytes, read "
        + got);
        }
        {code}

        We would suggest to change the code to something like this:

        {code:title=TarArchiveInputStream.java|borderStyle=solid}
        // Get rest of entry
        byte[] rest = new byte[len - read];
        int got = 0;
        while((ch = i.read()) != -1) {
        rest[got] = (byte) ch;
        got++;
        if(got == len - read) {
        break;
        }
        }
        {code}

        This would make sure, that it gets all bytes of the PAX header value.
        Patrik Burkhalter made changes -
        Description We have a scenario with a "slow" {{InputStream}} and are facing {{IOExceptions}} with {{TarArchiveEntry#getNextTarEntry()}}.

        If the {{InputStream}} does not deliver fast enough, {{TarArchiveEntry#parsePaxHeaders(InputStream i)}} fails at this location:

        {code:title=TarArchiveInputStream.java|borderStyle=solid}
        // Get rest of entry
        byte[] rest = new byte[len - read];
        int got = i.read(rest);
        if (got != len - read){
        throw new IOException("Failed to read "
        + "Paxheader. Expected "
        + (len - read)
        + " bytes, read "
        + got);
        }
        {code}

        We would suggest to change the code to something like this:

        {code:title=TarArchiveInputStream.java|borderStyle=solid}
        // Get rest of entry
        byte[] rest = new byte[len - read];
        int got = 0;
        while((ch = i.read()) != -1) {
        rest[got] = (byte) ch;
        got++;
        if(got == len - read) {
        break;
        }
        }
        {code}

        This would make sure, that it gets all bytes of the PAX header value.
        We have a scenario with a "slow" {{InputStream}} and are facing {{IOExceptions}} with {{TarArchiveEntry#getNextTarEntry()}}.

        If the {{InputStream}} does not deliver fast enough, {{TarArchiveEntry#parsePaxHeaders(InputStream i)}} fails at this location:

        {code:title=TarArchiveInputStream.java|borderStyle=solid}
        // Get rest of entry
        byte[] rest = new byte[len - read];
        int got = i.read(rest);
        if (got != len - read){
        throw new IOException("Failed to read "
        + "Paxheader. Expected "
        + (len - read)
        + " bytes, read "
        + got);
        }
        {code}

        We would suggest to change the code to something like this:

        {code:title=TarArchiveInputStream.java|borderStyle=solid}
        // Get rest of entry
        byte[] rest = new byte[len - read];
        int got = 0;
        while((ch = i.read()) != -1) {
        rest[got] = (byte) ch;
        got++;
        if(got == len - read) {
        break;
        }
        }
        if (got != len - read){
        throw new IOException("Failed to read "
        + "Paxheader. Expected "
        + (len - read)
        + " bytes, read "
        + got);
        }
        {code}

        This would make sure, that it gets all bytes of the PAX header value.
        Stefan Bodewig made changes -
        Fix Version/s 1.9 [ 12326506 ]
        Hide
        Stefan Bodewig added a comment -

        there is IOUtils#readFully and I had hoped I had caught all places where we expected read to succeed by now. Obviously not. Sorry.

        This case is fixed with svn revision 1582594 but I'll leave this issue open as a reminder to scan through the code base once again.

        Show
        Stefan Bodewig added a comment - there is IOUtils#readFully and I had hoped I had caught all places where we expected read to succeed by now. Obviously not. Sorry. This case is fixed with svn revision 1582594 but I'll leave this issue open as a reminder to scan through the code base once again.
        Hide
        Stefan Bodewig added a comment -

        found two more cases inside the snappy and the ar package

        Show
        Stefan Bodewig added a comment - found two more cases inside the snappy and the ar package
        Stefan Bodewig made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Stefan Bodewig made changes -
        Fix Version/s 1.8.1 [ 12326841 ]
        Fix Version/s 1.9 [ 12326506 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        1d 6h 40m 1 Stefan Bodewig 28/Mar/14 16:29

          People

          • Assignee:
            Unassigned
            Reporter:
            Patrik Burkhalter
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development