BaseEncoding#decode IllegalArgumentException leaks the use of Guava

[GoogleCodeExporter]

The IllegalArgumentException thrown by BaseEncoding#decode is constructed with 
the DecodeException's toString, so it contains the DecodeException's FQCN. As 
such, when the input comes from the a third party, 
IllegalArgumentException#getMessage cannot be safely used to tell the sender 
what's wrong with its input.

See 
https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/com
mon/io/BaseEncoding.java?r=8c7fbd6d742eefad76a61442f4d78d37d9e5079c#217

For example:

 BaseEncoding.base64().decode("€".getBytes(StandardCharsets.UTF_8))

will throw an IllegalArgumentException whose detail message is:

 com.google.common.io.BaseEncoding$DecodingException: Unrecognized character: €

instead of just:

 Unrecognized character: €

Workaround: use iae.getCause().getMessage() instead of iae.getMessage(), or 
just a static "malformed input" message.

Suggested fix: use

 new IllegalArgumentException(e.getMessage(), e)

instead of

 new IllegalArgumentException(e);

Original issue reported on code.google.com by t.broyer on 12 Mar 2014 at 5:20

[GoogleCodeExporter]

We don't understand why Throwable's constructor uses the chained exception's 
toString() instead of its getMessage(). That seems strange. However, being 
convinced that this is a problem worth worrying about enough to change all our 
usages of 'throw new FooException(e)' to 'throw new 
FooException(e.getMessage(), e)' and then try to remember that practice for the 
future... that's a different story. Does working around this strangeness have 
that much value?

The idea of displaying the message text of an unchecked exception to a user 
seems unsound to me.

Original comment by kevinb@google.com on 13 Mar 2014 at 7:50

[GoogleCodeExporter]

In my specific case, I was implementing HTTP Basic auth.

    String credentials;
    try {
      CharsetDecoder charsetDecoder = StandardCharsets.UTF_8.newDecoder();
      credentials = charsetDecoder.decode(ByteBuffer.wrap(BaseEncoding.base64().decode(parts.get(1)))).toString();
    } catch (CharacterCodingException | IllegalArgumentException e) {
      malformedCredentials(requestContext);
      return;
    }

Long story short: I originally forgot to catch IllegalArgumentException, which 
resulted in a 500 Internal Server Error. When revisiting the code to add the 
IAE, I thought I could possibly send the e.getMessage() to the user to he knows 
what's wrong, rather than the static "malformed credentials" message I current 
send; and stumbled on this strange behavior.

I can very well live with the current situation (and a static error message), 
but thought I should probably report it anyway it probably wasn't the expected 
behavior. So feel free to close the issue if you think it's not worth it.

Original comment by t.broyer on 15 Mar 2014 at 11:01

[GoogleCodeExporter]

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:09

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

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

• Changed state: Migrated

[GoogleCodeExporter]

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

• Added labels: Restrict-AddIssueComment-Commit