From 08efbd1c85bafb217bf4e825930b0a5077c1ed43 Mon Sep 17 00:00:00 2001 From: YevhenBondarenko Date: Fri, 14 Apr 2023 18:06:59 +0200 Subject: [PATCH] revert tskv value noxss validation --- application/src/main/resources/thingsboard.yml | 2 -- .../server/controller/BaseTelemetryControllerTest.java | 6 ++---- .../server/common/data/kv/JsonDataEntry.java | 3 --- .../server/common/data/kv/StringDataEntry.java | 5 +---- .../server/dao/attributes/AttributeUtils.java | 8 ++++---- .../server/dao/attributes/BaseAttributesService.java | 7 ++----- .../server/dao/attributes/CachedAttributesService.java | 7 ++----- .../server/dao/timeseries/BaseTimeseriesService.java | 9 +++------ .../java/org/thingsboard/server/dao/util/KvUtils.java | 10 ++++------ .../server/dao/service/ConstraintValidatorTest.java | 7 ------- 10 files changed, 18 insertions(+), 46 deletions(-) diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index 1c1036dc14..14bf47301b 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -259,13 +259,11 @@ sql: batch_max_delay: "${SQL_ATTRIBUTES_BATCH_MAX_DELAY_MS:100}" stats_print_interval_ms: "${SQL_ATTRIBUTES_BATCH_STATS_PRINT_MS:10000}" batch_threads: "${SQL_ATTRIBUTES_BATCH_THREADS:3}" # batch thread count have to be a prime number like 3 or 5 to gain perfect hash distribution - noxss_validation_enabled: "${SQL_ATTRIBUTES_NOXSS_VALIDATION_ENABLED:true}" ts: batch_size: "${SQL_TS_BATCH_SIZE:10000}" batch_max_delay: "${SQL_TS_BATCH_MAX_DELAY_MS:100}" stats_print_interval_ms: "${SQL_TS_BATCH_STATS_PRINT_MS:10000}" batch_threads: "${SQL_TS_BATCH_THREADS:3}" # batch thread count have to be a prime number like 3 or 5 to gain perfect hash distribution - noxss_validation_enabled: "${SQL_TS_NOXSS_VALIDATION_ENABLED:true}" ts_latest: batch_size: "${SQL_TS_LATEST_BATCH_SIZE:10000}" batch_max_delay: "${SQL_TS_LATEST_BATCH_MAX_DELAY_MS:100}" diff --git a/application/src/test/java/org/thingsboard/server/controller/BaseTelemetryControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/BaseTelemetryControllerTest.java index af0d486869..50ad731a79 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseTelemetryControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseTelemetryControllerTest.java @@ -16,6 +16,7 @@ package org.thingsboard.server.controller; import org.junit.Test; +import org.stringtemplate.v4.ST; import org.thingsboard.server.common.data.Device; import org.thingsboard.server.common.data.SaveDeviceWithCredentialsRequest; import org.thingsboard.server.common.data.security.DeviceCredentials; @@ -32,10 +33,7 @@ public abstract class BaseTelemetryControllerTest extends AbstractControllerTest String correctRequestBody = "{\"data\": \"value\"}"; doPostAsync("/api/plugins/telemetry/" + device.getId() + "/SHARED_SCOPE", correctRequestBody, String.class, status().isOk()); doPostAsync("/api/plugins/telemetry/DEVICE/" + device.getId() + "/timeseries/smth", correctRequestBody, String.class, status().isOk()); - String invalidRequestBody = "{\"data\": \"alert(document)\\\">\"}"; - doPostAsync("/api/plugins/telemetry/" + device.getId() + "/SHARED_SCOPE", invalidRequestBody, String.class, status().isBadRequest()); - doPostAsync("/api/plugins/telemetry/DEVICE/" + device.getId() + "/timeseries/smth", invalidRequestBody, String.class, status().isBadRequest()); - invalidRequestBody = "{\"alert(document)\\\">\": \"data\"}"; + String invalidRequestBody = "{\"alert(document)\\\">\": \"data\"}"; doPostAsync("/api/plugins/telemetry/" + device.getId() + "/SHARED_SCOPE", invalidRequestBody, String.class, status().isBadRequest()); doPostAsync("/api/plugins/telemetry/DEVICE/" + device.getId() + "/timeseries/smth", invalidRequestBody, String.class, status().isBadRequest()); } diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/kv/JsonDataEntry.java b/common/data/src/main/java/org/thingsboard/server/common/data/kv/JsonDataEntry.java index 4260c512f8..0736b50b84 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/kv/JsonDataEntry.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/kv/JsonDataEntry.java @@ -15,14 +15,11 @@ */ package org.thingsboard.server.common.data.kv; -import org.thingsboard.server.common.data.validation.NoXss; - import java.util.Objects; import java.util.Optional; public class JsonDataEntry extends BasicKvEntry { - @NoXss private final String value; public JsonDataEntry(String key, String value) { diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/kv/StringDataEntry.java b/common/data/src/main/java/org/thingsboard/server/common/data/kv/StringDataEntry.java index 9251cc80b3..18a54327a2 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/kv/StringDataEntry.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/kv/StringDataEntry.java @@ -15,8 +15,6 @@ */ package org.thingsboard.server.common.data.kv; -import org.thingsboard.server.common.data.validation.NoXss; - import java.util.Objects; import java.util.Optional; @@ -24,7 +22,6 @@ public class StringDataEntry extends BasicKvEntry { private static final long serialVersionUID = 1L; - @NoXss private final String value; public StringDataEntry(String key, String value) { @@ -68,7 +65,7 @@ public class StringDataEntry extends BasicKvEntry { public String toString() { return "StringDataEntry{" + "value='" + value + '\'' + "} " + super.toString(); } - + @Override public String getValueAsString() { return value; diff --git a/dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java b/dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java index 980057a231..d1abeda5b6 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java +++ b/dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java @@ -30,12 +30,12 @@ public class AttributeUtils { Validator.validateString(scope, "Incorrect scope " + scope); } - public static void validate(List kvEntries, boolean validateNoxss) { - kvEntries.forEach(kv -> validate(kv, validateNoxss)); + public static void validate(List kvEntries) { + kvEntries.forEach(AttributeUtils::validate); } - public static void validate(AttributeKvEntry kvEntry, boolean validateNoxss) { - KvUtils.validate(kvEntry, validateNoxss); + public static void validate(AttributeKvEntry kvEntry) { + KvUtils.validate(kvEntry); if (kvEntry.getDataType() == null) { throw new IncorrectParameterException("Incorrect kvEntry. Data type can't be null"); } else { diff --git a/dao/src/main/java/org/thingsboard/server/dao/attributes/BaseAttributesService.java b/dao/src/main/java/org/thingsboard/server/dao/attributes/BaseAttributesService.java index 08cb8beeae..abd55b6b12 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/attributes/BaseAttributesService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/attributes/BaseAttributesService.java @@ -46,9 +46,6 @@ import static org.thingsboard.server.dao.attributes.AttributeUtils.validate; public class BaseAttributesService implements AttributesService { private final AttributesDao attributesDao; - @Value("${sql.attributes.noxss_validation_enabled:true}") - private boolean noxssValidationEnabled; - public BaseAttributesService(AttributesDao attributesDao) { this.attributesDao = attributesDao; } @@ -86,14 +83,14 @@ public class BaseAttributesService implements AttributesService { @Override public ListenableFuture save(TenantId tenantId, EntityId entityId, String scope, AttributeKvEntry attribute) { validate(entityId, scope); - AttributeUtils.validate(attribute, noxssValidationEnabled); + AttributeUtils.validate(attribute); return attributesDao.save(tenantId, entityId, scope, attribute); } @Override public ListenableFuture> save(TenantId tenantId, EntityId entityId, String scope, List attributes) { validate(entityId, scope); - AttributeUtils.validate(attributes, noxssValidationEnabled); + AttributeUtils.validate(attributes); List> saveFutures = attributes.stream().map(attribute -> attributesDao.save(tenantId, entityId, scope, attribute)).collect(Collectors.toList()); return Futures.allAsList(saveFutures); } diff --git a/dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java b/dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java index ef81ed695b..b95ce39d9a 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java @@ -70,9 +70,6 @@ public class CachedAttributesService implements AttributesService { @Value("${cache.type:caffeine}") private String cacheType; - @Value("${sql.attributes.noxss_validation_enabled:true}") - private boolean noxssValidationEnabled; - public CachedAttributesService(AttributesDao attributesDao, StatsFactory statsFactory, CacheExecutorService cacheExecutorService, @@ -215,7 +212,7 @@ public class CachedAttributesService implements AttributesService { @Override public ListenableFuture save(TenantId tenantId, EntityId entityId, String scope, AttributeKvEntry attribute) { validate(entityId, scope); - AttributeUtils.validate(attribute, noxssValidationEnabled); + AttributeUtils.validate(attribute); ListenableFuture future = attributesDao.save(tenantId, entityId, scope, attribute); return Futures.transform(future, key -> evict(entityId, scope, attribute, key), cacheExecutor); } @@ -223,7 +220,7 @@ public class CachedAttributesService implements AttributesService { @Override public ListenableFuture> save(TenantId tenantId, EntityId entityId, String scope, List attributes) { validate(entityId, scope); - AttributeUtils.validate(attributes, noxssValidationEnabled); + AttributeUtils.validate(attributes); List> futures = new ArrayList<>(attributes.size()); for (var attribute : attributes) { diff --git a/dao/src/main/java/org/thingsboard/server/dao/timeseries/BaseTimeseriesService.java b/dao/src/main/java/org/thingsboard/server/dao/timeseries/BaseTimeseriesService.java index 1f5f21acc8..748c57b5dc 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/timeseries/BaseTimeseriesService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/timeseries/BaseTimeseriesService.java @@ -81,9 +81,6 @@ public class BaseTimeseriesService implements TimeseriesService { @Value("${database.ts_max_intervals}") private long maxTsIntervals; - @Value("${sql.ts.noxss_validation_enabled:true}") - private boolean noxssValidationEnabled; - @Autowired private TimeseriesDao timeseriesDao; @@ -159,7 +156,7 @@ public class BaseTimeseriesService implements TimeseriesService { @Override public ListenableFuture save(TenantId tenantId, EntityId entityId, TsKvEntry tsKvEntry) { - KvUtils.validate(tsKvEntry, noxssValidationEnabled); + KvUtils.validate(tsKvEntry); validate(entityId); List> futures = Lists.newArrayListWithExpectedSize(INSERTS_PER_ENTRY); saveAndRegisterFutures(tenantId, futures, entityId, tsKvEntry, 0L); @@ -177,7 +174,7 @@ public class BaseTimeseriesService implements TimeseriesService { } private ListenableFuture doSave(TenantId tenantId, EntityId entityId, List tsKvEntries, long ttl, boolean saveLatest) { - KvUtils.validate(tsKvEntries, noxssValidationEnabled); + KvUtils.validate(tsKvEntries); int inserts = saveLatest ? INSERTS_PER_ENTRY : INSERTS_PER_ENTRY_WITHOUT_LATEST; List> futures = Lists.newArrayListWithExpectedSize(tsKvEntries.size() * inserts); for (TsKvEntry tsKvEntry : tsKvEntries) { @@ -192,7 +189,7 @@ public class BaseTimeseriesService implements TimeseriesService { @Override public ListenableFuture> saveLatest(TenantId tenantId, EntityId entityId, List tsKvEntries) { - KvUtils.validate(tsKvEntries, noxssValidationEnabled); + KvUtils.validate(tsKvEntries); List> futures = Lists.newArrayListWithExpectedSize(tsKvEntries.size()); for (TsKvEntry tsKvEntry : tsKvEntries) { futures.add(timeseriesLatestDao.saveLatest(tenantId, entityId, tsKvEntry)); diff --git a/dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java b/dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java index 133fb4c85e..8e80bd992d 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java +++ b/dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java @@ -22,16 +22,14 @@ import org.thingsboard.server.dao.service.ConstraintValidator; import java.util.List; public class KvUtils { - public static void validate(List tsKvEntries, boolean validateNoxss) { - tsKvEntries.forEach(kvEntry -> validate(kvEntry, validateNoxss)); + public static void validate(List tsKvEntries) { + tsKvEntries.forEach(KvUtils::validate); } - public static void validate(KvEntry tsKvEntry, boolean validateNoxss) { + public static void validate(KvEntry tsKvEntry) { if (tsKvEntry == null) { throw new IncorrectParameterException("Key value entry can't be null"); } - if (validateNoxss) { - ConstraintValidator.validateFields(tsKvEntry); - } + ConstraintValidator.validateFields(tsKvEntry); } } diff --git a/dao/src/test/java/org/thingsboard/server/dao/service/ConstraintValidatorTest.java b/dao/src/test/java/org/thingsboard/server/dao/service/ConstraintValidatorTest.java index fa3d39df30..24a32b6a61 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/service/ConstraintValidatorTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/service/ConstraintValidatorTest.java @@ -18,7 +18,6 @@ package org.thingsboard.server.dao.service; import org.junit.Assert; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import org.thingsboard.server.common.data.kv.JsonDataEntry; import org.thingsboard.server.common.data.kv.StringDataEntry; import org.thingsboard.server.dao.exception.DataValidationException; @@ -30,15 +29,9 @@ class ConstraintValidatorTest { @Test void validateFields() { StringDataEntry stringDataEntryValid = new StringDataEntry("key", "value"); - StringDataEntry stringDataEntryInvalid1 = new StringDataEntry("", "value"); - StringDataEntry stringDataEntryInvalid2 = new StringDataEntry("key", ""); - - JsonDataEntry jsonDataEntryInvalid = new JsonDataEntry("key", "{\"value\": }"); Assert.assertThrows(DataValidationException.class, () -> ConstraintValidator.validateFields(stringDataEntryInvalid1)); - Assert.assertThrows(DataValidationException.class, () -> ConstraintValidator.validateFields(stringDataEntryInvalid2)); - Assert.assertThrows(DataValidationException.class, () -> ConstraintValidator.validateFields(jsonDataEntryInvalid)); ConstraintValidator.validateFields(stringDataEntryValid); }