Static analysis spotbugs (#458)

pankajagrawal16 · Pankaj Agrawal · web-flow · commit 9d4d9fe0c3a6 · 2021-07-06T15:20:34.000+02:00
* static analysis spot bug in build and fixes

* chore: spotbugs check on pr

Co-authored-by: Pankaj Agrawal &lt;pankaagr@amazon.com&gt;
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -48,7 +48,7 @@ jobs:
           distribution: 'zulu'
           java-version: ${{ matrix.java }}
       - name: Build with Maven
-        run: mvn -B package --file pom.xml
+        run: mvn -Pbuild-without-spotbugs -B package --file pom.xml
   savepr:
     runs-on: ubuntu-latest
     name: Save PR number if running on PR by dependabot
diff --git a/.github/workflows/spotbugs.yml b/.github/workflows/spotbugs.yml
@@ -0,0 +1,34 @@
+name: SpotBugs
+
+on:
+  pull_request:
+    branches:
+      - master
+    paths:
+      - 'powertools-core/**'
+      - 'powertools-logging/**'
+      - 'powertools-sqs/**'
+      - 'powertools-tracing/**'
+      - 'powertools-validation/**'
+      - 'powertools-parameters/**'
+      - 'powertools-metrics/**'
+      - 'pom.xml'
+      - '.github/workflows/**'
+jobs:
+  codecheck:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Setup java JDK 1.8
+        uses: actions/setup-java@v2
+        with:
+          distribution: 'zulu'
+          java-version: 8
+      - name: Build with Maven for spotbugs check to gather reports
+        run: mvn -Pbuild-with-spotbugs -B install --file pom.xml -DskipTests -Dmaven.javadoc.skip=true -Dspotbugs.failOnError=false
+      - uses: jwgmeligmeyling/spotbugs-github-action@master
+        with:
+          path: '**/spotbugsXml.xml'
+      # Can be simplified post this issue is fixed https://github.com/jwgmeligmeyling/spotbugs-github-action/issues/9
+      - name: Build with Maven for spotbugs check to mark build as fail if voilations found
+        run: mvn -Pbuild-with-spotbugs -B install --file pom.xml -DskipTests -Dmaven.javadoc.skip=true -Dspotbugs.failOnError=true
diff --git a/pom.xml b/pom.xml
@@ -399,10 +399,61 @@
             </build>
         </profile>
         <profile>
-            <id>build-extras</id>
+            <id>build-with-spotbugs</id>
             <activation>
                 <activeByDefault>true</activeByDefault>
             </activation>
+            <build>
+                <plugins>
+                    <plugin>
+                        <groupId>com.github.spotbugs</groupId>
+                        <artifactId>spotbugs-maven-plugin</artifactId>
+                        <version>4.2.2</version>
+                        <executions>
+                            <execution>
+                                <id>test</id>
+                                <goals>
+                                    <goal>check</goal>
+                                </goals>
+                            </execution>
+                        </executions>
+                        <configuration>
+                            <xmlOutput>true</xmlOutput>
+                        </configuration>
+                    </plugin>
+                    <plugin>
+                        <groupId>org.apache.maven.plugins</groupId>
+                        <artifactId>maven-source-plugin</artifactId>
+                        <executions>
+                            <execution>
+                                <id>attach-sources</id>
+                                <goals>
+                                    <goal>jar-no-fork</goal>
+                                </goals>
+                            </execution>
+                        </executions>
+                    </plugin>
+                    <plugin>
+                        <groupId>org.apache.maven.plugins</groupId>
+                        <artifactId>maven-javadoc-plugin</artifactId>
+                        <configuration>
+                            <doclint>none</doclint>
+                            <detectJavaApiLink>false</detectJavaApiLink>
+                        </configuration>
+                        <executions>
+                            <execution>
+                                <id>attach-javadocs</id>
+                                <goals>
+                                    <goal>jar</goal>
+                                </goals>
+                            </execution>
+                        </executions>
+                    </plugin>
+                </plugins>
+            </build>
+        </profile>
+        <profile>
+            <id>build-without-spotbugs</id>
             <build>
                 <plugins>
                     <plugin>
diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspect.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspect.java
@@ -236,7 +236,7 @@ private Object[] logFromInputStream(final ProceedingJoinPoint pjp) {
 
     private byte[] bytesFromInputStreamSafely(final InputStream inputStream) throws IOException {
         try (ByteArrayOutputStream out = new ByteArrayOutputStream();
-             InputStreamReader reader = new InputStreamReader(inputStream)) {
+             InputStreamReader reader = new InputStreamReader(inputStream, UTF_8)) {
             OutputStreamWriter writer = new OutputStreamWriter(out, UTF_8);
 
             IOUtils.copy(reader, writer);
diff --git a/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsUtils.java b/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsUtils.java
@@ -1,5 +1,6 @@
 package software.amazon.lambda.powertools.metrics;
 
+import java.util.Arrays;
 import java.util.Optional;
 import java.util.function.Consumer;
 
@@ -121,7 +122,7 @@ public static void withSingleMetric(final String name,
     }
 
     public static DimensionSet[] getDefaultDimensions() {
-        return defaultDimensions;
+        return Arrays.copyOf(defaultDimensions, defaultDimensions.length);
     }
 
     public static boolean hasDefaultDimension() {
diff --git a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SecretsProvider.java b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SecretsProvider.java
@@ -27,6 +27,8 @@
 import software.amazon.lambda.powertools.parameters.transform.TransformationManager;
 import software.amazon.lambda.powertools.parameters.transform.Transformer;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 /**
  * AWS Secrets Manager Parameter Provider<br/><br/>
  *
@@ -94,7 +96,7 @@ protected String getValue(String key) {
 
         String secretValue = client.getSecretValue(request).secretString();
         if (secretValue == null) {
-            secretValue = new String(Base64.getDecoder().decode(client.getSecretValue(request).secretBinary().asByteArray()));
+            secretValue = new String(Base64.getDecoder().decode(client.getSecretValue(request).secretBinary().asByteArray()), UTF_8);
         }
         return secretValue;
     }
diff --git a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/transform/Base64Transformer.java b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/transform/Base64Transformer.java
@@ -13,9 +13,11 @@
  */
 package software.amazon.lambda.powertools.parameters.transform;
 
+import java.util.Base64;
+
 import software.amazon.lambda.powertools.parameters.exception.TransformationException;
 
-import java.util.Base64;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 /**
  * Transformer that take a base64 encoded string and return a decoded string.
@@ -25,7 +27,7 @@ public class Base64Transformer extends BasicTransformer {
     @Override
     public String applyTransformation(String value) throws TransformationException {
         try {
-            return new String(Base64.getDecoder().decode(value));
+            return new String(Base64.getDecoder().decode(value), UTF_8);
         } catch (Exception e) {
             throw new TransformationException(e);
         }
diff --git a/powertools-validation/src/main/java/software/amazon/lambda/powertools/validation/ValidationConfig.java b/powertools-validation/src/main/java/software/amazon/lambda/powertools/validation/ValidationConfig.java
@@ -85,8 +85,13 @@ public SpecVersion.VersionFlag getSchemaVersion() {
      * @param <T> Must extends {@link BaseFunction}
      */
     public <T extends BaseFunction> void addFunction(T function) {
-        configuration.functionRegistry().extend(function);
-        jmesPath = new JacksonRuntime(configuration, getObjectMapper());
+        FunctionRegistry functionRegistryWithExtendedFunctions = configuration.functionRegistry().extend(function);
+
+        RuntimeConfiguration updatedConfig = new RuntimeConfiguration.Builder()
+                .withFunctionRegistry(functionRegistryWithExtendedFunctions)
+                .build();
+
+        jmesPath = new JacksonRuntime(updatedConfig, getObjectMapper());
     }
 
     /**
diff --git a/powertools-validation/src/main/java/software/amazon/lambda/powertools/validation/ValidationUtils.java b/powertools-validation/src/main/java/software/amazon/lambda/powertools/validation/ValidationUtils.java
@@ -13,6 +13,13 @@
  */
 package software.amazon.lambda.powertools.validation;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.JsonNodeType;
@@ -21,12 +28,6 @@
 import io.burt.jmespath.Expression;
 import software.amazon.lambda.powertools.validation.internal.ValidationAspect;
 
-import java.io.InputStream;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.stream.Collectors;
-
 /**
  * Validation utility, used to manually validate Json against Json Schema
  */
@@ -229,31 +230,37 @@ public static JsonSchema getJsonSchema(String schema) {
     public static JsonSchema getJsonSchema(String schema, boolean validateSchema) {
         JsonSchema jsonSchema = schemas.get(schema);
 
-        if (jsonSchema == null) {
-            if (schema.startsWith(CLASSPATH)) {
-                String filePath = schema.substring(CLASSPATH.length());
-                InputStream schemaStream = ValidationAspect.class.getResourceAsStream(filePath);
+        if (jsonSchema != null) {
+            return jsonSchema;
+        }
+
+        if (schema.startsWith(CLASSPATH)) {
+            String filePath = schema.substring(CLASSPATH.length());
+            try (InputStream schemaStream = ValidationAspect.class.getResourceAsStream(filePath)) {
                 if (schemaStream == null) {
                     throw new IllegalArgumentException("'" + schema + "' is invalid, verify '" + filePath + "' is in your classpath");
                 }
+
                 jsonSchema = ValidationConfig.get().getFactory().getSchema(schemaStream);
-            } else {
-                jsonSchema = ValidationConfig.get().getFactory().getSchema(schema);
+            } catch (IOException e) {
+                throw new IllegalArgumentException("'" + schema + "' is invalid, verify '" + filePath + "' is in your classpath");
             }
+        } else {
+            jsonSchema = ValidationConfig.get().getFactory().getSchema(schema);
+        }
 
-            if (validateSchema) {
-                String version = ValidationConfig.get().getSchemaVersion().toString();
-                try {
-                    validate(jsonSchema.getSchemaNode(),
-                            getJsonSchema("classpath:/schemas/meta_schema_" + version));
-                } catch (ValidationException ve) {
-                    throw new IllegalArgumentException("The schema " + schema + " is not valid, it does not respect the specification " + version, ve);
-                }
+        if (validateSchema) {
+            String version = ValidationConfig.get().getSchemaVersion().toString();
+            try {
+                validate(jsonSchema.getSchemaNode(),
+                        getJsonSchema("classpath:/schemas/meta_schema_" + version));
+            } catch (ValidationException ve) {
+                throw new IllegalArgumentException("The schema " + schema + " is not valid, it does not respect the specification " + version, ve);
             }
-
-            schemas.put(schema, jsonSchema);
         }
 
+        schemas.put(schema, jsonSchema);
+
         return jsonSchema;
     }
 
diff --git a/powertools-validation/src/main/java/software/amazon/lambda/powertools/validation/jmespath/Base64Function.java b/powertools-validation/src/main/java/software/amazon/lambda/powertools/validation/jmespath/Base64Function.java
@@ -13,16 +13,16 @@
  */
 package software.amazon.lambda.powertools.validation.jmespath;
 
+import java.nio.ByteBuffer;
+import java.util.Base64;
+import java.util.List;
+
 import io.burt.jmespath.Adapter;
 import io.burt.jmespath.JmesPathType;
 import io.burt.jmespath.function.ArgumentConstraints;
 import io.burt.jmespath.function.BaseFunction;
 import io.burt.jmespath.function.FunctionArgument;
 
-import java.nio.ByteBuffer;
-import java.util.Base64;
-import java.util.List;
-
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 /**
@@ -45,7 +45,7 @@ protected <T> T callFunction(Adapter<T> runtime, List<FunctionArgument<T>> argum
     }
 
     public static String decode(String encodedString) {
-        return new String(decode(encodedString.getBytes(UTF_8)));
+        return new String(decode(encodedString.getBytes(UTF_8)), UTF_8);
     }
 
     public static String decode(ByteBuffer byteBuffer) {
diff --git a/powertools-validation/src/main/java/software/amazon/lambda/powertools/validation/jmespath/Base64GZipFunction.java b/powertools-validation/src/main/java/software/amazon/lambda/powertools/validation/jmespath/Base64GZipFunction.java
@@ -13,12 +13,6 @@
  */
 package software.amazon.lambda.powertools.validation.jmespath;
 
-import io.burt.jmespath.Adapter;
-import io.burt.jmespath.JmesPathType;
-import io.burt.jmespath.function.ArgumentConstraints;
-import io.burt.jmespath.function.BaseFunction;
-import io.burt.jmespath.function.FunctionArgument;
-
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
@@ -27,6 +21,12 @@
 import java.util.List;
 import java.util.zip.GZIPInputStream;
 
+import io.burt.jmespath.Adapter;
+import io.burt.jmespath.JmesPathType;
+import io.burt.jmespath.function.ArgumentConstraints;
+import io.burt.jmespath.function.BaseFunction;
+import io.burt.jmespath.function.FunctionArgument;
+
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static software.amazon.lambda.powertools.validation.jmespath.Base64Function.decode;
 
@@ -68,7 +68,7 @@ public static String decompress(byte[] compressed) {
             }
             return out.toString();
         } catch (IOException e) {
-            return new String(compressed);
+            return new String(compressed, UTF_8);
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`package software.amazon.lambda.powertools.metrics;`
`2`	`2`
	`3`	`+import java.util.Arrays;`
`3`	`4`	`import java.util.Optional;`
`4`	`5`	`import java.util.function.Consumer;`
`5`	`6`
`@@ -121,7 +122,7 @@ public static void withSingleMetric(final String name,`
`121`	`122`	`}`
`122`	`123`
`123`	`124`	`public static DimensionSet[] getDefaultDimensions() {`
`124`		`- return defaultDimensions;`
	`125`	`+ return Arrays.copyOf(defaultDimensions, defaultDimensions.length);`
`125`	`126`	`}`
`126`	`127`
`127`	`128`	`public static boolean hasDefaultDimension() {`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,8 @@`
`27`	`27`	`import software.amazon.lambda.powertools.parameters.transform.TransformationManager;`
`28`	`28`	`import software.amazon.lambda.powertools.parameters.transform.Transformer;`
`29`	`29`
	`30`	`+import static java.nio.charset.StandardCharsets.UTF_8;`
	`31`	`+`
`30`	`32`	`/**`
`31`	`33`	`* AWS Secrets Manager Parameter Provider<br/><br/>`
`32`	`34`	`*`
`@@ -94,7 +96,7 @@ protected String getValue(String key) {`
`94`	`96`
`95`	`97`	`String secretValue = client.getSecretValue(request).secretString();`
`96`	`98`	`if (secretValue == null) {`
`97`		`- secretValue = new String(Base64.getDecoder().decode(client.getSecretValue(request).secretBinary().asByteArray()));`
	`99`	`+ secretValue = new String(Base64.getDecoder().decode(client.getSecretValue(request).secretBinary().asByteArray()), UTF_8);`
`98`	`100`	`}`
`99`	`101`	`return secretValue;`
`100`	`102`	`}`