Repository: knox Updated Branches: refs/heads/master 1c9020182 -> 5cb614de4
KNOX-892 - Fix FindBugs "Dodgy Code" issues (Colm O hEigeartaigh via lmccay) Project: http://git-wip-us.apache.org/repos/asf/knox/repo Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/5cb614de Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/5cb614de Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/5cb614de Branch: refs/heads/master Commit: 5cb614de4dcaf6da1a04d60db73e58306b4c6c22 Parents: 1c90201 Author: Larry McCay <[email protected]> Authored: Thu Apr 27 15:56:21 2017 -0400 Committer: Larry McCay <[email protected]> Committed: Thu Apr 27 15:56:21 2017 -0400 ---------------------------------------------------------------------- .../security/ldap/BaseDirectoryService.java | 2 +- .../loggers/test/TestMessageRecord.java | 36 ++++++++++++++++---- .../gateway/i18n/messages/MessagesTest.java | 4 +-- .../rewrite/impl/html/HtmlFilterReaderBase.java | 2 +- .../gateway/deploy/DeploymentFactory.java | 16 ++++----- .../topology/impl/DefaultTopologyService.java | 12 ++++--- .../org/apache/hadoop/gateway/util/KnoxCLI.java | 5 ++- .../gateway/util/ServiceDefinitionsLoader.java | 2 +- .../service/admin/TopologyMarshaller.java | 2 +- .../gateway/service/knoxsso/WebSSOResource.java | 2 +- .../dispatch/AbstractGatewayDispatch.java | 16 +++++---- .../dispatch/DefaultHttpClientFactory.java | 2 +- .../dispatch/PassAllHeadersDispatch.java | 10 +++--- .../hadoop/gateway/GatewayTestConfig.java | 2 +- .../gateway/util/urltemplate/Expander.java | 6 ++-- 15 files changed, 73 insertions(+), 46 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-demo-ldap/src/main/java/org/apache/hadoop/gateway/security/ldap/BaseDirectoryService.java ---------------------------------------------------------------------- diff --git a/gateway-demo-ldap/src/main/java/org/apache/hadoop/gateway/security/ldap/BaseDirectoryService.java b/gateway-demo-ldap/src/main/java/org/apache/hadoop/gateway/security/ldap/BaseDirectoryService.java index d18cef4..0ceca72 100644 --- a/gateway-demo-ldap/src/main/java/org/apache/hadoop/gateway/security/ldap/BaseDirectoryService.java +++ b/gateway-demo-ldap/src/main/java/org/apache/hadoop/gateway/security/ldap/BaseDirectoryService.java @@ -709,7 +709,7 @@ public class BaseDirectoryService implements DirectoryService //noinspection MismatchedQueryAndUpdateOfCollection List<LdifEntry> cloned = new ArrayList<LdifEntry>(); cloned.addAll( testEntries ); - this.testEntries = testEntries; + this.testEntries = cloned; } http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/loggers/test/TestMessageRecord.java ---------------------------------------------------------------------- diff --git a/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/loggers/test/TestMessageRecord.java b/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/loggers/test/TestMessageRecord.java index 63c4634..46edf98 100644 --- a/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/loggers/test/TestMessageRecord.java +++ b/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/loggers/test/TestMessageRecord.java @@ -24,12 +24,12 @@ import org.apache.hadoop.gateway.i18n.messages.MessageLevel; */ public class TestMessageRecord { - public TestMessageLogger logger; - public StackTraceElement caller; - public MessageLevel level; - public String id; - public String message; - public Throwable throwable; + private final TestMessageLogger logger; + private final StackTraceElement caller; + private final MessageLevel level; + private final String id; + private final String message; + private final Throwable throwable; public TestMessageRecord( TestMessageLogger logger, StackTraceElement caller, MessageLevel level, String id, String message, Throwable throwable ) { this.logger = logger; @@ -39,5 +39,29 @@ public class TestMessageRecord { this.message = message; this.throwable = throwable; } + + public TestMessageLogger getLogger() { + return logger; + } + + public StackTraceElement getCaller() { + return caller; + } + + public MessageLevel getLevel() { + return level; + } + + public String getId() { + return id; + } + + public String getMessage() { + return message; + } + + public Throwable getThrowable() { + return throwable; + } } http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-i18n/src/test/java/org/apache/hadoop/gateway/i18n/messages/MessagesTest.java ---------------------------------------------------------------------- diff --git a/gateway-i18n/src/test/java/org/apache/hadoop/gateway/i18n/messages/MessagesTest.java b/gateway-i18n/src/test/java/org/apache/hadoop/gateway/i18n/messages/MessagesTest.java index f9a6f2c..56f0b69 100644 --- a/gateway-i18n/src/test/java/org/apache/hadoop/gateway/i18n/messages/MessagesTest.java +++ b/gateway-i18n/src/test/java/org/apache/hadoop/gateway/i18n/messages/MessagesTest.java @@ -46,8 +46,8 @@ public class MessagesTest { TestMessageRecord record = logger.records.get( 0 ); - assertThat( record.caller.getClassName(), is( this.getClass().getName() ) ); - assertThat( record.caller.getMethodName(), is( "testFirst" ) ); + assertThat( record.getCaller().getClassName(), is( this.getClass().getName() ) ); + assertThat( record.getCaller().getMethodName(), is( "testFirst" ) ); } http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/html/HtmlFilterReaderBase.java ---------------------------------------------------------------------- diff --git a/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/html/HtmlFilterReaderBase.java b/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/html/HtmlFilterReaderBase.java index 1eb2c1d..8866f24 100644 --- a/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/html/HtmlFilterReaderBase.java +++ b/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/html/HtmlFilterReaderBase.java @@ -290,7 +290,7 @@ public abstract class HtmlFilterReaderBase extends Reader implements UrlRewriteF prefix = name.substring( 0, colon ); local = ( colon + 1 < name.length() ? name.substring( colon + 1 ) : "" ); } - String namespace = ( prefix == null ) ? null : getNamespace( prefix ); + String namespace = getNamespace( prefix ); return new QName( namespace, local, prefix ); } http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-server/src/main/java/org/apache/hadoop/gateway/deploy/DeploymentFactory.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/main/java/org/apache/hadoop/gateway/deploy/DeploymentFactory.java b/gateway-server/src/main/java/org/apache/hadoop/gateway/deploy/DeploymentFactory.java index d834e74..32628d8 100644 --- a/gateway-server/src/main/java/org/apache/hadoop/gateway/deploy/DeploymentFactory.java +++ b/gateway-server/src/main/java/org/apache/hadoop/gateway/deploy/DeploymentFactory.java @@ -756,15 +756,13 @@ public abstract class DeploymentFactory { // Explicit configuration that is wrong should just fail // rather than randomly select a provider. Implicit default // providers can be selected when no name is provided. - if (name != null) { - if (contributor == null || !contributor.getRole().equals(role) || - !contributor.getName().equals(name)) { - throw new DeploymentException( - "Failed to contribute provider. Role: " + - role + " Name: " + name + ". Please check the topology for" + - " errors in name and role and that the provider is " + - "on the classpath."); - } + if (contributor == null || !contributor.getRole().equals(role) || + !contributor.getName().equals(name)) { + throw new DeploymentException( + "Failed to contribute provider. Role: " + + role + " Name: " + name + ". Please check the topology for" + + " errors in name and role and that the provider is " + + "on the classpath."); } } return contributor; http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java b/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java index 8f2cb12..eec26ce 100644 --- a/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java +++ b/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java @@ -229,7 +229,7 @@ public class DefaultTopologyService private Map<File, Topology> loadTopologies(File directory) { Map<File, Topology> map = new HashMap<File, Topology>(); - if (directory.exists() && directory.canRead()) { + if (directory.isDirectory() && directory.canRead()) { for (File file : directory.listFiles(this)) { try { Topology loadTopology = loadTopology(file); @@ -331,7 +331,7 @@ public class DefaultTopologyService public void deleteTopology(Topology t) { File topoDir = directory; - if(topoDir.exists() && topoDir.canRead()) { + if(topoDir.isDirectory() && topoDir.canRead()) { File[] results = topoDir.listFiles(); for (File f : results) { String fName = FilenameUtils.getBaseName(f.getName()); @@ -357,9 +357,11 @@ public class DefaultTopologyService public Map<String, List<String>> getServiceTestURLs(Topology t, GatewayConfig config) { File tFile = null; Map<String, List<String>> urls = new HashMap<>(); - for(File f : directory.listFiles()){ - if(FilenameUtils.removeExtension(f.getName()).equals(t.getName())){ - tFile = f; + if(directory.isDirectory() && directory.canRead()) { + for(File f : directory.listFiles()){ + if(FilenameUtils.removeExtension(f.getName()).equals(t.getName())){ + tFile = f; + } } } Set<ServiceDefinition> defs; http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-server/src/main/java/org/apache/hadoop/gateway/util/KnoxCLI.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/main/java/org/apache/hadoop/gateway/util/KnoxCLI.java b/gateway-server/src/main/java/org/apache/hadoop/gateway/util/KnoxCLI.java index 6f8565d..30356ec 100644 --- a/gateway-server/src/main/java/org/apache/hadoop/gateway/util/KnoxCLI.java +++ b/gateway-server/src/main/java/org/apache/hadoop/gateway/util/KnoxCLI.java @@ -410,7 +410,6 @@ public class KnoxCLI extends Configured implements Tool { } private abstract class Command { - protected Service provider = null; public boolean validate() { return true; @@ -937,7 +936,7 @@ public class KnoxCLI extends Configured implements Tool { } else if(cluster == null) { // The following block of code retreieves the list of files in the topologies directory File tops = new File(topDir + "/topologies"); - if(tops.exists()) { + if(tops.isDirectory()) { out.println("List of files available in the topologies directory"); for (File f : tops.listFiles()) { if(f.getName().endsWith(".xml")) { @@ -996,7 +995,7 @@ public class KnoxCLI extends Configured implements Tool { File tops = new File(confDir + "/topologies"); out.println("List of files available in the topologies directory"); out.println(tops.toString()); - if(tops.exists()) { + if(tops.isDirectory()) { for (File f : tops.listFiles()) { if(f.getName().endsWith(".xml")) { String fName = f.getName().replace(".xml", ""); http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-server/src/main/java/org/apache/hadoop/gateway/util/ServiceDefinitionsLoader.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/main/java/org/apache/hadoop/gateway/util/ServiceDefinitionsLoader.java b/gateway-server/src/main/java/org/apache/hadoop/gateway/util/ServiceDefinitionsLoader.java index e94f52e..4ed148d 100644 --- a/gateway-server/src/main/java/org/apache/hadoop/gateway/util/ServiceDefinitionsLoader.java +++ b/gateway-server/src/main/java/org/apache/hadoop/gateway/util/ServiceDefinitionsLoader.java @@ -108,7 +108,7 @@ public class ServiceDefinitionsLoader { } }, TrueFileFilter.INSTANCE); } else { - return files = new HashSet<File>(); + files = new HashSet<File>(); } return files; http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-service-admin/src/main/java/org/apache/hadoop/gateway/service/admin/TopologyMarshaller.java ---------------------------------------------------------------------- diff --git a/gateway-service-admin/src/main/java/org/apache/hadoop/gateway/service/admin/TopologyMarshaller.java b/gateway-service-admin/src/main/java/org/apache/hadoop/gateway/service/admin/TopologyMarshaller.java index 3eb9384..eac4860 100644 --- a/gateway-service-admin/src/main/java/org/apache/hadoop/gateway/service/admin/TopologyMarshaller.java +++ b/gateway-service-admin/src/main/java/org/apache/hadoop/gateway/service/admin/TopologyMarshaller.java @@ -83,7 +83,7 @@ public class TopologyMarshaller implements MessageBodyWriter<Topology>, MessageB public Topology readFrom(Class<Topology> type, Type genericType, Annotation[] annotations, MediaType mediaType, MultivaluedMap<String, String> httpHeaders, InputStream entityStream) throws IOException, WebApplicationException { try { if(isReadable(type, genericType, annotations, mediaType)) { - Map<String, Object> properties = Collections.EMPTY_MAP; + Map<String, Object> properties = Collections.emptyMap(); JAXBContext context = JAXBContext.newInstance(new Class[]{Topology.class}, properties); InputStream is = entityStream; Unmarshaller u = context.createUnmarshaller(); http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-service-knoxsso/src/main/java/org/apache/hadoop/gateway/service/knoxsso/WebSSOResource.java ---------------------------------------------------------------------- diff --git a/gateway-service-knoxsso/src/main/java/org/apache/hadoop/gateway/service/knoxsso/WebSSOResource.java b/gateway-service-knoxsso/src/main/java/org/apache/hadoop/gateway/service/knoxsso/WebSSOResource.java index 417fe66..7cc5378 100644 --- a/gateway-service-knoxsso/src/main/java/org/apache/hadoop/gateway/service/knoxsso/WebSSOResource.java +++ b/gateway-service-knoxsso/src/main/java/org/apache/hadoop/gateway/service/knoxsso/WebSSOResource.java @@ -163,7 +163,7 @@ public class WebSSOResource { // we need to get it from the request parameters removeOriginalUrlCookie = false; original = getOriginalUrlFromQueryParams(); - if (original == null) { + if (original.isEmpty()) { log.originalURLNotFound(); throw new WebApplicationException("Original URL not found in the request.", Response.Status.BAD_REQUEST); } http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/AbstractGatewayDispatch.java ---------------------------------------------------------------------- diff --git a/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/AbstractGatewayDispatch.java b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/AbstractGatewayDispatch.java index 6ba16c6..4ae5e7a 100644 --- a/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/AbstractGatewayDispatch.java +++ b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/AbstractGatewayDispatch.java @@ -36,17 +36,19 @@ import java.util.Set; public abstract class AbstractGatewayDispatch implements Dispatch { private static final int STREAM_COPY_BUFFER_SIZE = 4096; - private static Set<String> REQUEST_EXCLUDE_HEADERS; - + private static final Set<String> REQUEST_EXCLUDE_HEADERS = new HashSet<>(); + + static { + REQUEST_EXCLUDE_HEADERS.add("Host"); + REQUEST_EXCLUDE_HEADERS.add("Authorization"); + REQUEST_EXCLUDE_HEADERS.add("Content-Length"); + REQUEST_EXCLUDE_HEADERS.add("Transfer-Encoding"); + } + protected HttpClient client; @Override public void init() { - REQUEST_EXCLUDE_HEADERS = new HashSet<>(); - REQUEST_EXCLUDE_HEADERS.add("Host"); - REQUEST_EXCLUDE_HEADERS.add("Authorization"); - REQUEST_EXCLUDE_HEADERS.add("Content-Length"); - REQUEST_EXCLUDE_HEADERS.add("Transfer-Encoding"); } protected void writeResponse(HttpServletRequest request, HttpServletResponse response, InputStream stream ) http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultHttpClientFactory.java ---------------------------------------------------------------------- diff --git a/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultHttpClientFactory.java b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultHttpClientFactory.java index 6f75f9e..83cf7c0 100644 --- a/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultHttpClientFactory.java +++ b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultHttpClientFactory.java @@ -120,7 +120,7 @@ public class DefaultHttpClientFactory implements HttpClientFactory { @Override public List<Cookie> getCookies() { - return Collections.EMPTY_LIST; + return Collections.emptyList(); } @Override http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/PassAllHeadersDispatch.java ---------------------------------------------------------------------- diff --git a/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/PassAllHeadersDispatch.java b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/PassAllHeadersDispatch.java index 92f9ed7..e493841 100644 --- a/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/PassAllHeadersDispatch.java +++ b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/PassAllHeadersDispatch.java @@ -23,18 +23,20 @@ import java.util.Set; public class PassAllHeadersDispatch extends DefaultDispatch { - private static Set<String> REQUEST_EXCLUDE_HEADERS; + private static final Set<String> REQUEST_EXCLUDE_HEADERS = new HashSet<>(); + + static { + REQUEST_EXCLUDE_HEADERS.add("Content-Length"); + } @Override public void init() { super.init(); - REQUEST_EXCLUDE_HEADERS = new HashSet<>(); - REQUEST_EXCLUDE_HEADERS.add("Content-Length"); } @Override public Set<String> getOutboundResponseExcludeHeaders() { - return Collections.EMPTY_SET; + return Collections.emptySet(); } @Override http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-test-release-utils/src/main/java/org/apache/hadoop/gateway/GatewayTestConfig.java ---------------------------------------------------------------------- diff --git a/gateway-test-release-utils/src/main/java/org/apache/hadoop/gateway/GatewayTestConfig.java b/gateway-test-release-utils/src/main/java/org/apache/hadoop/gateway/GatewayTestConfig.java index b0253cb..41588b3 100644 --- a/gateway-test-release-utils/src/main/java/org/apache/hadoop/gateway/GatewayTestConfig.java +++ b/gateway-test-release-utils/src/main/java/org/apache/hadoop/gateway/GatewayTestConfig.java @@ -363,7 +363,7 @@ public class GatewayTestConfig extends Configuration implements GatewayConfig { @Override public List<String> getGlobalRulesServices() { - return Collections.EMPTY_LIST; + return Collections.emptyList(); } /* (non-Javadoc) http://git-wip-us.apache.org/repos/asf/knox/blob/5cb614de/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Expander.java ---------------------------------------------------------------------- diff --git a/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Expander.java b/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Expander.java index e37c780..22cfe9e 100644 --- a/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Expander.java +++ b/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Expander.java @@ -34,7 +34,7 @@ public class Expander { private static Params EMPTY_PARAMS = new EmptyParams(); public static URI expand( Template template, Params params, Evaluator evaluator ) throws URISyntaxException { - return new Expander().expandToUri( template, params, evaluator ); + return Expander.expandToUri( template, params, evaluator ); } public static URI expandToUri( Template template, Params params, Evaluator evaluator ) throws URISyntaxException { @@ -307,12 +307,12 @@ public class Expander { private static class EmptyParams implements Params { @Override public Set<String> getNames() { - return Collections.EMPTY_SET; + return Collections.emptySet(); } @Override public List<String> resolve( String name ) { - return Collections.EMPTY_LIST; + return Collections.emptyList(); } }
