jokiazhang / metadata-extractor

Automatically exported from code.google.com/p/metadata-extractor
0 stars 0 forks source link

JpegSegmentReader.checkForBytesOnStream could be problematic #27

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The JpegSegmentReader.checkForBytesOnStream method uses stream.available() to 
see if enough bytes are available on a stream, also in a wait loop. This will 
not work with certain streams, even non-network or in-memory ones. Example: 
SequenceInputStream.available() will return 0 (forever) if the read position is 
at the boundary between two of the streams in the sequence, ie if the last byte 
of one member stream has been read and the next stream has not been started. 
(Frameworks like Apache mime4j use SequenceInputStreams.) AFAIK, there's no 
other reliable way of testing if there is at least one more byte available than 
trying to read one.

Original issue reported on code.google.com by uja...@gmail.com on 1 Jun 2011 at 11:13

GoogleCodeExporter commented 9 years ago
It would also be fine, to be able to control the timeout.

In my case, there is also a problem with EOF while reading Exif (timeout 
followed by "segment size would extend beyond file stream length").

Original comment by tschlenk...@googlemail.com on 16 Sep 2011 at 8:51

GoogleCodeExporter commented 9 years ago
Frankly all the code around InputStream handling (at least in 
JpegSegmentReader, that's the only class I saw) is an utter bunch of crap.

Requiring a BufferedInputStream in the method interface (ImageMetadataReader) 
is crazy.

The whole waitForBytes-flag is insane.

This checkForBytesOnStream method is beyond insane.

The contract of (Buffered)InputStream.read(byte[], int, int) clearly states 
that it may read fewer bytes than requested (and counting on available() in 
order to determine total size isn't going to work). If available() returns 0 
this is merely an indication that the next request to read might block! Which 
is not a problem since we do stream reading where blocking is to be expected!
The end of a stream when using said read(...) is reached IF AND ONLY IF read 
returns -1.

This is serious as the library cannot be used with perfectly valid InputStreams.

Original comment by android-...@chamaeleon.de on 21 Sep 2012 at 12:16

GoogleCodeExporter commented 9 years ago
You're absolutely right. I was quite an inexperienced programmer when I put 
this API together in 2002. Ten years later, it doesn't look quite as good. I'll 
implement these changes in the next release.

Related: http://code.google.com/p/metadata-extractor/issues/detail?id=33

Original comment by drewnoakes on 16 Oct 2012 at 3:51

GoogleCodeExporter commented 9 years ago

Original comment by drewnoakes on 16 Oct 2012 at 4:31

GoogleCodeExporter commented 9 years ago
A fix for this is in place and is undergoing testing. It's involved some 
significant API changes, and so will be made available in the next major 
release, 2.7.0.

For now it's available on a feature branch.

Original comment by drewnoakes on 29 Oct 2012 at 2:03