SebTardif commented on code in PR #4127:
URL: https://github.com/apache/logging-log4j2/pull/4127#discussion_r3403058505
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java:
##########
@@ -359,24 +360,44 @@ public String toString() {
try {
final File file = FileUtils.fileFromUri(url.toURI());
final URLConnection urlConnection =
UrlConnectionFactory.createConnection(url);
+ boolean success = false;
+ InputStream openedStream = null;
try {
+ final ConfigurationSource source;
if (file != null) {
- return new
ConfigurationSource(urlConnection.getInputStream(),
FileUtils.fileFromUri(url.toURI()));
+ openedStream = urlConnection.getInputStream();
+ source = new ConfigurationSource(openedStream, file);
} else if (urlConnection instanceof JarURLConnection) {
// Work around
https://bugs.openjdk.java.net/browse/JDK-6956385.
URL jarFileUrl = ((JarURLConnection)
urlConnection).getJarFileURL();
File jarFile = new File(jarFileUrl.getFile());
long lastModified = jarFile.lastModified();
- return new
ConfigurationSource(urlConnection.getInputStream(), url, lastModified);
+ openedStream = urlConnection.getInputStream();
+ source = new ConfigurationSource(openedStream, url,
lastModified);
} else {
- return new ConfigurationSource(
- urlConnection.getInputStream(), url,
urlConnection.getLastModified());
+ openedStream = urlConnection.getInputStream();
+ source = new ConfigurationSource(openedStream, url,
urlConnection.getLastModified());
}
- } catch (FileNotFoundException ex) {
+ success = true;
+ return source;
+ } catch (final FileNotFoundException ex) {
ConfigurationFactory.LOGGER.info("Unable to locate file {},
ignoring.", url.toString());
return null;
+ } finally {
+ if (!success) {
+ if (openedStream != null) {
+ try {
+ openedStream.close();
+ } catch (final IOException ignored) {
+ // Best-effort cleanup
+ }
+ }
+ if (urlConnection instanceof HttpURLConnection) {
+ ((HttpURLConnection) urlConnection).disconnect();
+ }
Review Comment:
Good catch, you are right that `openedStream.close()` was dead code.
Simplified as you suggested in 66b96e2:
- Split into two sequential try-catch blocks (resolve file/connection first,
then open stream)
- `disconnect()` only in the `IOException` catch, no `success` flag
- Moved `getLastModified()` before `getInputStream()` in the generic branch
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
