Merge pull request #368 from apache/add-spotbugs

Ad SpotBugs and FindSecBugs

.github/workflows/build.yml

@@ -44,7 +44,7 @@ jobs:
           cache: maven
 
       - name: Build with Maven
-        run: ./mvnw verify -V -B --no-transfer-progress
+        run: ./mvnw verify -Pci -V -B --no-transfer-progress
         env:
           GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GE_ACCESS_TOKEN }}
 

.github/workflows/cron.yml

@@ -42,6 +42,6 @@ jobs:
           cache: maven
 
       - name: Build with Maven
-        run: ./mvnw verify -Pci -V -B --no-transfer-progress
+        run: ./mvnw verify dependency-check:check -Pci -V -B --no-transfer-progress
         env:
           GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GE_ACCESS_TOKEN }}

pom.xml

@@ -619,6 +619,30 @@
             <rulesets>
               <ruleset>${session.executionRootDirectory}/src/pmd/ruleset.xml</ruleset>
             </rulesets>
+            <excludeRoots>
+              <excludeRoot>${project.build.directory}/generated-sources/</excludeRoot>
+            </excludeRoots>
+          </configuration>
+        </plugin>
+        <plugin>
+          <groupId>com.github.spotbugs</groupId>
+          <artifactId>spotbugs-maven-plugin</artifactId>
+          <version>4.7.3.5</version>
+          <configuration>
+            <plugins>
+              <plugin>
+                <groupId>com.h3xstream.findsecbugs</groupId>
+                <artifactId>findsecbugs-plugin</artifactId>
+                <version>1.12.0</version>
+              </plugin>
+            </plugins>
+            <effort>Max</effort>
+            <threshold>Medium</threshold>
+            <failOnError>true</failOnError>
+            <omitVisitors>CrlfLogInjectionDetector</omitVisitors>
+            <xmlOutput>true</xmlOutput>
+            <fork>false</fork>
+            <excludeFilterFile>${session.executionRootDirectory}/src/spotbugs/excludes.xml</excludeFilterFile>
           </configuration>
         </plugin>
       </plugins>
@@ -631,19 +655,21 @@
       <build>
         <plugins>
           <plugin>
-            <groupId>org.owasp</groupId>
-            <artifactId>dependency-check-maven</artifactId>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-pmd-plugin</artifactId>
             <executions>
               <execution>
+                <id>scan</id>
+                <phase>verify</phase>
                 <goals>
                   <goal>check</goal>
                 </goals>
               </execution>
             </executions>
           </plugin>
           <plugin>
-            <groupId>org.apache.maven.plugins</groupId>
-            <artifactId>maven-pmd-plugin</artifactId>
+            <groupId>com.github.spotbugs</groupId>
+            <artifactId>spotbugs-maven-plugin</artifactId>
             <executions>
               <execution>
                 <id>scan</id>

scim-core/src/main/java/org/apache/directory/scim/core/json/ObjectMapperFactory.java

