guava icon indicating copy to clipboard operation
guava copied to clipboard

Consider reporting errors during encoding in ReaderInputStream

Open bjmi opened this issue 1 year ago • 6 comments

API(s)

ReaderInputStream(reader, charset, bufferSize)

How do you want it to be improved?

Use the default error action for malformed input and unmappable characters when creating the encoder by removing following lines:

.onMalformedInput(CodingErrorAction.REPLACE)
.onUnmappableCharacter(CodingErrorAction.REPLACE)

and update the corr. Javadoc.

Why do we need it to be improved?

Data is silently changed and conversion errors are just ignored. This lead to corrupt data, it is particularly bad if it goes undetected for a long period of time. Therefore the default behavior should make the user aware of the problem. Newer introduced API (JDK 11) like java.nio.file.Files.readString(..) and java.nio.file.Files.writeString(..) also use the reporting error approach.

An alternative would be to make ReaderInputStream public and allow to pass a Reader and a CharsetEncoder. The encoder can be created by the caller via charset.newEncoder() easily and configured according the intended use case. This was requested in #5376.

Example

No code needs to be changed beforehand. It's just about reporting a problem.

Current Behavior

Encoding errors are silently ignored and lead to corrupt text files.

Desired Behavior

Encoding errors should raise an exception and make errors visible, subsequently the code or data in question gets fixed.

Concrete Use Cases

We process texts from customers that use German and Cyrillic letters and it is crucial that the content remains intact when decoding / encoding is used.

Checklist

bjmi avatar Jan 26 '24 09:01 bjmi

Further ideas

  • Introduce com.google.common.io.CharStreams.asInputStream(Reader, Charset): InputStream and com.google.common.io.CharStreams.asInputStream(Reader, CharsetEncoder): InputStream
  • Overload com.google.common.io.CharSource.asByteSource(Supplier<CharsetEncoder>)

Passing a CharsetEncoder allows you to define the behavior on malformed input and on unmappable characters. The existing Charset is actually an encoder factory.

bjmi avatar Jan 29 '24 09:01 bjmi

The relevant public API is CharSource.asByteSource(Charset).openStream(), right? I don't see ReaderInputStream being used from anywhere else.

chaoren avatar Feb 13 '24 16:02 chaoren

You are right :)

bjmi avatar Feb 14 '24 04:02 bjmi

I gave this a shot in #7218. I'd appreciate any input you might have. I only implemented the suggestions in https://github.com/google/guava/issues/6944#issue-2101889858 . Please don't yell at me if I did something wrong because I only based my changes on the original comment and am quite new to this codebase. ;)

tobbi avatar May 14 '24 05:05 tobbi

Should I try tackling the stuff mentioned in https://github.com/google/guava/issues/6944#issuecomment-1914342198 as well?

tobbi avatar May 14 '24 05:05 tobbi

Should I try tackling the stuff mentioned in #6944 (comment) as well?

Thank you for your effort. It would suffice to provide com.google.common.io.CharStreams.asInputStream(Reader, Charset): InputStream without changing CodingErrorActions of the encoder.

bjmi avatar May 15 '24 06:05 bjmi