From 6702634494d0ca705604547f059d1fe3b35931c3 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 17 Aug 2023 07:37:40 -0500 Subject: [PATCH] Issue #10213 Ensuring proper usage of StartLog logging (to prevent accidental exceptions) (#10214) --- .../main/java/org/eclipse/jetty/start/BaseBuilder.java | 4 ++-- .../src/main/java/org/eclipse/jetty/start/FS.java | 5 +---- .../java/org/eclipse/jetty/start/FileInitializer.java | 6 +----- .../src/main/java/org/eclipse/jetty/start/Main.java | 10 +++++----- .../main/java/org/eclipse/jetty/start/PathFinder.java | 10 +++++----- .../java/org/eclipse/jetty/start/PathMatchers.java | 6 +++--- .../main/java/org/eclipse/jetty/start/StartArgs.java | 8 +++++--- .../java/org/eclipse/jetty/start/StartEnvironment.java | 4 ++-- .../eclipse/jetty/start/builders/StartDirBuilder.java | 2 +- .../jetty/start/fileinits/BaseHomeFileInitializer.java | 4 ++-- .../jetty/start/fileinits/DownloadFileInitializer.java | 2 +- .../jetty/start/fileinits/LocalFileInitializer.java | 2 +- .../start/fileinits/MavenLocalRepoFileInitializer.java | 4 ++-- 13 files changed, 31 insertions(+), 36 deletions(-) diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java index b0a460e81e4..2ab2ec06f6e 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java @@ -207,7 +207,7 @@ public class BaseBuilder List startLines = new ArrayList<>(); for (Path path : paths) { - StartLog.info("copy " + baseHome.toShortForm(path) + " into " + baseHome.toShortForm(startini)); + StartLog.info("copy %s into %s", baseHome.toShortForm(path), baseHome.toShortForm(startini)); startLines.add(""); startLines.add("# Config from " + baseHome.toShortForm(path)); startLines.addAll(Files.readAllLines(path)); @@ -250,7 +250,7 @@ public class BaseBuilder if (FS.ensureDirectoryExists(startd)) { - StartLog.info("mkdir " + baseHome.toShortForm(startd)); + StartLog.info("mkdir %s", baseHome.toShortForm(startd)); modified.set(true); } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java index 559161a8ebe..a1608d48f8c 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java @@ -23,12 +23,9 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.FileTime; -import java.util.HashMap; import java.util.Iterator; -import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jetty.util.FileID; @@ -130,7 +127,7 @@ public class FS if (!Files.isDirectory(path)) { // not a directory (as expected) - StartLog.warn("Not a directory: " + path); + StartLog.warn("Not a directory: %s", path); return false; } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java index 0d26cdcf894..7dd9a2b788a 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java @@ -14,14 +14,10 @@ package org.eclipse.jetty.start; import java.io.IOException; -import java.io.InputStream; -import java.net.HttpURLConnection; import java.net.URI; -import java.net.URLConnection; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardCopyOption; import java.util.HashSet; import java.util.Set; @@ -167,7 +163,7 @@ public abstract class FileInitializer { if (FS.ensureDirectoryExists(to)) { - StartLog.info("mkdir " + _basehome.toShortForm(to)); + StartLog.info("mkdir %s", _basehome.toShortForm(to)); modified = true; } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java index 4ccc2325ac3..c7bf741455a 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java @@ -186,8 +186,8 @@ public class Main StartLog.error("Do not start with ${jetty.base} == ${jetty.home}!"); else StartLog.error("No enabled jetty modules found!"); - StartLog.info("${jetty.home} = " + getBaseHome().getHomePath()); - StartLog.info("${jetty.base} = " + getBaseHome().getBasePath()); + StartLog.info("${jetty.home} = %s", getBaseHome().getHomePath()); + StartLog.info("${jetty.base} = %s", getBaseHome().getBasePath()); StartLog.error("Please create and/or configure a ${jetty.base} directory."); usageExit(ERR_INVOKE_MAIN); return; @@ -201,7 +201,7 @@ public class Main } catch (ClassNotFoundException e) { - StartLog.error("Unable to find: " + mainclass); + StartLog.error("Unable to find: %s", mainclass); StartLog.debug(e); usageExit(ERR_INVOKE_MAIN); return; @@ -489,7 +489,7 @@ public class Main final Process process = pbuilder.start(); Runtime.getRuntime().addShutdownHook(new Thread(() -> { - StartLog.debug("Destroying " + process); + StartLog.debug("Destroying %s", process); process.destroy(); })); @@ -685,7 +685,7 @@ public class Main } else { - StartLog.warn("Unable to find resource: " + resourceName); + StartLog.warn("Unable to find resource: %s", resourceName); } } catch (IOException e) diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathFinder.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathFinder.java index c9fb2c16f15..a139e142914 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathFinder.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathFinder.java @@ -42,7 +42,7 @@ public class PathFinder extends SimpleFileVisitor private void addHit(Path path) { String relPath = basePath.relativize(path).toString(); - StartLog.debug("Found [" + relPath + "] " + path); + StartLog.debug("Found [%s] %s", relPath, path); hits.put(relPath, path); } @@ -76,7 +76,7 @@ public class PathFinder extends SimpleFileVisitor { if (dirMatcher.matches(dir)) { - StartLog.trace("Following dir: " + dir); + StartLog.trace("Following dir: %s", dir); if (includeDirsInResults && fileMatcher.matches(dir)) { addHit(dir); @@ -85,7 +85,7 @@ public class PathFinder extends SimpleFileVisitor } else { - StartLog.trace("Skipping dir: " + dir); + StartLog.trace("Skipping dir: %s", dir); return FileVisitResult.SKIP_SUBTREE; } } @@ -131,7 +131,7 @@ public class PathFinder extends SimpleFileVisitor } else { - StartLog.trace("Ignoring file: " + file); + StartLog.trace("Ignoring file: %s", file); } return FileVisitResult.CONTINUE; } @@ -143,7 +143,7 @@ public class PathFinder extends SimpleFileVisitor { if (!NOTIFIED_PATHS.contains(file)) { - StartLog.warn("skipping detected filesystem loop: " + file); + StartLog.warn("skipping detected filesystem loop: %s", file); NOTIFIED_PATHS.add(file); } return FileVisitResult.SKIP_SUBTREE; diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathMatchers.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathMatchers.java index 27766f6d19c..437fbcd06cb 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathMatchers.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathMatchers.java @@ -84,7 +84,7 @@ public class PathMatchers // use FileSystem default pattern behavior if (pattern.startsWith("glob:") || pattern.startsWith("regex:")) { - StartLog.debug("Using Standard " + fs.getClass().getName() + " pattern: " + pattern); + StartLog.debug("Using Standard %s pattern: %s", fs.getClass().getName(), pattern); return fs.getPathMatcher(pattern); } @@ -93,14 +93,14 @@ public class PathMatchers if (isAbsolute(pattern)) { String pat = "glob:" + pattern; - StartLog.debug("Using absolute path pattern: " + pat); + StartLog.debug("Using absolute path pattern: %s", pat); return fs.getPathMatcher(pat); } // Doesn't start with filesystem root, then assume the pattern // is a relative file path pattern. String pat = "glob:**/" + pattern; - StartLog.debug("Using relative path pattern: " + pat); + StartLog.debug("Using relative path pattern: %s", pat); return fs.getPathMatcher(pat); } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java index 8e6e255c171..30d73fa2f08 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java @@ -428,9 +428,9 @@ public class StartArgs for (String rawlibref : module.getLibs()) { - StartLog.debug("rawlibref = " + rawlibref); + StartLog.debug("rawlibref = %s", rawlibref); String libref = environment.getProperties().expand(rawlibref); - StartLog.debug("expanded = " + libref); + StartLog.debug("expanded = %s", libref); for (Path libpath : baseHome.getPaths(libref)) { @@ -555,6 +555,7 @@ public class StartArgs if (parts.contains("path")) { Classpath classpath = jettyEnvironment.getClasspath(); + StartLog.debug("classpath=%s - isJPMS=%b", classpath, isJPMS()); if (isJPMS()) { Map> dirsAndFiles = StreamSupport.stream(classpath.spliterator(), false) @@ -605,6 +606,7 @@ public class StartArgs } generateJpmsArgs(cmd); + StartLog.debug("JPMS resulting cmd=%s", cmd.toCommandLine()); } else if (!classpath.isEmpty()) { @@ -1255,7 +1257,7 @@ public class StartArgs if (arg.startsWith("--add-to-start=") || arg.startsWith("--add-to-startd=")) { String value = Props.getValue(arg); - StartLog.warn("Option " + arg.split("=")[0] + " is deprecated! Instead use: --add-modules=%s", value); + StartLog.warn("Option %s is deprecated! Instead use: --add-modules=%s", arg.split("=")[0], value); } startModules.addAll(Props.getValues(arg)); run = false; diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartEnvironment.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartEnvironment.java index 6c328cdf595..a0ee205ca13 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartEnvironment.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartEnvironment.java @@ -231,9 +231,9 @@ public class StartEnvironment StartLog.debug("Expanding Libs"); for (String rawlibref : _libRefs) { - StartLog.debug("rawlibref = " + rawlibref); + StartLog.debug("rawlibref = %s", rawlibref); String libref = getProperties().expand(rawlibref); - StartLog.debug("expanded = " + libref); + StartLog.debug("expanded = %s", libref); // perform path escaping (needed by windows) libref = libref.replaceAll("\\\\([^\\\\])", "\\\\\\\\$1"); diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/builders/StartDirBuilder.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/builders/StartDirBuilder.java index 528c77921d3..909ab543e2d 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/builders/StartDirBuilder.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/builders/StartDirBuilder.java @@ -42,7 +42,7 @@ public class StartDirBuilder implements BaseBuilder.Config this.baseHome = baseBuilder.getBaseHome(); this.startDir = baseHome.getBasePath("start.d"); if (FS.ensureDirectoryExists(startDir)) - StartLog.info("mkdir " + baseHome.toShortForm(startDir)); + StartLog.info("mkdir %s", baseHome.toShortForm(startDir)); } @Override diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/BaseHomeFileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/BaseHomeFileInitializer.java index 202e228e921..55a5eb59e54 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/BaseHomeFileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/BaseHomeFileInitializer.java @@ -64,7 +64,7 @@ public class BaseHomeFileInitializer extends FileInitializer else if (FS.ensureDirectoryExists(destination)) { modified = true; - StartLog.info("mkdir " + _basehome.toShortForm(destination)); + StartLog.info("mkdir %s", _basehome.toShortForm(destination)); } copyDirectory(source, destination); @@ -74,7 +74,7 @@ public class BaseHomeFileInitializer extends FileInitializer if (FS.ensureDirectoryExists(destination.getParent())) { modified = true; - StartLog.info("mkdir " + _basehome.toShortForm(destination.getParent())); + StartLog.info("mkdir %s", _basehome.toShortForm(destination.getParent())); } if (!FS.exists(destination)) diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java index c6aca076d95..738c7999b77 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java @@ -61,7 +61,7 @@ public abstract class DownloadFileInitializer extends FileInitializer } if (FS.ensureDirectoryExists(destination.getParent())) - StartLog.info("mkdir " + _basehome.toShortForm(destination.getParent())); + StartLog.info("mkdir %s", _basehome.toShortForm(destination.getParent())); StartLog.info("download %s to %s", uri, _basehome.toShortForm(destination)); diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/LocalFileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/LocalFileInitializer.java index de084099132..9769d8cf8bc 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/LocalFileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/LocalFileInitializer.java @@ -73,7 +73,7 @@ public class LocalFileInitializer extends FileInitializer // Create directory boolean mkdir = FS.ensureDirectoryExists(destination); if (mkdir) - StartLog.info("mkdir " + _basehome.toShortForm(destination)); + StartLog.info("mkdir %s", _basehome.toShortForm(destination)); return mkdir; } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java index a9f1c3c4e12..cbd8674dadd 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java @@ -165,7 +165,7 @@ public class MavenLocalRepoFileInitializer extends DownloadFileInitializer if (!FS.canReadFile(localFile)) { if (FS.ensureDirectoryExists(localFile.getParent())) - StartLog.info("mkdir " + _basehome.toShortForm(localFile.getParent())); + StartLog.info("mkdir %s", _basehome.toShortForm(localFile.getParent())); download(coords, localFile); if (!FS.canReadFile(localFile)) { @@ -209,7 +209,7 @@ public class MavenLocalRepoFileInitializer extends DownloadFileInitializer if (localRepoFile != null) { if (FS.ensureDirectoryExists(destination.getParent())) - StartLog.info("mkdir " + _basehome.toShortForm(destination.getParent())); + StartLog.info("mkdir %s", _basehome.toShortForm(destination.getParent())); StartLog.info("copy %s to %s", localRepoFile, _basehome.toShortForm(destination)); Files.copy(localRepoFile, destination); return true;