JCgH4164838Gh792C124B5 commented on a change in pull request #408:
URL: https://github.com/apache/struts/pull/408#discussion_r415400292
##########
File path: plugins/junit/src/main/java/org/apache/struts2/util/TestUtils.java
##########
@@ -85,13 +79,13 @@ public static boolean compare(URL url, String text) throws
Exception {
public static void assertEquals(URL source, String text) throws Exception {
String writerString = TestUtils.normalize(text, true);
String bufferString = TestUtils.normalize(readContent(source), true);
- Assert.assertEquals(bufferString,writerString);
+ Assert.assertEquals(bufferString, writerString);
}
public static String readContent(URL url) throws Exception {
if (url == null)
throw new Exception("unable to verify a null URL");
- return IOUtils.toString(url.openStream());
+ return IOUtils.toString(url.openStream(), StandardCharsets.UTF_8);
Review comment:
It appears the previous implementation would have implicitly used
`Charset.defaultCharset()` for processing the `URL`. Some users might want (or
need) a way to specify the `Charset` for processing the `URL`, so maybe a small
refactoring into:
```
public static String readContent(final URL url) throws Exception {
return readContent(url, StandardCharsets.UTF_8);
}
public static String readContent(final URL url, final Charset encoding)
throws Exception {
if (url == null) {
throw new IllegalArgumentException("Unable to verify a null
URL");
}
if (encoding == null) {
throw new IllegalArgumentException("Unable to verify the URL
using a null Charset");
}
return IOUtils.toString(url.openStream(), encoding);
}
```
might be useful ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
