From 4e1a34f948f4ef45e38e94da30b3af0994d16e8f Mon Sep 17 00:00:00 2001 From: Pankaj Agrawal Date: Tue, 6 Jul 2021 13:39:48 +0200 Subject: [PATCH 1/2] static analysis spot bug in build and fixes --- .github/workflows/build.yml | 2 +- pom.xml | 54 ++++++++++++++++++- .../logging/internal/LambdaLoggingAspect.java | 2 +- .../powertools/metrics/MetricsUtils.java | 3 +- .../parameters/SecretsProvider.java | 4 +- .../transform/Base64Transformer.java | 6 ++- .../validation/ValidationConfig.java | 9 +++- .../validation/ValidationUtils.java | 51 ++++++++++-------- .../validation/jmespath/Base64Function.java | 10 ++-- .../jmespath/Base64GZipFunction.java | 14 ++--- 10 files changed, 112 insertions(+), 43 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0413a8ca9..774120b5a 100644 --- 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/pom.xml b/pom.xml index 9d05cf6d4..3bba8f49d 100644 --- a/pom.xml +++ b/pom.xml @@ -399,10 +399,62 @@ - build-extras + build-with-spotbugs true + + + + com.github.spotbugs + spotbugs-maven-plugin + 4.2.2 + + + test + + check + + + + + true + true + + + + org.apache.maven.plugins + maven-source-plugin + + + attach-sources + + jar-no-fork + + + + + + org.apache.maven.plugins + maven-javadoc-plugin + + none + false + + + + attach-javadocs + + jar + + + + + + + + + build-without-spotbugs 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 index 06e1e8067..74da2a8ce 100644 --- 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 index f7465e663..443ef3976 100644 --- 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 index d5ddb5076..a0a8fe51e 100644 --- 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

* @@ -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 index b9af4fbd4..944f4f03c 100644 --- 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 index 7e3bf3731..191c50107 100644 --- 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 Must extends {@link BaseFunction} */ public 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 index f57e1c251..e45df21da 100644 --- 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 index 2a418d086..c5693f8a7 100644 --- 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 callFunction(Adapter runtime, List> 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 index e5eb52bd4..bd4b338c4 100644 --- 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); } } From b204d3fc790e39a9b2e1d2e4b527c67d7e7fc09d Mon Sep 17 00:00:00 2001 From: Pankaj Agrawal Date: Tue, 6 Jul 2021 13:57:56 +0200 Subject: [PATCH 2/2] chore: spotbugs check on pr --- .github/workflows/spotbugs.yml | 34 ++++++++++++++++++++++++++++++++++ pom.xml | 1 - 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/spotbugs.yml diff --git a/.github/workflows/spotbugs.yml b/.github/workflows/spotbugs.yml new file mode 100644 index 000000000..da0939fbb --- /dev/null +++ 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 \ No newline at end of file diff --git a/pom.xml b/pom.xml index 3bba8f49d..497756b65 100644 --- a/pom.xml +++ b/pom.xml @@ -419,7 +419,6 @@ true - true