Skip to content

Switch to spotbugs and fix some issues #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 10, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<parent>
<groupId>org.jenkins-ci</groupId>
<artifactId>jenkins</artifactId>
<version>1.51</version>
<version>1.52</version>
<relativePath />
</parent>

Expand All @@ -20,8 +20,8 @@
<jetty.version>9.4.19.v20190610</jetty.version>
<alpn.api.version>1.1.3.v20160715</alpn.api.version>
<java.level>8</java.level>
<!-- TODO: remove once FindBugs issues are fixed. 57 so far -->
<findbugs.failOnError>false</findbugs.failOnError>
<!-- TODO: remove once SpotBugs issues are fixed. 57 so far -->
<spotbugs.failOnError>false</spotbugs.failOnError>
</properties>

<licenses>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ protected void configureSsl( Map args, Server server ) throws IOException
}
} else if (opensslCert!=null) {
// load from openssl style key files
CertificateFactory cf = CertificateFactory.getInstance( "X509");
CertificateFactory cf = CertificateFactory.getInstance("X509");
try(InputStream inputStream = new FileInputStream( opensslCert); //
FileReader fileReader = new FileReader(opensslKey)) {
Certificate cert = cf.generateCertificate(inputStream);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/winstone/Ajp13ConnectorFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
/**
* Implements the main listener daemon thread. This is the class that gets
* launched by the command line, and owns the server socket, etc.
*
*
* @author mailto: <a href="[email protected]">Rick Knowles</a>
* @version $Id: Ajp13ConnectorFactory.java,v 1.12 2006/03/24 17:24:22 rickknowles Exp $
*/
public class Ajp13ConnectorFactory implements ConnectorFactory {
public boolean start(Map args, Server server) throws IOException {
int listenPort = Option.AJP13_PORT.get(args);
String listenAddress = Option.AJP13_LISTEN_ADDRESS.get(args);
// String listenAddress = Option.AJP13_LISTEN_ADDRESS.get(args);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If unused, please.do one of the following..

  • delete it
  • Create a follow-up bug, link it from a TODO comment

Looks like it is a bug unless I miss something


if (listenPort < 0) {
return false;
Expand Down
138 changes: 77 additions & 61 deletions src/main/java/winstone/HostConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
/**
* Manages the references to individual webapps within the container. This object handles
* the mapping of url-prefixes to webapps, and init and shutdown of any webapps it manages.
*
*
* @author <a href="mailto:[email protected]">Rick Knowles</a>
* @version $Id: HostConfiguration.java,v 1.8 2007/08/02 06:16:00 rickknowles Exp $
*/
Expand All @@ -61,8 +61,8 @@ public HostConfiguration(Server server, String hostname, ClassLoader commonLibCL

try {
// Build the realm
Class realmClass = Option.REALM_CLASS_NAME.get(args, LoginService.class, commonLibCL);
Constructor realmConstr = realmClass.getConstructor(Map.class);
Class<? extends LoginService> realmClass = Option.REALM_CLASS_NAME.get(args, LoginService.class, commonLibCL);
Constructor<? extends LoginService> realmConstr = realmClass.getConstructor(Map.class);
loginService = (LoginService) realmConstr.newInstance(args);
} catch (Throwable err) {
throw (IOException)new IOException("Failed to setup authentication realm").initCause(err);
Expand Down Expand Up @@ -133,10 +133,10 @@ private void loadBuiltinMimeTypes() {
*/
private Handler configureAccessLog(Handler handler, String webAppName) {
try {
Class loggerClass = Option.ACCESS_LOGGER_CLASSNAME.get(args, RequestLog.class, commonLibCL);
Class<? extends RequestLog> loggerClass = Option.ACCESS_LOGGER_CLASSNAME.get(args, RequestLog.class, commonLibCL);
if (loggerClass!=null) {
// Build the realm
Constructor loggerConstr = loggerClass.getConstructor(String.class, Map.class);
Constructor<? extends RequestLog> loggerConstr = loggerClass.getConstructor(String.class, Map.class);
RequestLogHandler rlh = new RequestLogHandler();
rlh.setHandler(handler);
rlh.setRequestLog((RequestLog) loggerConstr.newInstance(webAppName, args));
Expand Down Expand Up @@ -192,7 +192,7 @@ public void postConfigure() throws Exception {
public String getHostname() {
return this.hostname;
}

public void reloadWebApp(String prefix) {
WebAppContext webApp = this.webapps.get(prefix);
if (webApp != null) {
Expand Down Expand Up @@ -230,10 +230,12 @@ protected File getWebRoot(File requestedWebroot, File warfile) throws IOExceptio
File tempFile = File.createTempFile("dummy", "dummy");
String userName = System.getProperty("user.name");
unzippedDir = new File(tempFile.getParent(),
(userName != null ? WinstoneResourceBundle.globalReplace(userName,
(userName != null ? WinstoneResourceBundle.globalReplace(userName,
new String[][] {{"/", ""}, {"\\", ""}, {",", ""}}) + "/" : "") +
"winstone/" + warfile.getName());
tempFile.delete();
if (!tempFile.delete()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or there is a Commons IO utility, or you can use java.nio.file.Files.delete which throws a meaningful exception.

Logger.logDirectMessage(Logger.WARNING, null, "Failed To delete dummy file", null);
}
}
if (unzippedDir.exists()) {
if (!unzippedDir.isDirectory()) {
Expand All @@ -250,43 +252,51 @@ protected File getWebRoot(File requestedWebroot, File warfile) throws IOExceptio
if(!timestampFile.exists() || Math.abs(timestampFile.lastModified()- warfile.lastModified())>1000) {
// contents of the target directory is inconsistent from the war.
deleteRecursive(unzippedDir);
unzippedDir.mkdirs();
if (!unzippedDir.mkdirs()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Logger.logDirectMessage(Logger.WARNING, null, "Failed to recreate dirs " + unzippedDir.getAbsolutePath(), null);
}
} else {
// files are up to date
return unzippedDir;
}

// Iterate through the files
byte buffer[] = new byte[8192];
JarFile warArchive = new JarFile(warfile);
for (Enumeration e = warArchive.entries(); e.hasMoreElements();) {
JarEntry element = (JarEntry) e.nextElement();
if (element.isDirectory()) {
continue;
}
String elemName = element.getName();
try (JarFile warArchive = new JarFile(warfile)) {
for (Enumeration<JarEntry> e = warArchive.entries(); e.hasMoreElements();) {
JarEntry element = e.nextElement();
if (element.isDirectory()) {
continue;
}
String elemName = element.getName();

// If archive date is newer than unzipped file, overwrite
File outFile = new File(unzippedDir, elemName);
if (outFile.exists() && (outFile.lastModified() > warfile.lastModified())) {
continue;
}
outFile.getParentFile().mkdirs();

// Copy out the extracted file
try (InputStream inContent = warArchive.getInputStream(element);
OutputStream outStream = new FileOutputStream(outFile)) {
int readBytes = inContent.read( buffer );
while ( readBytes != -1 ) {
outStream.write( buffer, 0, readBytes );
readBytes = inContent.read( buffer );
// If archive date is newer than unzipped file, overwrite
File outFile = new File(unzippedDir, elemName);
if (outFile.exists() && (outFile.lastModified() > warfile.lastModified())) {
continue;
}
File parentOutFile = outFile.getParentFile();
if (!parentOutFile.mkdirs()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can drop Java 6 later and start using Files here.
CC @olamy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll clean this bit up then

Logger.logDirectMessage(Logger.WARNING, null, "Failed to create dirs " + parentOutFile.getAbsolutePath(), null);
}

// Copy out the extracted file
try (InputStream inContent = warArchive.getInputStream(element);
OutputStream outStream = new FileOutputStream(outFile)) {
int readBytes = inContent.read( buffer );
while ( readBytes != -1 ) {
outStream.write( buffer, 0, readBytes );
readBytes = inContent.read( buffer );
}
}
}
}

// extraction completed
new FileOutputStream(timestampFile).close();
timestampFile.setLastModified(warfile.lastModified());
if(!timestampFile.setLastModified(warfile.lastModified())) {
Logger.logDirectMessage(Logger.WARNING, null, "Failed to set timestamp " + timestampFile.getAbsolutePath(), null);
}

// Return webroot
return unzippedDir;
Expand All @@ -302,7 +312,9 @@ private void deleteRecursive(File dir) {
deleteRecursive(child);
}
}
dir.delete();
if (!dir.delete()) {
Logger.logDirectMessage(Logger.WARNING, null, "Failed to delete dirs " + dir.getAbsolutePath(), null);
}
}

protected ContextHandlerCollection initMultiWebappDir(File webappsDir) {
Expand All @@ -317,42 +329,46 @@ protected ContextHandlerCollection initMultiWebappDir(File webappsDir) {
throw new WinstoneException(Launcher.RESOURCES.getString("HostConfig.WebAppDirIsNotDirectory", webappsDir.getPath()));
} else {
File children[] = webappsDir.listFiles();
for (File aChildren : children) {
String childName = aChildren.getName();

// Check any directories for warfiles that match, and skip: only deploy the war file
if (aChildren.isDirectory()) {
File matchingWarFile = new File(webappsDir, aChildren.getName() + ".war");
if (matchingWarFile.exists() && matchingWarFile.isFile()) {
Logger.log(Logger.DEBUG, Launcher.RESOURCES, "HostConfig.SkippingWarfileDir", childName);
} else {
String prefix = childName.equalsIgnoreCase("ROOT") ? "" : "/" + childName;
if (children != null) {
for (File aChildren : children) {
String childName = aChildren.getName();

// Check any directories for warfiles that match, and skip: only deploy the war file
if (aChildren.isDirectory()) {
File matchingWarFile = new File(webappsDir, aChildren.getName() + ".war");
if (matchingWarFile.exists() && matchingWarFile.isFile()) {
Logger.log(Logger.DEBUG, Launcher.RESOURCES, "HostConfig.SkippingWarfileDir", childName);
} else {
String prefix = childName.equalsIgnoreCase("ROOT") ? "" : "/" + childName;
if (!this.webapps.containsKey(prefix)) {
try {
WebAppContext context = create(aChildren, prefix);
webApps.addHandler(configureAccessLog(context,childName));
Logger.log(Logger.INFO, Launcher.RESOURCES, "HostConfig.DeployingWebapp", childName);
} catch (Throwable err) {
Logger.log(Logger.ERROR, Launcher.RESOURCES, "HostConfig.WebappInitError", prefix, err);
}
}
}
} else if (childName.endsWith(".war")) {
String outputName = childName.substring(0, childName.lastIndexOf(".war"));
String prefix = outputName.equalsIgnoreCase("ROOT") ? "" : "/" + outputName;

if (!this.webapps.containsKey(prefix)) {
File outputDir = new File(webappsDir, outputName);
if (!outputDir.mkdirs()) {
Logger.logDirectMessage(Logger.WARNING, null, "Failed to mkdirs " + outputDir.getAbsolutePath(), null);
}
try {
WebAppContext context = create(aChildren, prefix);
webApps.addHandler(configureAccessLog(context,childName));
WebAppContext context = create(
getWebRoot(new File(webappsDir, outputName), aChildren), prefix);
webApps.addHandler(configureAccessLog(context,outputName));
Logger.log(Logger.INFO, Launcher.RESOURCES, "HostConfig.DeployingWebapp", childName);
} catch (Throwable err) {
Logger.log(Logger.ERROR, Launcher.RESOURCES, "HostConfig.WebappInitError", prefix, err);
}
}
}
} else if (childName.endsWith(".war")) {
String outputName = childName.substring(0, childName.lastIndexOf(".war"));
String prefix = outputName.equalsIgnoreCase("ROOT") ? "" : "/" + outputName;

if (!this.webapps.containsKey(prefix)) {
File outputDir = new File(webappsDir, outputName);
outputDir.mkdirs();
try {
WebAppContext context = create(
getWebRoot(new File(webappsDir, outputName), aChildren), prefix);
webApps.addHandler(configureAccessLog(context,outputName));
Logger.log(Logger.INFO, Launcher.RESOURCES, "HostConfig.DeployingWebapp", childName);
} catch (Throwable err) {
Logger.log(Logger.ERROR, Launcher.RESOURCES, "HostConfig.WebappInitError", prefix, err);
}
}
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions src/main/java/winstone/HostGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,28 @@

/**
* Manages the references to individual hosts within the container. This object handles
* the mapping of ip addresses and hostnames to groups of webapps, and init and
* the mapping of ip addresses and hostnames to groups of webapps, and init and
* shutdown of any hosts it manages.
*
*
* @author <a href="mailto:[email protected]">Rick Knowles</a>
* @version $Id: HostGroup.java,v 1.4 2006/03/24 17:24:21 rickknowles Exp $
*/
public class HostGroup {

private final static String DEFAULT_HOSTNAME = "default";
private final Server server;

// private Map args;
private Map hostConfigs;
private Map<String, HostConfiguration> hostConfigs;
private String defaultHostName;

public HostGroup(
Server server, ClassLoader commonLibCL,
File commonLibCLPaths[], Map args) throws IOException {
this.server = server;
// this.args = args;
this.hostConfigs = new Hashtable();
this.hostConfigs = new Hashtable<>();

// Is this the single or multiple configuration ? Check args
File webappsDir = Option.WEBAPPS_DIR.get(args);

Expand All @@ -58,7 +58,7 @@ public HostConfiguration getHostByName(String hostname) {
}
return (HostConfiguration) this.hostConfigs.get(this.defaultHostName);
}

protected void initHost(File webappsDir, String hostname,
ClassLoader commonLibCL,
File commonLibCLPaths[], Map args) throws IOException {
Expand Down
Loading