Hi Vikas:

Very tricky, and strange, situation! The fix seems reasonable to me. My
two comments are just conservative programming suggestions/questions.
Thoughts welcome.

Cheers,
John


http://codereview.appspot.com/1854041/diff/18001/19001
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java
(right):

http://codereview.appspot.com/1854041/diff/18001/19001#newcode480
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java:480:
private byte[] toByteArraySafe(final HttpEntity entity) throws
IOException {
Is there a reliable test we can do in this method to circumscribe the
fix only to the potential condition in which failure occurs, ie. when
retrieving ZLIB-inflated content?

For instance, entity.getContent() instanceof InflaterInputStream? I
don't know the implementation well enough to say whether that's a
sensible solution.

http://codereview.appspot.com/1854041/diff/18001/19001#newcode513
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java:513:
if (!eofe.getMessage().equals("Unexpected end of ZLIB input stream")) {
alternately (or in addition), per your comment:
"The bug lies in the JDK 'Inflater.finished()' call, that erroneously
returns 'false' even if the input stream (zip) is done reading the
response. Whenever this happens,
'java.util.zip.InflaterInputStream.fill' will throw the 'EOFException'."

...it sounds like you could pull "int l" out of the try-block and only
swallow the EOFException if l == -1 (ie. if the InputStream is finished
reading).

http://codereview.appspot.com/1854041/show

Reply via email to