@@ -79,6 +79,8 @@ public static ObjectMapper createObjectMapper(SchemaRegistry schemaRegistry) {
 
   static class ScimResourceModule extends SimpleModule {
 
+    private static final long serialVersionUID = 6849840952304999849L;
+
     private final SchemaRegistry schemaRegistry;
 
     public ScimResourceModule(SchemaRegistry schemaRegistry) {

scim-core/src/main/java/org/apache/directory/scim/core/json/ScimResourceDeserializer.java

@@ -32,6 +32,8 @@
 import java.util.stream.StreamSupport;
 
 public class ScimResourceDeserializer extends StdDeserializer<ScimResource> {
+
+  private static final long serialVersionUID = -2125441391108866034L;
   private final SchemaRegistry schemaRegistry;
 
   public ScimResourceDeserializer(SchemaRegistry schemaRegistry) {

scim-core/src/main/java/org/apache/directory/scim/core/repository/PatchHandlerImpl.java

@@ -221,7 +221,7 @@ default <T extends ScimResource> void applyExtensionValue(final T source, Map<St
     <T extends ScimResource> void applyMultiValue(final T source, Map<String, Object> sourceAsMap, Schema schema, Attribute attribute, ValuePathExpression valuePathExpression, Object value);
   }
 
-  private class AddOperationHandler implements PatchOperationHandler {
+  private static class AddOperationHandler implements PatchOperationHandler {
 
     @Override
     public <T extends ScimResource> void applyExtensionValue(T source, Map<String, Object> sourceAsMap, Schema schema, Attribute attribute, ValuePathExpression valuePathExpression, String urn, Object value) {
@@ -314,7 +314,7 @@ public <T extends ScimResource> void applyMultiValue(T source, Map<String, Objec
     }
   }
 
-  private class ReplaceOperationHandler implements PatchOperationHandler {
+  private static class ReplaceOperationHandler implements PatchOperationHandler {
 
     @Override
     public void applySingleValue(Map<String, Object> sourceAsMap, Attribute attribute, AttributeReference attributeReference, Object value) {
@@ -366,7 +366,7 @@ public <T extends ScimResource> void applyMultiValue(T source, Map<String, Objec
     }
   }
 
-  private class RemoveOperationHandler implements PatchOperationHandler {
+  private static class RemoveOperationHandler implements PatchOperationHandler {
 
     @Override
     public void applySingleValue(Map<String, Object> sourceAsMap, Attribute attribute, AttributeReference attributeReference, Object value) {

scim-core/src/main/java/org/apache/directory/scim/core/repository/PrioritySortingComparator.java

@@ -19,14 +19,16 @@
 
 package org.apache.directory.scim.core.repository;
 
+import java.io.Serializable;
 import java.util.Comparator;
 import java.util.Set;
 
 import org.apache.directory.scim.spec.resources.TypedAttribute;
 
-class PrioritySortingComparator implements Comparator<Object> {
+class PrioritySortingComparator implements Comparator<Object>, Serializable {
 
-  private Set<Object> priorities;
+  private static final long serialVersionUID = -8759531575215428525L;
+  private final Set<Object> priorities;
 
   public PrioritySortingComparator(Set<Object> priorities) {
     this.priorities = priorities;

scim-core/src/main/java/org/apache/directory/scim/core/repository/UpdateRequest.java

@@ -293,7 +293,7 @@ List<PatchOperation> convertToPatchOperations(JsonNode node) throws IllegalArgum
   private List<PatchOperation> convertNodeToPatchOperations(String operationNode, String diffPath, JsonNode valueNode) throws IllegalArgumentException, IllegalAccessException, JsonProcessingException {
     log.debug("convertNodeToPatchOperations: {} , {}", operationNode, diffPath);
     List<PatchOperation> operations = new ArrayList<>();
-    PatchOperation.Type patchOpType = PatchOperation.Type.valueOf(operationNode.toUpperCase());
+    PatchOperation.Type patchOpType = PatchOperation.Type.valueOf(operationNode.toUpperCase(Locale.ROOT));
 
     if (diffPath != null && diffPath.length() >= 1) {
       ParseData parseData = new ParseData(diffPath);

scim-core/src/main/java/org/apache/directory/scim/core/schema/SchemaRegistry.java

@@ -19,6 +19,7 @@
 
 package org.apache.directory.scim.core.schema;
 
+import java.io.Serializable;
 import java.util.*;
 
 import org.apache.directory.scim.spec.annotation.ScimExtensionType;
@@ -33,8 +34,9 @@
 import org.apache.directory.scim.spec.schema.Schemas;
 
 @Slf4j
-public class SchemaRegistry {
+public class SchemaRegistry implements Serializable {
 
+  private static final long serialVersionUID = 2644269305703474835L;
   private final Map<String, Schema> schemaMap = new HashMap<>();
 
   private final Map<String, Class<? extends ScimResource>> schemaUrnToScimResourceClass = new HashMap<>();

scim-server-examples/scim-server-jersey/src/main/java/org/apache/directory/scim/example/jersey/JerseyApplication.java

@@ -76,7 +76,7 @@ public static void main(String[] args) {
         .thenAccept(instance -> instance.stopOnShutdown(stopResult -> container.close()));
       URI uri = UriBuilder.fromUri("http://localhost/").port(8080).build();
 
-      System.out.printf("Application started: %s\nStop the application using CTRL+C%n", uri.toString());
+      System.out.printf("Application started: %s%nStop the application using CTRL+C%n", uri.toString());
 
       // block and wait shut down signal, like CTRL+C
       Thread.currentThread().join();

scim-server/src/main/java/org/apache/directory/scim/server/rest/AttributeUtil.java

@@ -19,8 +19,10 @@
 
 package org.apache.directory.scim.server.rest;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.directory.scim.core.json.ObjectMapperFactory;
 import org.apache.directory.scim.server.exception.AttributeDoesNotExistException;
 import org.apache.directory.scim.server.exception.AttributeException;
 import org.apache.directory.scim.spec.filter.attribute.AttributeReference;
@@ -35,11 +37,6 @@
 import org.apache.directory.scim.spec.schema.Schema.Attribute.Type;
 import org.apache.directory.scim.core.schema.SchemaRegistry;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -53,8 +50,11 @@ class AttributeUtil {
 
   SchemaRegistry schemaRegistry;
 
+  private final ObjectMapper objectMapper;
+
   AttributeUtil(SchemaRegistry schemaRegistry) {
     this.schemaRegistry = schemaRegistry;
+    this.objectMapper = ObjectMapperFactory.createObjectMapper(schemaRegistry);
   }
 
   public <T extends ScimResource> T keepAlwaysAttributesForDisplay(T resource) throws AttributeException {
@@ -165,20 +165,8 @@ public <T extends ScimResource> T setExcludedAttributesForDisplay(T resource, Se
   }
 
   @SuppressWarnings("unchecked")
-  private <T extends ScimResource> T cloneScimResource(T original) throws AttributeException {
-    try {
-    ByteArrayOutputStream boas = new ByteArrayOutputStream();
-    ObjectOutputStream oos = new ObjectOutputStream(boas);
-    oos.writeObject(original);
-
-    ByteArrayInputStream bais = new ByteArrayInputStream(boas.toByteArray());
-    ObjectInputStream ois = new ObjectInputStream(bais);
-    return (T) ois.readObject();
-    } catch (ClassNotFoundException e) {
-      throw new IllegalStateException(e);
-    } catch (IOException e) {
-      throw new AttributeException(e);
-    }
+  private <T extends ScimResource> T cloneScimResource(T original) {
+      return (T) this.objectMapper.convertValue(original, original.getClass());
   }
 
   private void removeAttributesOfType(Object object, AttributeContainer attributeContainer, Returned returned) throws AttributeException {

scim-server/src/main/java/org/apache/directory/scim/server/rest/ScimResourceHelper.java

@@ -72,18 +72,6 @@ private ScimResourceHelper() {
     // Make this a utility class
   }
 
-  /**
-   * Provides a set of JAX-RS annotated classes for the basic SCIM protocol
-   * functionality.
-   * 
-   * @return the JAX-RS annotated classes.
-   * @deprecated use {@link ScimResourceHelper#scimpleFeatureAndResourceClasses}
-   */
-  @Deprecated
-  public static Set<Class<?>> getScimClassesToLoad() {
-    return SCIMPLE_CLASSES;
-  }
-
   public static Set<Class<?>> scimpleFeatureAndResourceClasses() {
     Set<Class<?>> classes = new HashSet<>(RESOURCE_CLASSES);
     classes.add(ScimpleFeature.class);

scim-server/src/test/java/org/apache/directory/scim/server/rest/AttributeUtilTest.java

@@ -64,19 +64,10 @@ public class AttributeUtilTest {
 
   @BeforeEach
   public void setup() {
-    schemaRegistry = Mockito.mock(SchemaRegistry.class);
+    schemaRegistry = new SchemaRegistry();
+    schemaRegistry.addSchema(ScimUser.class, List.of(EnterpriseExtension.class, ExampleObjectExtension.class));
 
     attributeUtil = new AttributeUtil(schemaRegistry);
-    Schema scimUserSchema = Schemas.schemaFor(ScimUser.class);
-    Schema scimEnterpriseUserSchema = Schemas.schemaForExtension(EnterpriseExtension.class);
-    Schema scimExampleSchema = Schemas.schemaForExtension(ExampleObjectExtension.class);
-
-    Mockito.when(schemaRegistry.getBaseSchemaOfResourceType(ScimUser.RESOURCE_NAME)).thenReturn(scimUserSchema);
-    Mockito.when(schemaRegistry.getSchema(ScimUser.SCHEMA_URI)).thenReturn(scimUserSchema);
-    Mockito.when(schemaRegistry.getSchema(EnterpriseExtension.URN)).thenReturn(scimEnterpriseUserSchema);
-    Mockito.when(schemaRegistry.getSchema(ExampleObjectExtension.URN)).thenReturn(scimExampleSchema);
-    Mockito.when(schemaRegistry.getAllSchemas()).thenReturn(Arrays.asList(scimUserSchema, scimEnterpriseUserSchema, scimExampleSchema));
-    Mockito.when(schemaRegistry.getAllSchemaUrns()).thenReturn(new HashSet<>(Arrays.asList(ScimUser.SCHEMA_URI, EnterpriseExtension.URN, ExampleObjectExtension.URN)));
   }
 
   @Test

scim-spec/scim-spec-protocol/src/main/java/org/apache/directory/scim/protocol/data/BulkOperation.java

@@ -32,10 +32,14 @@
 import lombok.AllArgsConstructor;
 import lombok.Data;
 
+import java.io.Serializable;
+
 @Data
 @XmlType(propOrder = {"method","path","bulkId","data"})
 @XmlAccessorType(XmlAccessType.NONE)
-public class BulkOperation {
+public class BulkOperation implements Serializable {
+
+  private static final long serialVersionUID = 6528874816710788132L;
 
   public enum Method {
     @XmlEnumValue("POST") POST,
@@ -47,8 +51,10 @@ public enum Method {
   @Data
   @AllArgsConstructor
   @XmlAccessorType(XmlAccessType.NONE)
-  public static class StatusWrapper {
-
+  public static class StatusWrapper implements Serializable {
+
+    private static final long serialVersionUID = 1544738718748608248L;
+
     public static StatusWrapper wrap(Status code) {
       return new StatusWrapper(code);
     }

scim-spec/scim-spec-protocol/src/main/java/org/apache/directory/scim/protocol/data/BulkRequest.java

@@ -37,7 +37,9 @@
 public class BulkRequest extends BaseResource<BulkRequest> {
 
   public static final String SCHEMA_URI = "urn:ietf:params:scim:api:messages:2.0:BulkRequest";
-
+
+  private static final long serialVersionUID = -296570866318702047L;
+
   @XmlElement
   Integer failOnErrors;
 

scim-spec/scim-spec-protocol/src/main/java/org/apache/directory/scim/protocol/data/ErrorResponse.java

@@ -40,7 +40,7 @@ public class ErrorResponse extends BaseResource<ErrorResponse> {
 
   private static final long serialVersionUID = 9045421198080348116L;
 
-  public static String SCHEMA_URI = "urn:ietf:params:scim:api:messages:2.0:Error";
+  public static final String SCHEMA_URI = "urn:ietf:params:scim:api:messages:2.0:Error";
 
   @XmlElement(nillable = true)
   private String detail;

scim-spec/scim-spec-protocol/src/main/java/org/apache/directory/scim/protocol/data/StatusAdapter.java

@@ -31,7 +31,7 @@ public Status unmarshal(String v) throws Exception {
     }
 
     for (Status status : Status.values()) {
-      if (status.getStatusCode() == Integer.valueOf(v)) {
+      if (status.getStatusCode() == Integer.parseInt(v)) {
         return status;
       }
     }

scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/AttributeComparisonExpression.java

@@ -32,6 +32,7 @@
 
 @Value
 public class AttributeComparisonExpression implements FilterExpression, ValueFilterExpression {
+  private static final long serialVersionUID = -2865840428089850575L;
   AttributeReference attributePath;
   CompareOperator operation;
   Object compareValue;

scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/AttributePresentExpression.java

@@ -24,6 +24,7 @@
 
 @Value
 public class AttributePresentExpression implements FilterExpression, ValueFilterExpression {
+  private static final long serialVersionUID = -4491412651236977273L;
   AttributeReference attributePath;
 
   @Override

scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/ExpressionBuildingListener.java

@@ -21,6 +21,7 @@
 
 import java.util.ArrayDeque;
 import java.util.Deque;
+import java.util.Locale;
 
 import org.apache.commons.lang3.StringEscapeUtils;
 import org.slf4j.Logger;
@@ -92,7 +93,7 @@ public void exitFilterAttributeCompareExpression(FilterAttributeCompareExpressio
     AttributeComparisonExpression attributeComparisonExpression;
     String attributePathText = ctx.attributePath.getText();
     AttributeReference attributePath = new AttributeReference(attributePathText);
-    CompareOperator compareOperator = CompareOperator.valueOf(ctx.op.getText().toUpperCase());
+    CompareOperator compareOperator = CompareOperator.valueOf(ctx.op.getText().toUpperCase(Locale.ROOT));
     String compareValueText = ctx.compareValue.getText();
     Object compareValue = parseJsonType(compareValueText);
     attributeComparisonExpression = new AttributeComparisonExpression(attributePath, compareOperator, compareValue);
@@ -102,7 +103,7 @@ public void exitFilterAttributeCompareExpression(FilterAttributeCompareExpressio
 
   @Override
   public void exitFilterLogicExpression(FilterLogicExpressionContext ctx) {
-    String op = ctx.op.getText().toUpperCase();
+    String op = ctx.op.getText().toUpperCase(Locale.ROOT);
     LogicalOperator logicalOperator = LogicalOperator.valueOf(op);
     FilterExpression right = expressionStack.pop();
     FilterExpression left = expressionStack.pop();
@@ -113,7 +114,7 @@ public void exitFilterLogicExpression(FilterLogicExpressionContext ctx) {
 
   @Override
   public void exitAttributeLogicExpression(AttributeLogicExpressionContext ctx) {
-    String op = ctx.op.getText().toUpperCase();
+    String op = ctx.op.getText().toUpperCase(Locale.ROOT);
     LogicalOperator logicalOperator = LogicalOperator.valueOf(op);
     FilterExpression right = expressionStack.pop();
     FilterExpression left = expressionStack.pop();
@@ -134,7 +135,7 @@ public void exitAttributeGroupExpression(AttributeGroupExpressionContext ctx) {
   @Override
   public void exitAttributeCompareExpression(AttributeCompareExpressionContext ctx) {
     String attributeName = ctx.attributeName.getText();
-    CompareOperator compareOperator = CompareOperator.valueOf(ctx.op.getText().toUpperCase());
+    CompareOperator compareOperator = CompareOperator.valueOf(ctx.op.getText().toUpperCase(Locale.ROOT));
     Object value = parseJsonType(ctx.compareValue.getText());
     AttributeReference attributeReference = new AttributeReference(attributeName);
     AttributeComparisonExpression expression = new AttributeComparisonExpression(attributeReference, compareOperator, value);