Replace deprecated methods that don't properly handle encoding #1033
Conversation
| MetadataXpp3Reader mappingReader = new MetadataXpp3Reader(); | ||
| try (InputStream in = Files.newInputStream(mappingFile.toPath())) { | ||
| Metadata result = mappingReader.read(in, false); | ||
| return result; | ||
| } catch (FileNotFoundException e) { |
There was a problem hiding this comment.
I don't see the benefit of the change.
mappingReader.read( Files.newInputStream( mappingFile.toPath() ), false ) is exactly the same as mappingReader.read( ReaderFactory.newXmlReader( mappingFile ), false ).
The reason is that the read method accepting an InputStream does create an xml reader using ReaderFactory.newXmlReader.
There was a problem hiding this comment.
That is indeed something else that needs to be changed. There is no reason to convert an input stream to a reader immediately before passing it to an XML parser, and in fact that introduces an additional way things can break if the encoding is guessed wrong.
Until that can be done too, this at least removes a couple more dependencies on plexus-util.
| try (Writer writer = WriterFactory.newXmlWriter(metadataFile)) { | ||
| new MetadataXpp3Writer().write(writer, metadata); | ||
| try (OutputStream out = Files.newOutputStream(metadataFile.toPath())) { | ||
| new MetadataXpp3Writer().write(out, metadata); |
There was a problem hiding this comment.
If the change has an actual effect, this means that the XmlWriter created by the WriterFactory cannot detect the encoding properly. If that's the case, it should also be fixed.
There was a problem hiding this comment.
WriterFactory here is an unnecessary layer of indirection. It accomplishes nothing that can't be done with JDK standard APIs. I ultimately want to get rid of it.
|
Test cases here would be iffy, because the behavior is system dependent and most of the time this will just work. However there can be flaky behavior with non-ASCII characters when the Locale is something uncommon like Turkish. Generally when parsing XML one should let the parser detect the encoding, rather than trying to guess it. |
gnodet
changed the title
No description provided.