diff options
author | Steve Marion <steve.marion@sonarsource.com> | 2024-03-12 16:11:13 +0100 |
---|---|---|
committer | sonartech <sonartech@sonarsource.com> | 2024-03-16 20:02:47 +0000 |
commit | 720d6dd5c4dc08327d8b2271949a66855fb88852 (patch) | |
tree | 47787da0581f8052d69d6b29c4c7e7823e91514e | |
parent | f4896ace30bcad83d4df1bcf4a768e6ff68facb0 (diff) | |
download | sonarqube-720d6dd5c4dc08327d8b2271949a66855fb88852.tar.gz sonarqube-720d6dd5c4dc08327d8b2271949a66855fb88852.zip |
SONAR-21451 replace everit json schema validation library with json-sKema. exclude commons-collection 3 from dependencies.
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": { |