You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by enh <en...@google.com> on 2009/11/27 06:29:06 UTC
Re: [classlib][archive] problems with ZipFile 9was: Re: [testing] M12
testing on Windows x86)
may as well fix the lot, rather than sniff these out one by one...
./rmi/src/main/java/java/rmi/server/RMIClassLoader.java:214:
byte[] buf = new byte[in.available()];
./luni/src/main/java/org/apache/harmony/luni/util/InputStreamExposer.java:86:
buffer = new byte[available];
./luni/src/main/java/org/apache/harmony/luni/util/ExposedByteArrayInputStream.java:52:
final byte[] buffer = new byte[available];
./luni/src/test/api/common/org/apache/harmony/luni/tests/java/io/SerializationStressTest.java:670:
byte[] input = new byte[dis.available()];
./print/src/test/api/java/common/javax/print/ValueTests.java:562:
res = new byte [iStream.available()];
./security/src/main/java/common/org/apache/harmony/security/utils/JarUtils.java:147:
byte[] sfBytes = new byte[signature.available()];
./swing/src/main/java/common/javax/swing/plaf/basic/BasicLookAndFeel.java:1022:
byte[] data = new
byte[audioStream.available()];
./swing/src/main/java/common/javax/swing/text/html/parser/DTD.java:145:
byte[] enc = new byte[stream.available()];
--elliott
On Thu, Nov 26, 2009 at 13:18, Tim Ellison <t....@gmail.com> wrote:
> On 26/Nov/2009 18:44, Tim Ellison wrote:
>> On 26/Nov/2009 17:00, Oliver Deakin wrote:
>>> A little more progress.
>>> It looks like we're reading javax\swing\text\html\parser\html32.bdtd
>>> (which is where the ZipFile/ZipEntry classes come in). I see that with
>>> the M12 version of those classes we end up throwing an ASN1Exception at
>>> line 886 of BerInputStream.java (right at the start of it's read()
>>> method). This unravels the stack up to ParserDelegator.createDTD() which
>>> has a "try ... catch(Exception e) { // do nothing }" around the DTD
>>> read. The exception is ignored and we carry on, but the DTD has not been
>>> fully populated and it looks like this is the cause of the html not
>>> being processed correctly and the tests failing. Needless to say, this
>>> exception is not thrown with the M11 version of ZipFile/ZipEntry.
>>>
>>> The question now is why offset==buffer.length at the start of the
>>> BerInputStream.read() call with the new ZipFile/ZipEntry classes. Ill
>>> keep digging.
>>
>> I'll bet you a beer that it is our friend available() again...
>>
>> Consider this:
>>
>> static final String NAME = "javax/swing/text/html/parser/html32.bdtd";
>>
>> public void test() throws IOException {
>> JarFile jar = new JarFile("swing.jar");
>> ZipEntry ze = jar.getEntry(NAME);
>> InputStream is = jar.getInputStream(ze);
>>
>> System.out.println("Size = " + ze.getSize());
>> System.out.println("Available = " + is.available());
>>
>> jar.close();
>> }
>>
>> On Harmony it prints:
>> Size = 51140
>> Available = 1
>>
>> On the RI it prints:
>> Size = 51140
>> Available = 51140
>>
>> I know the arguments that say available() should not be used to judge
>> the total number of bytes that can be read, but it seems that a number
>> of applications (including our generated parser?) use it in this way.
>>
>> I haven't tried changing it yet[1], but I'll take that bet if you want :-)
>>
>> Regards,
>> Tim
>>
>> [1] I'm supposed to be packing for a long weekend away, so I'll be quiet
>> for the next few days.
>
> Ok, so I may owe you a beer :-)
>
> A quick trial of implementing available() for the inflater stream fixes
> this test case (above) but not the original problem.
>
> Given there are these differences we see emerging, I wonder if we should
> revert the offending code and retry again after M12?
>
> FYI here is my hacked available() impl, which needs some testing before
> it is good to go in..
>
> Index: src/main/java/java/util/zip/ZipFile.java
> ===================================================================
> --- src/main/java/java/util/zip/ZipFile.java (revision 884543)
> +++ src/main/java/java/util/zip/ZipFile.java (working copy)
> @@ -247,7 +247,7 @@
> rafstrm.skip(entry.nameLen + localExtraLenOrWhatever);
> rafstrm.mLength = rafstrm.mOffset + entry.compressedSize;
> if (entry.compressionMethod == ZipEntry.DEFLATED) {
> - return new InflaterInputStream(rafstrm, new
> Inflater(true));
> + return new ZipInflaterInputStream(rafstrm, new
> Inflater(true), 1024, entry);
> } else {
> return rafstrm;
> }
> @@ -415,4 +415,27 @@
> return n;
> }
> }
> +
> + static class ZipInflaterInputStream extends InflaterInputStream {
> +
> + ZipEntry entry;
> + long bytesRead = 0;
> +
> + public ZipInflaterInputStream(InputStream is, Inflater inf, int
> bsize, ZipEntry entry) {
> + super(is, inf, bsize);
> + this.entry = entry;
> + }
> +
> + public int read(byte[] buffer, int off, int nbytes) throws
> IOException {
> + int i = super.read(buffer, off, nbytes);
> + if (i != -1) {
> + bytesRead += i;
> + }
> + return i;
> + }
> +
> + public int available() throws IOException {
> + return super.available() == 0 ? 0 : (int) (entry.getSize()
> - bytesRead);
> + }
> + }
> }
>
>
>
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Re: [classlib][archive] problems with ZipFile 9was: Re: [testing]
M12 testing on Windows x86)
Posted by Oliver Deakin <ol...@googlemail.com>.
enh wrote:
> may as well fix the lot, rather than sniff these out one by one...
>
Agreed, although we cannot fix them now for M12. Could you log these in
a JIRA so we have a record of them please?
Regards,
Oliver
> ./rmi/src/main/java/java/rmi/server/RMIClassLoader.java:214:
> byte[] buf = new byte[in.available()];
> ./luni/src/main/java/org/apache/harmony/luni/util/InputStreamExposer.java:86:
> buffer = new byte[available];
> ./luni/src/main/java/org/apache/harmony/luni/util/ExposedByteArrayInputStream.java:52:
> final byte[] buffer = new byte[available];
> ./luni/src/test/api/common/org/apache/harmony/luni/tests/java/io/SerializationStressTest.java:670:
> byte[] input = new byte[dis.available()];
> ./print/src/test/api/java/common/javax/print/ValueTests.java:562:
> res = new byte [iStream.available()];
> ./security/src/main/java/common/org/apache/harmony/security/utils/JarUtils.java:147:
> byte[] sfBytes = new byte[signature.available()];
> ./swing/src/main/java/common/javax/swing/plaf/basic/BasicLookAndFeel.java:1022:
> byte[] data = new
> byte[audioStream.available()];
> ./swing/src/main/java/common/javax/swing/text/html/parser/DTD.java:145:
> byte[] enc = new byte[stream.available()];
>
> --elliott
>
> On Thu, Nov 26, 2009 at 13:18, Tim Ellison <t....@gmail.com> wrote:
>
>> On 26/Nov/2009 18:44, Tim Ellison wrote:
>>
>>> On 26/Nov/2009 17:00, Oliver Deakin wrote:
>>>
>>>> A little more progress.
>>>> It looks like we're reading javax\swing\text\html\parser\html32.bdtd
>>>> (which is where the ZipFile/ZipEntry classes come in). I see that with
>>>> the M12 version of those classes we end up throwing an ASN1Exception at
>>>> line 886 of BerInputStream.java (right at the start of it's read()
>>>> method). This unravels the stack up to ParserDelegator.createDTD() which
>>>> has a "try ... catch(Exception e) { // do nothing }" around the DTD
>>>> read. The exception is ignored and we carry on, but the DTD has not been
>>>> fully populated and it looks like this is the cause of the html not
>>>> being processed correctly and the tests failing. Needless to say, this
>>>> exception is not thrown with the M11 version of ZipFile/ZipEntry.
>>>>
>>>> The question now is why offset==buffer.length at the start of the
>>>> BerInputStream.read() call with the new ZipFile/ZipEntry classes. Ill
>>>> keep digging.
>>>>
>>> I'll bet you a beer that it is our friend available() again...
>>>
>>> Consider this:
>>>
>>> static final String NAME = "javax/swing/text/html/parser/html32.bdtd";
>>>
>>> public void test() throws IOException {
>>> JarFile jar = new JarFile("swing.jar");
>>> ZipEntry ze = jar.getEntry(NAME);
>>> InputStream is = jar.getInputStream(ze);
>>>
>>> System.out.println("Size = " + ze.getSize());
>>> System.out.println("Available = " + is.available());
>>>
>>> jar.close();
>>> }
>>>
>>> On Harmony it prints:
>>> Size = 51140
>>> Available = 1
>>>
>>> On the RI it prints:
>>> Size = 51140
>>> Available = 51140
>>>
>>> I know the arguments that say available() should not be used to judge
>>> the total number of bytes that can be read, but it seems that a number
>>> of applications (including our generated parser?) use it in this way.
>>>
>>> I haven't tried changing it yet[1], but I'll take that bet if you want :-)
>>>
>>> Regards,
>>> Tim
>>>
>>> [1] I'm supposed to be packing for a long weekend away, so I'll be quiet
>>> for the next few days.
>>>
>> Ok, so I may owe you a beer :-)
>>
>> A quick trial of implementing available() for the inflater stream fixes
>> this test case (above) but not the original problem.
>>
>> Given there are these differences we see emerging, I wonder if we should
>> revert the offending code and retry again after M12?
>>
>> FYI here is my hacked available() impl, which needs some testing before
>> it is good to go in..
>>
>> Index: src/main/java/java/util/zip/ZipFile.java
>> ===================================================================
>> --- src/main/java/java/util/zip/ZipFile.java (revision 884543)
>> +++ src/main/java/java/util/zip/ZipFile.java (working copy)
>> @@ -247,7 +247,7 @@
>> rafstrm.skip(entry.nameLen + localExtraLenOrWhatever);
>> rafstrm.mLength = rafstrm.mOffset + entry.compressedSize;
>> if (entry.compressionMethod == ZipEntry.DEFLATED) {
>> - return new InflaterInputStream(rafstrm, new
>> Inflater(true));
>> + return new ZipInflaterInputStream(rafstrm, new
>> Inflater(true), 1024, entry);
>> } else {
>> return rafstrm;
>> }
>> @@ -415,4 +415,27 @@
>> return n;
>> }
>> }
>> +
>> + static class ZipInflaterInputStream extends InflaterInputStream {
>> +
>> + ZipEntry entry;
>> + long bytesRead = 0;
>> +
>> + public ZipInflaterInputStream(InputStream is, Inflater inf, int
>> bsize, ZipEntry entry) {
>> + super(is, inf, bsize);
>> + this.entry = entry;
>> + }
>> +
>> + public int read(byte[] buffer, int off, int nbytes) throws
>> IOException {
>> + int i = super.read(buffer, off, nbytes);
>> + if (i != -1) {
>> + bytesRead += i;
>> + }
>> + return i;
>> + }
>> +
>> + public int available() throws IOException {
>> + return super.available() == 0 ? 0 : (int) (entry.getSize()
>> - bytesRead);
>> + }
>> + }
>> }
>>
>>
>>
>>
>
>
>
>
--
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU