Merge pull request #1054 from folio-org/release-33.2.10

Release 33.2.10

NEWS.md

@@ -1,3 +1,15 @@
+## 33.2.10 2022-05-31
+
+Fixes:
+
+* [RMB-913](https://issues.folio.org/browse/RMB-913) Fail if PostgreSQL version is too old
+* [RMB-914](https://issues.folio.org/browse/RMB-914) Update raml-util to avoid localhost fetch RMB-914
+* [RMB-915](https://issues.folio.org/browse/RMB-915) public PostgresClient#getTenantId()
+* [RMB-917](https://issues.folio.org/browse/RMB-917) id == "", foreignKeyId == "" should never match
+* [RMB-918](https://issues.folio.org/browse/RMB-918) Fix Jakarta Expression Language validation (CVE-2021-28170)
+* [RMB-919](https://issues.folio.org/browse/RMB-919) Delete by CQL rejects missing or empty CQL
+* [RMB-920](https://issues.folio.org/browse/RMB-920) Upgrade dependencies: Vert.x 4.3.1, ...
+
 ## 33.2.9 2022-04-22
 
 Fix:

cql2pgjson-cli/pom.xml

@@ -6,7 +6,7 @@
   <parent>
     <groupId>org.folio</groupId>
     <artifactId>raml-module-builder</artifactId>
-    <version>33.2.10-SNAPSHOT</version>
+    <version>33.2.11-SNAPSHOT</version>
   </parent>
 
   <properties>

cql2pgjson/pom.xml

@@ -7,7 +7,7 @@
   <parent>
     <groupId>org.folio</groupId>
     <artifactId>raml-module-builder</artifactId>
-    <version>33.2.10-SNAPSHOT</version>
+    <version>33.2.11-SNAPSHOT</version>
   </parent>
 
   <properties>
@@ -89,7 +89,13 @@
     <dependency>
       <groupId>org.postgresql</groupId>
       <artifactId>postgresql</artifactId>
-      <version>42.3.2</version>
+      <version>42.3.6</version>
+      <scope>test</scope>
+    </dependency>
+
+    <dependency>
+      <groupId>org.folio.okapi</groupId>
+      <artifactId>okapi-testing</artifactId>
       <scope>test</scope>
     </dependency>
 

cql2pgjson/src/main/java/org/folio/cql2pgjson/CQL2PgJSON.java

@@ -608,9 +608,6 @@ private String pgId(CQLTermNode node, String columnName) throws QueryValidationE
           + "UUID " + columnName + " only supports '=', '==', and '<>' (possibly with right truncation)");
     }
 
-    if (StringUtils.isEmpty(term)) {
-      term = "*";
-    }
     if ("*".equals(term) && "id".equals(columnName)) {
       return equals ? "true" : "false";  // no need to check
       // since id is a mandatory field, so

cql2pgjson/src/test/java/org/folio/cql2pgjson/CQL2PgJSONTest.java

@@ -800,12 +800,11 @@ public void toSqlException() throws QueryValidationException {
 
   @Test
   @Parameters({
-    "id=*,                                       true",
-    "id=\"\",                                    true",
+    "id=*,    true",
+    "id=\"\", false /* id == invalid UUID */",  // exact (field) match, empty string never matches UUID
     "groupId=*,                                  (groupId BETWEEN '00000000-0000-0000-0000-000000000000' "
                                                            + "AND 'ffffffff-ffff-ffff-ffff-ffffffffffff')",
-    "groupId=\"\",                               (groupId BETWEEN '00000000-0000-0000-0000-000000000000' "
-                                                           + "AND 'ffffffff-ffff-ffff-ffff-ffffffffffff')",
+    "groupId=\"\",                               false /* groupId == invalid UUID */",
     "id=\"11111111-1111-1111-1111-111111111111\",              id='11111111-1111-1111-1111-111111111111'",
     "id=\"2*\",                                       (id BETWEEN '20000000-0000-0000-0000-000000000000' "
                                                            + "AND '2fffffff-ffff-ffff-ffff-ffffffffffff')",
@@ -850,8 +849,9 @@ public void idColumnSort(String cql, String expectedSql) throws CQL2PgJSONExcept
     "id=11111111-1111-1111-1111-11111111111    #",
     "id=11111111-1111-1111-1111-1111111111111  #",
     "id=11111111-1111-1111-1111-111111111111-1 #",
-    "id=\"\"                                   # Jo Jane; Ka Keller; Lea Long",
-    "id<>\"\"                                  #",
+    "id=\"\"                                   #",    // exact match, empty string never matches UUID
+    "id==\"\"                                  #",
+    "id<>\"\"                                  # Jo Jane; Ka Keller; Lea Long",
     "id=1*                                     # Jo Jane",
     "id=1**                                    # Jo Jane",
     "id=1***                                   # Jo Jane",

dbschema/pom.xml

@@ -7,7 +7,7 @@
   <parent>
     <groupId>org.folio</groupId>
     <artifactId>raml-module-builder</artifactId>
-    <version>33.2.10-SNAPSHOT</version>
+    <version>33.2.11-SNAPSHOT</version>
   </parent>
 
   <properties>

domain-models-api-aspects/pom.xml

@@ -3,7 +3,7 @@
   <parent>
     <groupId>org.folio</groupId>
     <artifactId>raml-module-builder</artifactId>
-    <version>33.2.10-SNAPSHOT</version>
+    <version>33.2.11-SNAPSHOT</version>
   </parent>
   <artifactId>domain-models-api-aspects</artifactId>
 
@@ -40,7 +40,7 @@
     </dependency>
     <dependency>
       <groupId>org.glassfish</groupId>
-      <artifactId>javax.el</artifactId>
+      <artifactId>jakarta.el</artifactId>
     </dependency>
     <dependency>
       <groupId>org.apache.commons</groupId>
@@ -57,7 +57,7 @@
     <dependency>
       <groupId>org.javassist</groupId>
       <artifactId>javassist</artifactId>
-      <version>3.28.0-GA</version>
+      <version>3.29.0-GA</version>
     </dependency>
 
     <!-- test dependencies -->

domain-models-api-interfaces/pom.xml

@@ -3,7 +3,7 @@
   <parent>
     <groupId>org.folio</groupId>
     <artifactId>raml-module-builder</artifactId>
-    <version>33.2.10-SNAPSHOT</version>
+    <version>33.2.11-SNAPSHOT</version>
   </parent>
   <artifactId>domain-models-api-interfaces</artifactId>
   <properties>

domain-models-maven-plugin/pom.xml

@@ -3,7 +3,7 @@
   <parent>
     <groupId>org.folio</groupId>
     <artifactId>raml-module-builder</artifactId>
-    <version>33.2.10-SNAPSHOT</version>
+    <version>33.2.11-SNAPSHOT</version>
   </parent>
   <artifactId>domain-models-maven-plugin</artifactId>
   <description>Domain Model Generator that reads RAML files and writes Java files</description>

domain-models-runtime-it/pom.xml

@@ -6,7 +6,7 @@
   <parent>
     <groupId>org.folio</groupId>
     <artifactId>raml-module-builder</artifactId>
-    <version>33.2.10-SNAPSHOT</version>
+    <version>33.2.11-SNAPSHOT</version>
   </parent>
 
   <repositories>

domain-models-runtime/pom.xml

@@ -6,7 +6,7 @@
   <parent>
     <groupId>org.folio</groupId>
     <artifactId>raml-module-builder</artifactId>
-    <version>33.2.10-SNAPSHOT</version>
+    <version>33.2.11-SNAPSHOT</version>
   </parent>
 
   <repositories>

domain-models-runtime/src/main/java/org/folio/rest/impl/TenantAPI.java

@@ -49,6 +49,25 @@ PostgresClient postgresClient(Context context) {
     return PostgresClient.getInstance(context.owner());
   }
 
+  Future<Void> requirePostgres(Context context, String minNum, String minVersion) {
+    return
+        postgresClient(context)
+        .select("SELECT current_setting('server_version_num') AS num, current_setting('server_version') AS version")
+        .map(rowSet -> {
+          String num = rowSet.iterator().next().getString("num");
+          String version = rowSet.iterator().next().getString("version");
+          if (minNum.compareTo(num) > 0) {
+            throw new UnsupportedOperationException(
+                "Expected PostgreSQL server version " + minVersion + " or later but found " + version);
+          }
+          return null;
+        });
+  }
+
+  Future<Void> requirePostgres12(Context context) {
+    return requirePostgres(context, "120000", "12.0");
+  }
+
   Future<Boolean> tenantExists(Context context, String tenantId){
     /* connect as user in postgres-conf.json file (super user) - so that all commands will be available */
     return postgresClient(context).select(
@@ -207,7 +226,8 @@ private void postTenant(boolean async, TenantAttributes tenantAttributes, Map<St
     job.setComplete(false);
 
     String location = "/_/tenant/" + id;
-    tenantExists(context, tenantId)
+    requirePostgres12(context)
+        .compose(x -> tenantExists(context, tenantId))
         .compose(exists -> sqlFile(context, tenantId, tenantAttributes, exists))
         .onFailure(cause -> {
           log.error(cause.getMessage(), cause);

domain-models-runtime/src/main/java/org/folio/rest/persist/PgUtil.java

@@ -25,6 +25,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import javax.ws.rs.core.Response;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.folio.dbschema.ObjectMapperTool;
@@ -918,17 +919,16 @@ static <T, C> Future<Response> get(PreparedCQL preparedCql, Class<T> clazz, Clas
     }
   }
 
-    /**
-    * Delete records by CQL.
-    * @param table  the table that contains the records
-    * @param cql  the CQL query for filtering the records
-    * @param okapiHeaders  http headers provided by okapi
-    * @param vertxContext  the current context
-    * @param responseDelegateClass  the ResponseDelegate class generated as defined by the RAML file,
-    *    must have these methods:  respond204(), respond400WithTextPlain(Object), respond500WithTextPlain(Object).
-    * @param asyncResultHandler  where to return the result created by the responseDelegateClass
-    */
-  @SuppressWarnings({"unchecked", "squid:S107"})     // Method has >7 parameters
+  /**
+   * Delete records by CQL.
+   * @param table  the table that contains the records
+   * @param cql  the CQL query for filtering the records, required
+   * @param okapiHeaders  http headers provided by okapi
+   * @param vertxContext  the current context
+   * @param responseDelegateClass  the ResponseDelegate class generated as defined by the RAML file,
+   *    must have these methods:  respond204(), respond400WithTextPlain(Object), respond500WithTextPlain(Object).
+   * @param asyncResultHandler  where to return the result created by the responseDelegateClass
+   */
   public static void delete(String table,
       String cql,
       Map<String, String> okapiHeaders, Context vertxContext,
@@ -937,17 +937,16 @@ public static void delete(String table,
     delete(table, cql, okapiHeaders, vertxContext, responseDelegateClass).onComplete(asyncResultHandler);
   }
 
-   /**
+  /**
    * Delete records by CQL.
-    * @param table  the table that contains the records
-    * @param cql  the CQL query for filtering the records
-    * @param okapiHeaders  http headers provided by okapi
-    * @param vertxContext  the current context
-    * @param responseDelegateClass  the ResponseDelegate class generated as defined by the RAML file,
-*    must have these methods:  respond204(), respond400WithTextPlain(Object), respond500WithTextPlain(Object).
-    * @return future where to return the result created by the responseDelegateClass
-    */
-  @SuppressWarnings({"unchecked", "squid:S107"})     // Method has >7 parameters
+   * @param table  the table that contains the records
+   * @param cql  the CQL query for filtering the records, required
+   * @param okapiHeaders  http headers provided by okapi
+   * @param vertxContext  the current context
+   * @param responseDelegateClass  the ResponseDelegate class generated as defined by the RAML file,
+   *     must have these methods:  respond204(), respond400WithTextPlain(Object), respond500WithTextPlain(Object).
+   * @return future where to return the result created by the responseDelegateClass
+   */
   public static Future<Response> delete(String table,
       String cql,
       Map<String, String> okapiHeaders, Context vertxContext,
@@ -972,6 +971,10 @@ public static Future<Response> delete(String table,
     }
 
     try {
+      if (StringUtils.isBlank(cql)) {
+        return response("query with CQL expression is required", respond400, respond500);
+      }
+
       CQL2PgJSON cql2pgJson = new CQL2PgJSON(table + "." + JSON_COLUMN);
       CQLWrapper cqlWrapper = new CQLWrapper(cql2pgJson, cql, -1, -1);
       PreparedCQL preparedCql = new PreparedCQL(table, cqlWrapper, okapiHeaders);
@@ -1003,7 +1006,7 @@ public static Future<Response> delete(String table,
     }
   }
 
-    /**
+  /**
    * Get a record by id.
    *
    * <p>All exceptions are caught and reported via the asyncResultHandler.

domain-models-runtime/src/main/java/org/folio/rest/persist/PostgresClient.java

@@ -4247,7 +4247,7 @@ static String getModuleName(final String className) {
   /**
    * @return the tenantId of this PostgresClient
    */
-  String getTenantId() {
+  public String getTenantId() {
     return tenantId;
   }
 

domain-models-runtime/src/test/java/org/folio/rest/impl/TenantAPIIT.java

@@ -3,6 +3,7 @@
 import static org.hamcrest.CoreMatchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.startsWith;
 import static org.mockito.Mockito.*;
 
 import java.io.IOException;
@@ -260,6 +261,13 @@ private void testMetadata(TestContext context) {
     assertThat(book.getMetadata(), is(nullValue()));
   }
 
+  @Test
+  public void requirePostgres(TestContext context) {
+    new TenantAPI().requirePostgres(vertx.getOrCreateContext(), "990000", "99.0")
+    .onComplete(context.asyncAssertFailure(
+        e -> assertThat(e.getMessage(), startsWith("Expected PostgreSQL server version 99.0 or later"))));
+  }
+
   @Test
   public void previousSchemaSqlExistsTrue(TestContext context) {
     TenantAPI tenantAPI = new TenantAPI();
@@ -294,6 +302,11 @@ PostgresClient postgresClient(Context context) {
         return postgresClient;
       }
 
+      @Override
+      Future<Void> requirePostgres12(Context context) {
+        return Future.succeededFuture();
+      }
+
       @Override
       Future<Boolean> tenantExists(Context context, String tenantId) {
         return Future.succeededFuture(false);
@@ -318,6 +331,11 @@ PostgresClient postgresClient(Context context) {
         return postgresClient;
       }
 
+      @Override
+      Future<Void> requirePostgres12(Context context) {
+        return Future.succeededFuture();
+      }
+
       @Override
       Future<Boolean> tenantExists(Context context, String tenantId) {
         return Future.succeededFuture(false);

domain-models-runtime/src/test/java/org/folio/rest/persist/PgUtilIT.java

@@ -413,6 +413,27 @@ public void deleteByCQLNullHeaders(TestContext testContext) {
         asyncAssertSuccess(testContext, 400, "null"));
   }
 
+  @Test
+  public void deleteByCQLWithoutCql(TestContext testContext) {
+    PgUtil.delete("users", null,
+        null, vertx.getOrCreateContext(), Users.DeleteUsersByUserIdResponse.class,
+        asyncAssertSuccess(testContext, 400, "query with CQL expression is required"));
+  }
+
+  @Test
+  public void deleteByCQLWithEmptyCql(TestContext testContext) {
+    PgUtil.delete("users", "",
+        null, vertx.getOrCreateContext(), Users.DeleteUsersByUserIdResponse.class,
+        asyncAssertSuccess(testContext, 400, "query with CQL expression is required"));
+  }
+
+  @Test
+  public void deleteByCQLWithWhitespaceCql(TestContext testContext) {
+    PgUtil.delete("users", "\t\n \t\n ",
+        null, vertx.getOrCreateContext(), Users.DeleteUsersByUserIdResponse.class,
+        asyncAssertSuccess(testContext, 400, "query with CQL expression is required"));
+  }
+
   @Test
   public void deleteByCQLOK(TestContext testContext) {
     PostgresClient pg = PostgresClient.getInstance(vertx, "testtenant");
@@ -453,15 +474,13 @@ public void deleteByCQLOK(TestContext testContext) {
 
   @Test
   public void deleteByCQLSyntaxError(TestContext testContext) {
-    PostgresClient pg = PostgresClient.getInstance(vertx, "testtenant");
     PgUtil.delete("users",  "username==",
         okapiHeaders, vertx.getOrCreateContext(), Users.DeleteUsersByUserIdResponse.class,
         asyncAssertSuccess(testContext, 400, "expected index or term, got EOF"));
   }
 
   @Test
   public void deleteByCQLBadTable(TestContext testContext) {
-    PostgresClient pg = PostgresClient.getInstance(vertx, "testtenant");
     PgUtil.delete("users1",  "username==delete_test",
         okapiHeaders, vertx.getOrCreateContext(), Users.DeleteUsersByUserIdResponse.class,
         asyncAssertSuccess(testContext, 400, "relation \"testtenant_raml_module_builder.users1\" does not exist"));