yangxu998 / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

ByteArrayDataInput.readFully() throws a different exception than what's documented #549

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I'm using com.google.common.io.ByteArrayDataInput in guava-r08, which I am 
creating using the factory method in ByteStreams:

byte[] buffer = ...;
ByteArrayDataInput in = ByteStreams.newDataInput(buffer);

Then I read data from this:

byte[] bytes = new bytes[80];
in.readFully(bytes);

When end-of-file is reached, according to the API documentation readFully() 
throws a java.io.EOFException. However, it really throws a 
java.lang.IllegalStateException inside which the EOFException is wrapped. 
Example stack trace:

java.lang.IllegalStateException: java.io.EOFException
    at com.google.common.io.ByteStreams$ByteArrayDataInputStream.readFully(ByteStreams.java:269)
    ...
Caused by: java.io.EOFException
    at java.io.DataInputStream.readFully(DataInputStream.java:180)
    at java.io.DataInputStream.readFully(DataInputStream.java:152)
    at com.google.common.io.ByteStreams$ByteArrayDataInputStream.readFully(ByteStreams.java:267)
    ... 25 more

Note that the API documentation for ByteArrayDataInput.readFully() was copied 
from java.io.DataInput.readFully().

Why is this method wrapping the EOFException in an IllegalStateException?

I suspect that the same problem exists in other methods of ByteArrayDataInput 
and also of ByteArrayDataOutput.

Original issue reported on code.google.com by jes...@gmail.com on 17 Feb 2011 at 8:58

GoogleCodeExporter commented 9 years ago

Original comment by boppenh...@google.com on 21 Feb 2011 at 10:23

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 16 Jul 2011 at 8:18

GoogleCodeExporter commented 9 years ago

Original comment by fry@google.com on 10 Dec 2011 at 4:03

GoogleCodeExporter commented 9 years ago
I'm sorry it took me a while to come back and think about this.

I know this is strange, but this is actually the intended behavior. The 
ByteArrayDataInput class docs say:

"If any method encounters the end of the array prematurely, it throws (ISE)."

We opted to treat it as *programmer error* to read too far in one of these 
things. This is not an entirely unreasonable choice since you do have the byte 
array and you should know whether it's big enough ahead of time.

If we were to change that, it actually removes a fair chunk of the entire 
purpose of having this type in the first place. I think when we redo common.io 
from scratch we won't have anything like this.

I'm going to make this a little more clear in the doc.

Original comment by kevinb@google.com on 16 Mar 2012 at 9:51

GoogleCodeExporter commented 9 years ago
Uhm, java.io.DataInput states that EOFException should be thrown -- that's the 
interface contract. It says precisely the following:

Throws:
EOFException - if this stream reaches the end before reading all the bytes.

That's pretty clear documentation to me.

Original comment by tv@duh.org on 17 Mar 2012 at 12:49

GoogleCodeExporter commented 9 years ago
BTW, the reason I ignored "you do have the byte array and you should know 
whether it's big enough ahead of time" is that the point of implementing an 
interface is to provide a flexible way to address the object _outside_ of the 
creation context.

Code written to make use of the DataInput _interface_, away from where the 
array actually exists, very likely won't know the size of the array ahead of 
time.

Original comment by tv@duh.org on 17 Mar 2012 at 12:51

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 18 Jun 2012 at 5:42

GoogleCodeExporter commented 9 years ago
The doc has been upgraded, and I'm marking this fixed -- as fixed as it can be.

I'm sorry that you disagree with our choice, but we made it a long time ago and 
even removed this interface from @Beta status more than two years ago.  And as 
stated, we *also* disagree with our own choices here in a major way and want to 
put our energies on solving these problems properly in the future.

Original comment by kevinb@google.com on 19 Jun 2012 at 9:14

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:15

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:09