From 00486833ea6951006c07feda4488ab570a47123d Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Thu, 29 Sep 2022 10:37:19 +0530 Subject: [PATCH] fix: MongoPlugin smart substitution for regex arguments (#16977) * BugFix: Fix for mongoPlugin smart substitution of regex attributes, Fixes: https://github.com/appsmithorg/appsmith/issues/11880 Changes: MongoPlugin.java MongoPluginTest.java * Added test for decimal numbers and made changes to regex * code formatting: Formatted some test cases in MongoPluginTest.java --- .../com/external/plugins/MongoPlugin.java | 70 ++++++ .../com/external/plugins/MongoPluginTest.java | 206 +++++++++++++++++- 2 files changed, 275 insertions(+), 1 deletion(-) diff --git a/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java b/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java index 215d00636b..d61ff5bc19 100644 --- a/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java +++ b/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java @@ -156,6 +156,14 @@ public class MongoPlugin extends BasePlugin { */ private static final String MONGO_URI_REGEX = "^(mongodb(?:\\+srv)?://)(?:(.+):(.+)@)?([^/?]+)/?([^?]+)?\\??(.+)?$"; + /** + * We use this regex to identify the $regex attribute and the respective argument provided: + * e.g. {"code" : {$regex: value, $options: value}} / {"code" : {$regex: value}} + * capturing the value group for regex. + * e.g {"code" : {$regex: 8777}}, this whole substring will be matched and 8777 is captured for further processing. + */ + private static final String mongo$regexWithNumberIdentifier = ".*\\{[\\s\\n]*\\$regex[\\s\\n]*:([\\s\\n]*(?:(?:\\\"[/]*)|(?:/[\\\"]*)|)[-]?[\\d]*\\.?[\\d]*(?:(?:[/]*\\\")|(?:[\\\"]*/)|)[\\s\\n]*)(?:,|\\})"; + /** * We use this regex to find usage of special Mongo data types like ObjectId(...) wrapped inside double quotes * e.g. "ObjectId(...)". Case for single quotes e.g. 'ObjectId(...)' is not added because the way client sends @@ -574,9 +582,71 @@ public class MongoPlugin extends BasePlugin { params, parameters); + updatedQuery = makeMongoRegexSubstitutionValid (updatedQuery); + return updatedQuery; } + + private static StringBuilder makeValidForRegex(String value) { + + StringBuilder valueSB = new StringBuilder(value.trim()); + + char quote = '\"'; + char forwardSlash = '/'; + if ( valueSB.charAt(0) != quote && valueSB.charAt(0) != forwardSlash) { + valueSB.insert(0, quote ); + // adds quote at the end of the line + valueSB.append(quote); + } + + return valueSB; + } + + /** + * Smart Substitution Helper for mongo regex: Regex takes pattern as "" or //, + * Rest are not illegal arguments to the $regex attribute in mongo. + * @Param: Smart substituted query string. + * @Returns: Updated query string. + */ + private static String makeMongoRegexSubstitutionValid(String inputQuery) { + + int startIndex = 0; + StringBuilder inputQuerySB = new StringBuilder(); + + Pattern mongo$regexIdentifierPattern = Pattern.compile(mongo$regexWithNumberIdentifier); + Matcher mongo$regexIdentifierMatcher = mongo$regexIdentifierPattern.matcher(inputQuery); + + while (mongo$regexIdentifierMatcher.find()) { + + StringBuilder valueSB; + try { + valueSB = makeValidForRegex(mongo$regexIdentifierMatcher.group(1)); + + } catch(StringIndexOutOfBoundsException e) { + // when the match group detected is of length zero this error is thrown; + return inputQuery; + } catch (Exception e) { + return inputQuery; + } + + + //groups are discovered in greedy manner hence it's sorted by default + // a sub-group within a big group could be due to the regex arguments provide by user hence will not parse that + if (startIndex > mongo$regexIdentifierMatcher.start()) { + // the matcher walks greedily to find the pattern, if there is a group overlapping with other group means that it's a subquery, and it's not meant to be parsed. + continue; + } + inputQuerySB.append(inputQuery.substring(startIndex, mongo$regexIdentifierMatcher.start(1))); + inputQuerySB.append(valueSB); + startIndex = mongo$regexIdentifierMatcher.end(1); + } + + inputQuerySB.append(inputQuery.substring(startIndex)); + + return inputQuerySB.toString(); + } + /** * !!!WARNING!!! : formData gets updated as part of this flow (and hence is not being returned) * This function replaces the mustache variables using smart substitution and updates the existing formData diff --git a/app/server/appsmith-plugins/mongoPlugin/src/test/java/com/external/plugins/MongoPluginTest.java b/app/server/appsmith-plugins/mongoPlugin/src/test/java/com/external/plugins/MongoPluginTest.java index 0071584a15..dc45e5a8bc 100644 --- a/app/server/appsmith-plugins/mongoPlugin/src/test/java/com/external/plugins/MongoPluginTest.java +++ b/app/server/appsmith-plugins/mongoPlugin/src/test/java/com/external/plugins/MongoPluginTest.java @@ -163,6 +163,28 @@ public class MongoPluginTest { "state", "UP" )) ))).block(); + + + final MongoCollection teamCollection = mongoClient.getDatabase("test") + .getCollection("teams"); + Mono.from(teamCollection.insertMany(List.of( + new Document(Map.of( + "name", "Noisy Neighbours 2", + "goals_allowed", "20", + "goals_forwarded", "41", + "goal_difference", "+21", + "xGD", "-2.5", + "best_scoreline", "5-2" + )), + new Document(Map.of( + "name", "Red Side of the city", + "goals_allowed", "35", + "goals_forwarded", "28", + "goal_difference", "-7", + "xGD" , "+3.6", + "best_scoreline", "8-3" + )) + ))).block(); } return Mono.empty(); }).block(); @@ -558,8 +580,9 @@ public class MongoPluginTest { -> t2.getName().compareTo(t1.getName()) ); assertNotNull(structure); - assertEquals(2, structure.getTables().size()); + assertEquals(3, structure.getTables().size()); + // now there are three tables named final DatasourceStructure.Table usersTable = structure.getTables().get(0); assertEquals("users", usersTable.getName()); assertEquals(DatasourceStructure.TableType.COLLECTION, usersTable.getType()); @@ -2852,4 +2875,185 @@ public class MongoPluginTest { assertEquals(1, strings.size()); assertTrue(strings.contains("Missing default database name.")); } + + @Test + public void testRegexStringQueryWithSmartSubstitution() { + DatasourceConfiguration dsConfig = createDatasourceConfiguration(); + Mono dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig); + + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setEncodeParamsToggle(Boolean.TRUE); + + Map configMap = new HashMap<>(); + setDataValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE); + setDataValueSafelyInFormData(configMap, COMMAND, "RAW"); + + + String rawFind = "{ find: \"users\", \n " + + "filter: {\"name\":{$regex: \"{{appsmith.store.variable}}\"}}}"; + setDataValueSafelyInFormData(configMap, BODY, rawFind); + + actionConfiguration.setFormData(configMap); + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + executeActionDTO.setParams(List.of(new Param("appsmith.store.variable", "[a-zA-Z]{0,3}.*Ci.*"))); + + Mono actionExecutionResultMono = dsConnectionMono.flatMap(clientConnection -> pluginExecutor.executeParameterized(clientConnection, + executeActionDTO, + dsConfig, + actionConfiguration)); + StepVerifier.create(actionExecutionResultMono) + .assertNext(actionExecutionResult -> { + assertNotNull(actionExecutionResult); + assertTrue(actionExecutionResult.getIsExecutionSuccess()); + assertEquals(1, ((ArrayNode) actionExecutionResult.getBody()).size()); + assertEquals(List.of(new ParsedDataType(JSON), new ParsedDataType(RAW)).toString(), actionExecutionResult.getDataTypes().toString()); + }) + .verifyComplete(); + } + + + @Test + public void testRegexNumberQueryWithSmartSubstitution() { + DatasourceConfiguration dsConfig = createDatasourceConfiguration(); + Mono dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig); + + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setEncodeParamsToggle(Boolean.TRUE); + + Map configMap = new HashMap<>(); + setDataValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE); + setDataValueSafelyInFormData(configMap, COMMAND, "RAW"); + + String rawFind = "{ find: \"teams\", \n " + + "filter: {\"goals_allowed\":{$regex: \"{{appsmith.store.variable}}\"}}}"; + setDataValueSafelyInFormData(configMap, BODY, rawFind); + + actionConfiguration.setFormData(configMap); + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + executeActionDTO.setParams(List.of(new Param("appsmith.store.variable", "35"))); + + Mono actionExecutionResultMono = dsConnectionMono.flatMap(clientConnection -> pluginExecutor.executeParameterized(clientConnection, + executeActionDTO, + dsConfig, + actionConfiguration)); + + StepVerifier.create(actionExecutionResultMono) + .assertNext(actionExecutionResult -> { + assertNotNull(actionExecutionResult); + assertTrue(actionExecutionResult.getIsExecutionSuccess()); + assertEquals(1, ((ArrayNode) actionExecutionResult.getBody()).size()); + assertEquals(List.of(new ParsedDataType(JSON), new ParsedDataType(RAW)).toString(), actionExecutionResult.getDataTypes().toString()); + }) + .verifyComplete(); + } + + @Test + public void testRegexStringWithNumbersQueryWithSmartSubstitution() { + DatasourceConfiguration dsConfig = createDatasourceConfiguration(); + Mono dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig); + + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setEncodeParamsToggle(Boolean.TRUE); + + Map configMap = new HashMap<>(); + setDataValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE); + setDataValueSafelyInFormData(configMap, COMMAND, "RAW"); + + + String rawFind = "{ find: \"teams\", \n " + + "filter: {\"best_scoreline\":{$regex: \"{{appsmith.store.variable}}\"}}}"; + setDataValueSafelyInFormData(configMap, BODY, rawFind); + + actionConfiguration.setFormData(configMap); + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + executeActionDTO.setParams(List.of(new Param("appsmith.store.variable", "5-.*"))); + + Mono actionExecutionResultMono = dsConnectionMono.flatMap(clientConnection -> pluginExecutor.executeParameterized(clientConnection, + executeActionDTO, + dsConfig, + actionConfiguration)); + StepVerifier.create(actionExecutionResultMono) + .assertNext(actionExecutionResult -> { + assertNotNull(actionExecutionResult); + assertTrue(actionExecutionResult.getIsExecutionSuccess()); + System.out.println(actionExecutionResult.getBody()); + assertEquals(1, ((ArrayNode) actionExecutionResult.getBody()).size()); + assertEquals(List.of(new ParsedDataType(JSON), new ParsedDataType(RAW)).toString(), actionExecutionResult.getDataTypes().toString()); + }) + .verifyComplete(); + } + + @Test + public void testRegexNegativeNumbersQueryWithSmartSubstitution() { + DatasourceConfiguration dsConfig = createDatasourceConfiguration(); + Mono dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig); + + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setEncodeParamsToggle(Boolean.TRUE); + + Map configMap = new HashMap<>(); + setDataValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE); + setDataValueSafelyInFormData(configMap, COMMAND, "RAW"); + + + String rawFind = "{ find: \"teams\", \n " + + "filter: {\"goal_difference\":{$regex: \"{{appsmith.store.variable}}\"}}}"; + setDataValueSafelyInFormData(configMap, BODY, rawFind); + + actionConfiguration.setFormData(configMap); + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + executeActionDTO.setParams(List.of(new Param("appsmith.store.variable", "-7"))); + + Mono actionExecutionResultMono = dsConnectionMono.flatMap(clientConnection -> pluginExecutor.executeParameterized(clientConnection, + executeActionDTO, + dsConfig, + actionConfiguration)); + StepVerifier.create(actionExecutionResultMono) + .assertNext(actionExecutionResult -> { + assertNotNull(actionExecutionResult); + assertTrue(actionExecutionResult.getIsExecutionSuccess()); + System.out.println(actionExecutionResult.getBody()); + assertEquals(1, ((ArrayNode) actionExecutionResult.getBody()).size()); + assertEquals(List.of(new ParsedDataType(JSON), new ParsedDataType(RAW)).toString(), actionExecutionResult.getDataTypes().toString()); + }) + .verifyComplete(); + } + + + @Test + public void testRegexNegativeDecimalNumberQueryWithSmartSubstitution() { + DatasourceConfiguration dsConfig = createDatasourceConfiguration(); + Mono dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig); + + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setEncodeParamsToggle(Boolean.TRUE); + + Map configMap = new HashMap<>(); + setDataValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE); + setDataValueSafelyInFormData(configMap, COMMAND, "RAW"); + + + String rawFind = "{ find: \"teams\", \n " + + "filter: {\"xGD\":{$regex: \"{{appsmith.store.variable}}\"}}}"; + setDataValueSafelyInFormData(configMap, BODY, rawFind); + + actionConfiguration.setFormData(configMap); + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + executeActionDTO.setParams(List.of(new Param("appsmith.store.variable", "-2.5"))); + + Mono actionExecutionResultMono = dsConnectionMono.flatMap(clientConnection -> pluginExecutor.executeParameterized(clientConnection, + executeActionDTO, + dsConfig, + actionConfiguration)); + StepVerifier.create(actionExecutionResultMono) + .assertNext(actionExecutionResult -> { + assertNotNull(actionExecutionResult); + assertTrue(actionExecutionResult.getIsExecutionSuccess()); + System.out.println(actionExecutionResult.getBody()); + assertEquals(1, ((ArrayNode) actionExecutionResult.getBody()).size()); + assertEquals(List.of(new ParsedDataType(JSON), new ParsedDataType(RAW)).toString(), actionExecutionResult.getDataTypes().toString()); + }) + .verifyComplete(); + } + }