Re: [PATCH] Fix PR libgcj/36252, OutOfMemoryError in String constructor.

[David Daney <ddaney at avtrex dot com>]

Tom Tromey wrote:
> David> My fix is to assume that the size of the largest atomic output
> David> sequence for *all* encodings is bounded and if the output
> David> buffer is larger than this bound, then there is a problem with
> David> the input and we should quit trying rather than expanding the
> David> size of the output buffer.
>
> There is code in InputStreamReader that handles this case in a
> different way.  I think both pieces of code should work the same way;
> so could you look at that one and see (1) if it is susceptible to this
> or some related problem (i.e., does it ensure that we make progress?)

InputStreamReader already handles this case in much the same manner that 
I am proposing that we fix String.  The details are slightly different 
as InputStreamReader can throw an exception on malformed input.  The 
string constructor cannot throw an exception, so it has to return 
successfully and throw away some portion of the data.


> and (2) which is better?

With my patch they would be essentially equivalent.  My patch uses an 
upper bound on the needed size of the output array of 20. 
InputStreamReader uses 1.  I don't know if 1 is correct, but 20 is 
certainly safe if 1 is.


> David> A bonus fix is to catch java.io.CharConversionException and
> David> stop the conversion.  This keeps this checked exception from
> David> escaping from the constructor which does not declare that that
> David> it throws said exception.
>
> It seems bad to throw this at all.  IMO we should either declare it
> properly, and fix the fallout in the Java code, or not throw it at
> all.
>
> For the String constructor one question is what the JDK does here.
> I vaguely remember some funny business, but no details.

I will go off of Sun's documentation.  The String constructor is not 
allowed to throw the exception, The convert code must throw the 
exception so that InputStreamReader *can* throw it.

Quoting the JDK 1.6 documentation:

 The behavior of this constructor when the given bytes are not valid in 
the given charset is unspecified. The CharsetDecoder class should be 
used when more control over the decoding process is required.

My patch to String's native code is simple and lets the String contain 
all the valid characters.  Throwing away invalid data is contemplated 
and allowed by the specification.


David Daney