DomGarguilo commented on code in PR #3927:
URL: https://github.com/apache/accumulo/pull/3927#discussion_r1383545518


##########
core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java:
##########
@@ -136,6 +141,9 @@ public enum PropertyType {
       "An arbitrary string of characters whose format is unspecified and"
           + " interpreted based on the context of the property to which it 
applies."),
 
+  JSON("json", x -> new ValidJson().test(x),

Review Comment:
   Would this create a new `ValidJson` object each time this is used? If so, is 
there a way to create a single instance for reuse?



##########
core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java:
##########
@@ -186,6 +195,39 @@ public boolean isValidFormat(String value) {
     return predicate.test(value);
   }
 
+  /**
+   * Validate that the provided string can be parsed into a json object. This 
implementation uses
+   * jackson databind because it is less permissive that GSON for what is 
considered valid. This
+   * implementation cannot guarantee that the json is valid for the target 
usage. That would require
+   * something like a json schema or a check specific to the use-case. This is 
only trying to
+   * provide a generic, minimal check that at least the json is valid.
+   */
+  private static class ValidJson implements Predicate<String> {
+    private static final Logger log = LoggerFactory.getLogger(ValidJson.class);
+
+    // set a limit of 1 million characters on the string as rough sanity check
+    private static final int ONE_MILLION = 1024 * 1024;
+
+    @Override
+    public boolean test(String value) {
+      try {
+        if (value.length() > ONE_MILLION) {
+          log.info("provided json string length {} is greater than limit of {} 
for parsing",
+              value.length(), ONE_MILLION);
+          return false;
+        }
+        ObjectMapper mapper =
+            new 
ObjectMapper().enable(DeserializationFeature.FAIL_ON_READING_DUP_TREE_KEY)
+                .enable(DeserializationFeature.FAIL_ON_TRAILING_TOKENS);

Review Comment:
   Similar comment here. Not sure how expensive this object is to create but 
maybe a `private static final ObjectMapper` could be created within this class 
for reuse.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to