aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSteve Marion <steve.marion@sonarsource.com>2024-03-12 16:11:13 +0100
committersonartech <sonartech@sonarsource.com>2024-03-16 20:02:47 +0000
commit720d6dd5c4dc08327d8b2271949a66855fb88852 (patch)
tree47787da0581f8052d69d6b29c4c7e7823e91514e
parentf4896ace30bcad83d4df1bcf4a768e6ff68facb0 (diff)
downloadsonarqube-720d6dd5c4dc08327d8b2271949a66855fb88852.tar.gz
sonarqube-720d6dd5c4dc08327d8b2271949a66855fb88852.zip
SONAR-21451 replace everit json schema validation library with json-sKema. exclude commons-collection 3 from dependencies.
-rw-r--r--build.gradle5
-rw-r--r--server/sonar-webserver-webapi/build.gradle3
-rw-r--r--server/sonar-webserver-webapi/src/it/java/org/sonar/server/setting/ws/SetActionIT.java207
-rw-r--r--server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/ResetAction.java3
-rw-r--r--server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SetAction.java5
-rw-r--r--server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java106
-rw-r--r--server/sonar-webserver-webapi/src/main/resources/json-schemas/security.json1
7 files changed, 173 insertions, 157 deletions
diff --git a/build.gradle b/build.gradle
index 512f7a40107..a4561ea8083 100644
--- a/build.gradle
+++ b/build.gradle
@@ -302,7 +302,10 @@ subprojects {
entry 'scribejava-apis'
entry 'scribejava-core'
}
- dependency 'com.github.everit-org.json-schema:org.everit.json.schema:1.14.4'
+ dependency('com.github.erosb:json-sKema:0.13.0') {
+ // this version of json-sKema does not make use of commons-collections, so we can exclude it safely
+ exclude 'commons-collections:commons-collections'
+ }
// This project is no longer maintained and was forked
// by https://github.com/java-diff-utils/java-diff-utils
// (io.github.java-diff-utils:java-diff-utils).
diff --git a/server/sonar-webserver-webapi/build.gradle b/server/sonar-webserver-webapi/build.gradle
index d53038947f3..198c26ee104 100644
--- a/server/sonar-webserver-webapi/build.gradle
+++ b/server/sonar-webserver-webapi/build.gradle
@@ -8,8 +8,7 @@ dependencies {
// please keep the list grouped by configuration and ordered by name
api 'com.google.guava:guava'
- api 'com.github.everit-org.json-schema:org.everit.json.schema'
-
+ implementation 'com.github.erosb:json-sKema'
api 'io.prometheus:simpleclient_common'
api 'io.prometheus:simpleclient_servlet'
diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/setting/ws/SetActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/setting/ws/SetActionIT.java
index 838a9866ea5..314851cca4f 100644
--- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/setting/ws/SetActionIT.java
+++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/setting/ws/SetActionIT.java
@@ -26,7 +26,7 @@ import com.tngtech.java.junit.dataprovider.UseDataProvider;
import java.net.HttpURLConnection;
import java.util.List;
import java.util.Map;
-import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
import javax.annotation.Nullable;
import org.junit.Before;
import org.junit.Rule;
@@ -437,37 +437,36 @@ public class SetActionIT {
@Test
@UseDataProvider("securityJsonProperties")
public void successfully_validate_json_schema(String securityPropertyKey) {
- String security_custom_config = "{\n" +
- " \"S3649\": {\n" +
- " \"sources\": [\n" +
- " {\n" +
- " \"methodId\": \"My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery\"\n" +
- " }\n" +
- " ],\n" +
- " \"sanitizers\": [\n" +
- " {\n" +
- " \"methodId\": \"str_replace\"," +
- " \"args\": [\n" +
- " 0\n" +
- " ]\n" +
- " }\n" +
- " ],\n" +
- " \"validators\": [\n" +
- " {\n" +
- " \"methodId\": \"is_valid\"," +
- " \"args\": [\n" +
- " 1\n" +
- " ]\n" +
- " }\n" +
- " ],\n" +
- " \"sinks\": [\n" +
- " {\n" +
- " \"methodId\": \"mysql_query\",\n" +
- " \"args\": [1]\n" +
- " }\n" +
- " ]\n" +
- " }\n" +
- "}";
+ String security_custom_config = """
+ {
+ "S3649": {
+ "sources": [
+ {
+ "methodId": "My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery"
+ }
+ ],
+ "sanitizers": [
+ {
+ "methodId": "str_replace", "args": [
+ 0
+ ]
+ }
+ ],
+ "validators": [
+ {
+ "methodId": "is_valid", "args": [
+ 1
+ ]
+ }
+ ],
+ "sinks": [
+ {
+ "methodId": "mysql_query",
+ "args": [1]
+ }
+ ]
+ }
+ }""";
definitions.addComponent(PropertyDefinition
.builder(securityPropertyKey)
.name("foo")
@@ -485,21 +484,22 @@ public class SetActionIT {
@Test
@UseDataProvider("securityJsonProperties")
public void fail_json_schema_validation_when_property_has_incorrect_type(String securityPropertyKey) {
- String security_custom_config = "{\n" +
- " \"S3649\": {\n" +
- " \"sources\": [\n" +
- " {\n" +
- " \"methodId\": \"My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery\"\n" +
- " }\n" +
- " ],\n" +
- " \"sinks\": [\n" +
- " {\n" +
- " \"methodId\": 12345,\n" +
- " \"args\": [1]\n" +
- " }\n" +
- " ]\n" +
- " }\n" +
- "}";
+ String security_custom_config = """
+ {
+ "S3649": {
+ "sources": [
+ {
+ "methodId": "My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery"
+ }
+ ],
+ "sinks": [
+ {
+ "methodId": 12345,
+ "args": [1]
+ }
+ ]
+ }
+ }""";
definitions.addComponent(PropertyDefinition
.builder(securityPropertyKey)
.name("foo")
@@ -511,26 +511,27 @@ public class SetActionIT {
assertThatThrownBy(() -> callForGlobalSetting(securityPropertyKey, security_custom_config))
.isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("S3649/sinks/0/methodId: expected type: String, found: Integer");
+ .hasMessageContaining("expected type: string, actual: integer at line 10, character 21, pointer: #/S3649/sinks/0/methodId");
}
@Test
@UseDataProvider("securityJsonProperties")
public void fail_json_schema_validation_when_sanitizers_have_no_args(String securityPropertyKey) {
- String security_custom_config = "{\n" +
- " \"S3649\": {\n" +
- " \"sources\": [\n" +
- " {\n" +
- " \"methodId\": \"My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery\"\n" +
- " }\n" +
- " ],\n" +
- " \"sanitizers\": [\n" +
- " {\n" +
- " \"methodId\": \"SomeSanitizer\"\n" +
- " }\n" +
- " ]\n" +
- " }\n" +
- "}";
+ String security_custom_config = """
+ {
+ "S3649": {
+ "sources": [
+ {
+ "methodId": "My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery"
+ }
+ ],
+ "sanitizers": [
+ {
+ "methodId": "SomeSanitizer"
+ }
+ ]
+ }
+ }""";
definitions.addComponent(PropertyDefinition
.builder(securityPropertyKey)
.name("foo")
@@ -542,27 +543,28 @@ public class SetActionIT {
assertThatThrownBy(() -> callForGlobalSetting(securityPropertyKey, security_custom_config))
.isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("Provided JSON is invalid [#/S3649/sanitizers/0: #: only 1 subschema matches out of 2]");
+ .hasMessageContaining("required properties are missing: args at line 9, character 8, pointer: #/S3649/sanitizers/0");
}
@Test
@UseDataProvider("securityJsonProperties")
public void fail_json_schema_validation_when_validators_have_empty_args_array(String securityPropertyKey) {
- String security_custom_config = "{\n" +
- " \"S3649\": {\n" +
- " \"sources\": [\n" +
- " {\n" +
- " \"methodId\": \"My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery\"\n" +
- " }\n" +
- " ],\n" +
- " \"validators\": [\n" +
- " {\n" +
- " \"methodId\": \"SomeValidator\",\n" +
- " \"args\": []\n" +
- " }\n" +
- " ]\n" +
- " }\n" +
- "}";
+ String security_custom_config = """
+ {
+ "S3649": {
+ "sources": [
+ {
+ "methodId": "My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery"
+ }
+ ],
+ "validators": [
+ {
+ "methodId": "SomeValidator",
+ "args": []
+ }
+ ]
+ }
+ }""";
definitions.addComponent(PropertyDefinition
.builder(securityPropertyKey)
.name("foo")
@@ -574,27 +576,28 @@ public class SetActionIT {
assertThatThrownBy(() -> callForGlobalSetting(securityPropertyKey, security_custom_config))
.isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("Provided JSON is invalid [#/S3649/validators/0: #: only 1 subschema matches out of 2]");
+ .hasMessageContaining("expected minimum items: 1, found only 0 at line 11, character 18, pointer: #/S3649/validators/0/args");
}
@Test
@UseDataProvider("securityJsonProperties")
public void fail_json_schema_validation_when_property_has_unknown_attribute(String securityPropertyKey) {
- String security_custom_config = "{\n" +
- " \"S3649\": {\n" +
- " \"sources\": [\n" +
- " {\n" +
- " \"methodId\": \"My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery\"\n" +
- " }\n" +
- " ],\n" +
- " \"unknown\": [\n" +
- " {\n" +
- " \"methodId\": 12345,\n" +
- " \"args\": [1]\n" +
- " }\n" +
- " ]\n" +
- " }\n" +
- "}";
+ String security_custom_config = """
+ {
+ "S3649": {
+ "sources": [
+ {
+ "methodId": "My\\\\Namespace\\\\ClassName\\\\ServerRequest::getQuery"
+ }
+ ],
+ "unknown": [
+ {
+ "methodId": 12345,
+ "args": [1]
+ }
+ ]
+ }
+ }""";
definitions.addComponent(PropertyDefinition
.builder(securityPropertyKey)
.name("foo")
@@ -606,7 +609,7 @@ public class SetActionIT {
assertThatThrownBy(() -> callForGlobalSetting(securityPropertyKey, security_custom_config))
.isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("extraneous key [unknown] is not permitted");
+ .hasMessageContaining("false schema always fails at line 8, character 16, pointer: #/S3649/unknown");
}
@Test
@@ -1101,11 +1104,11 @@ public class SetActionIT {
@Test
public void fail_when_component_not_found() {
- assertThatThrownBy(() -> ws.newRequest()
+ TestRequest testRequest = ws.newRequest()
.setParam("key", "foo")
.setParam("value", "2")
- .setParam("component", "unknown")
- .execute())
+ .setParam("component", "unknown");
+ assertThatThrownBy(testRequest::execute)
.isInstanceOf(NotFoundException.class)
.hasMessage("Component key 'unknown' not found");
}
@@ -1116,11 +1119,11 @@ public class SetActionIT {
logInAsProjectAdministrator(project);
String settingKey = ProcessProperties.Property.JDBC_URL.getKey();
- assertThatThrownBy(() -> ws.newRequest()
+ TestRequest testRequest = ws.newRequest()
.setParam("key", settingKey)
.setParam("value", "any value")
- .setParam("component", project.getKey())
- .execute())
+ .setParam("component", project.getKey());
+ assertThatThrownBy(testRequest::execute)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(format("Setting '%s' can only be used in sonar.properties", settingKey));
}
@@ -1257,6 +1260,6 @@ public class SetActionIT {
}
private ProjectData randomPublicOrPrivateProject() {
- return new Random().nextBoolean() ? db.components().insertPrivateProject() : db.components().insertPublicProject();
+ return ThreadLocalRandom.current().nextBoolean() ? db.components().insertPrivateProject() : db.components().insertPublicProject();
}
}
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/ResetAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/ResetAction.java
index 0b90b57d1e7..ef98aaedd63 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/ResetAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/ResetAction.java
@@ -100,7 +100,8 @@ public class ResetAction implements SettingsWsAction {
resetRequest.getKeys().forEach(key -> {
SettingsWsSupport.validateKey(key);
SettingData data = new SettingData(key, emptyList(), entity.orElse(null));
- List.of(validations.scope(), validations.qualifier()).forEach(validation -> validation.accept(data));
+ validations.validateScope(data);
+ validations.validateQualifier(data);
});
List<String> keys = getKeys(resetRequest);
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SetAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SetAction.java
index c3ac6e87296..5a87916e21d 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SetAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SetAction.java
@@ -210,8 +210,9 @@ public class SetAction implements SettingsWsAction {
checkValueIsSet(request);
String settingKey = request.getKey();
SettingData settingData = new SettingData(settingKey, valuesFromRequest(request), entity.orElse(null));
- List.of(validations.scope(), validations.qualifier(), validations.valueType())
- .forEach(validation -> validation.accept(settingData));
+ validations.validateScope(settingData);
+ validations.validateQualifier(settingData);
+ validations.validateValueType(settingData);
}
private static void validatePropertySet(SetRequest request, @Nullable PropertyDefinition definition) {
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java
index d2eca73ccc5..260d0b246b2 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java
@@ -19,22 +19,23 @@
*/
package org.sonar.server.setting.ws;
+import com.github.erosb.jsonsKema.JsonParseException;
+import com.github.erosb.jsonsKema.JsonParser;
+import com.github.erosb.jsonsKema.JsonValue;
+import com.github.erosb.jsonsKema.SchemaLoader;
+import com.github.erosb.jsonsKema.ValidationFailure;
+import com.github.erosb.jsonsKema.Validator;
import com.google.gson.Gson;
import com.google.gson.JsonElement;
import java.io.IOException;
-import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.Set;
-import java.util.function.Consumer;
-import java.util.stream.Collectors;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
-import org.everit.json.schema.ValidationException;
-import org.everit.json.schema.loader.SchemaLoader;
-import org.json.JSONObject;
-import org.json.JSONTokener;
+import org.apache.commons.io.IOUtils;
import org.sonar.api.PropertyType;
import org.sonar.api.config.PropertyDefinition;
import org.sonar.api.config.PropertyDefinitions;
@@ -43,7 +44,6 @@ import org.sonar.core.i18n.I18n;
import org.sonar.db.DbClient;
import org.sonar.db.DbSession;
import org.sonar.db.entity.EntityDto;
-import org.sonar.db.metric.MetricDto;
import org.sonar.db.user.UserDto;
import org.sonar.server.exceptions.BadRequestException;
@@ -63,28 +63,30 @@ public class SettingValidations {
private final PropertyDefinitions definitions;
private final DbClient dbClient;
private final I18n i18n;
+ private final ValueTypeValidation valueTypeValidation;
public SettingValidations(PropertyDefinitions definitions, DbClient dbClient, I18n i18n) {
this.definitions = definitions;
this.dbClient = dbClient;
this.i18n = i18n;
+ this.valueTypeValidation = new ValueTypeValidation();
}
- public Consumer<SettingData> scope() {
- return data -> {
- PropertyDefinition definition = definitions.get(data.key);
- checkRequest(data.entity != null || definition == null || definition.global() || isGlobal(definition),
- "Setting '%s' cannot be global", data.key);
- };
+ public void validateScope(SettingData data) {
+ PropertyDefinition definition = definitions.get(data.key);
+ checkRequest(data.entity != null || definition == null || definition.global() || isGlobal(definition),
+ "Setting '%s' cannot be global", data.key);
}
- public Consumer<SettingData> qualifier() {
- return data -> {
- String qualifier = data.entity == null ? "" : data.entity.getQualifier();
- PropertyDefinition definition = definitions.get(data.key);
- checkRequest(checkComponentQualifier(data, definition),
- "Setting '%s' cannot be set on a %s", data.key, i18n.message(Locale.ENGLISH, "qualifier." + qualifier, null));
- };
+ public void validateQualifier(SettingData data) {
+ String qualifier = data.entity == null ? "" : data.entity.getQualifier();
+ PropertyDefinition definition = definitions.get(data.key);
+ checkRequest(checkComponentQualifier(data, definition),
+ "Setting '%s' cannot be set on a %s", data.key, i18n.message(Locale.ENGLISH, "qualifier." + qualifier, null));
+ }
+
+ public void validateValueType(SettingData data) {
+ valueTypeValidation.validateValueType(data);
}
private static boolean checkComponentQualifier(SettingData data, @Nullable PropertyDefinition definition) {
@@ -98,15 +100,11 @@ public class SettingValidations {
return definition.qualifiers().contains(entity.getQualifier());
}
- public Consumer<SettingData> valueType() {
- return new ValueTypeValidation();
- }
-
private static boolean isGlobal(PropertyDefinition definition) {
return !definition.global() && definition.qualifiers().isEmpty();
}
- static class SettingData {
+ public static class SettingData {
private final String key;
private final List<String> values;
@CheckForNull
@@ -119,10 +117,25 @@ public class SettingValidations {
}
}
- private class ValueTypeValidation implements Consumer<SettingData> {
+ private class ValueTypeValidation {
+
+ private final Validator schemaValidator;
+
+ public ValueTypeValidation() {
+ this.schemaValidator = Optional.ofNullable(this.getClass().getClassLoader().getResourceAsStream("json-schemas/security.json"))
+ .map(schemaStream -> {
+ try {
+ return IOUtils.toString(schemaStream, StandardCharsets.UTF_8);
+ } catch (IOException e) {
+ return null;
+ }
+ }).map(schemaString -> new JsonParser(schemaString).parse())
+ .map(schemaJson -> new SchemaLoader(schemaJson).load())
+ .map(Validator::forSchema)
+ .orElseThrow(() -> new IllegalStateException("Unable to create security schema validator"));
+ }
- @Override
- public void accept(SettingData data) {
+ public void validateValueType(SettingData data) {
PropertyDefinition definition = definitions.get(data.key);
if (definition == null) {
return;
@@ -144,23 +157,15 @@ public class SettingValidations {
.findAny()
.ifPresent(result -> {
throw BadRequestException.create(i18n.message(Locale.ENGLISH, "property.error." + result.getErrorKey(),
- format("Error when validating setting with key '%s' and value [%s]", data.key, data.values.stream().collect(Collectors.joining(", ")))));
+ format("Error when validating setting with key '%s' and value [%s]", data.key, String.join(", ", data.values))));
});
}
- private void validateMetric(SettingData data) {
- try (DbSession dbSession = dbClient.openSession(false)) {
- List<MetricDto> metrics = dbClient.metricDao().selectByKeys(dbSession, data.values).stream().filter(MetricDto::isEnabled).toList();
- checkRequest(data.values.size() == metrics.size(), "Error when validating metric setting with key '%s' and values [%s]. A value is not a valid metric key.",
- data.key, data.values.stream().collect(Collectors.joining(", ")));
- }
- }
-
private void validateLogin(SettingData data) {
try (DbSession dbSession = dbClient.openSession(false)) {
List<UserDto> users = dbClient.userDao().selectByLogins(dbSession, data.values).stream().filter(UserDto::isActive).toList();
checkRequest(data.values.size() == users.size(), "Error when validating login setting with key '%s' and values [%s]. A value is not a valid login.",
- data.key, data.values.stream().collect(Collectors.joining(", ")));
+ data.key, String.join(", ", data.values));
}
}
@@ -170,9 +175,7 @@ public class SettingValidations {
try {
new Gson().getAdapter(JsonElement.class).fromJson(jsonContent.get());
validateJsonSchema(jsonContent.get(), definition);
- } catch (ValidationException e) {
- throw new IllegalArgumentException(String.format("Provided JSON is invalid [%s]", e.getMessage()));
- } catch (IOException e) {
+ } catch (JsonParseException | IOException e) {
throw new IllegalArgumentException("Provided JSON is invalid");
}
}
@@ -180,13 +183,20 @@ public class SettingValidations {
private void validateJsonSchema(String json, PropertyDefinition definition) {
if (SECURITY_JSON_PROPERTIES.contains(definition.key())) {
- InputStream jsonSchemaInputStream = this.getClass().getClassLoader().getResourceAsStream("json-schemas/security.json");
- if (jsonSchemaInputStream != null) {
- JSONObject jsonSchema = new JSONObject(new JSONTokener(jsonSchemaInputStream));
- JSONObject jsonSubject = new JSONObject(new JSONTokener(json));
- SchemaLoader.load(jsonSchema).validate(jsonSubject);
- }
+ JsonValue jsonToValidate = new JsonParser(json).parse();
+ Optional.ofNullable(schemaValidator.validate(jsonToValidate))
+ .ifPresent(validationFailure -> {
+ ValidationFailure rootCause = getRootCause(validationFailure);
+ throw new IllegalArgumentException(String.format("Provided JSON is invalid : [%s at %s]", rootCause.getMessage(), rootCause.getInstance().getLocation()));
+ });
}
}
+
+ private static ValidationFailure getRootCause(ValidationFailure base) {
+ return base.getCauses().stream()
+ .map(ValueTypeValidation::getRootCause)
+ .findFirst()
+ .orElse(base);
+ }
}
}
diff --git a/server/sonar-webserver-webapi/src/main/resources/json-schemas/security.json b/server/sonar-webserver-webapi/src/main/resources/json-schemas/security.json
index e064fa83b54..a1bf14c6ccf 100644
--- a/server/sonar-webserver-webapi/src/main/resources/json-schemas/security.json
+++ b/server/sonar-webserver-webapi/src/main/resources/json-schemas/security.json
@@ -1,5 +1,4 @@
{
- "$schema": "http://json-schema.org/draft-07/schema#",
"title": "Custom configuration schema",
"description": "Schema to validate custom configuration given as input to the custom configuration properties",
"definitions": {