Skip to content
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

Static analysis spotbugs #458

Merged
merged 2 commits into from
Jul 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions .github/workflows/spotbugs.yml
Original file line number Diff line number Diff line change
@@ -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'
msailes marked this conversation as resolved.
Show resolved Hide resolved
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
53 changes: 52 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package software.amazon.lambda.powertools.metrics;

import java.util.Arrays;
import java.util.Optional;
import java.util.function.Consumer;

Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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/>
*
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a neat finding and was actually a bug

Copy link
Contributor

Choose a reason for hiding this comment

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

👍


RuntimeConfiguration updatedConfig = new RuntimeConfiguration.Builder()
.withFunctionRegistry(functionRegistryWithExtendedFunctions)
.build();

jmesPath = new JacksonRuntime(updatedConfig, getObjectMapper());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
*/
Expand Down Expand Up @@ -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)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This too

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}
}

Expand Down