#684: add tag checking for invalid characters

src/main/java/org/influxdb/dto/BatchPoints.java

@@ -10,6 +10,7 @@
 import java.util.concurrent.TimeUnit;
 
 import org.influxdb.InfluxDB.ConsistencyLevel;
+import org.influxdb.dto.utils.CheckTags;
 
 /**
  * {Purpose of This Type}.
@@ -90,7 +91,11 @@ public Builder retentionPolicy(final String policy) {
      * @return the Builder instance.
      */
     public Builder tag(final String tagName, final String value) {
-      this.tags.put(tagName, value);
+      Objects.requireNonNull(tagName, "tagName");
+      Objects.requireNonNull(value, "value");
+      if (CheckTags.isLegalFullCheck(tagName, value)) {
+        this.tags.put(tagName, value);
+      }
       return this;
     }
 

src/main/java/org/influxdb/dto/Point.java

@@ -18,6 +18,7 @@
 import org.influxdb.annotation.Column;
 import org.influxdb.annotation.Measurement;
 import org.influxdb.annotation.TimeColumn;
+import org.influxdb.dto.utils.CheckTags;
 import org.influxdb.impl.Preconditions;
 
 /**
@@ -116,7 +117,7 @@ public static final class Builder {
     public Builder tag(final String tagName, final String value) {
       Objects.requireNonNull(tagName, "tagName");
       Objects.requireNonNull(value, "value");
-      if (!tagName.isEmpty() && !value.isEmpty()) {
+      if (CheckTags.isLegalFullCheck(tagName, value)) {
         tags.put(tagName, value);
       }
       return this;
@@ -131,7 +132,9 @@ public Builder tag(final String tagName, final String value) {
      */
     public Builder tag(final Map<String, String> tagsToAdd) {
       for (Entry<String, String> tag : tagsToAdd.entrySet()) {
-        tag(tag.getKey(), tag.getValue());
+        if (CheckTags.isLegalFullCheck(tag.getKey(), tag.getValue())) {
+          tags.put(tag.getKey(), tag.getValue());
+        }
       }
       return this;
     }

src/main/java/org/influxdb/dto/utils/CheckTags.java

@@ -0,0 +1,50 @@
+package org.influxdb.dto.utils;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Utility class intended for internal use.
+ * Checks the format of tag names and values to see if they conform to a regular expression.
+ *
+ * @author Brenton Poke
+ *
+ */
+public final class CheckTags {
+  static final String NAMEREGEX = "[^\r\n]+";
+  static final String VALUEREGEX = "[\\x00-\\x7F]+";
+  static final Pattern NAMEPATTERN = Pattern.compile(NAMEREGEX);
+  static final Pattern VALUEPATTERN = Pattern.compile(VALUEREGEX, Pattern.MULTILINE);
+
+  private CheckTags() { }
+  /**
+   * Check a single tag's name according to the corresponding regular expression.
+   *
+   * @param name
+   *            the tag name
+   * @return Boolean indicating that the tag name is legal
+   *
+   */
+  public static Boolean isTagNameLegal(final String name) {
+    final Matcher matcher = NAMEPATTERN.matcher(name);
+    return matcher.matches();
+  }
+  /**
+   * Check a single tag's name according to the corresponding regular expression.
+   *
+   * @param value
+   *            the tag value
+   * @return Boolean indicating that the tag value is legal
+   *
+   */
+  public static Boolean isTagValueLegal(final String value) {
+    final Matcher matcher = VALUEPATTERN.matcher(value);
+    return matcher.matches();
+  }
+  public static Boolean isLegalFullCheck(final String name, final String value) {
+    return !name.isEmpty()
+        && !value.isEmpty()
+        && CheckTags.isTagNameLegal(name)
+        && CheckTags.isTagValueLegal(value);
+  }
+}

src/test/java/org/influxdb/dto/CheckTagsTest.java

@@ -0,0 +1,147 @@
+package org.influxdb.dto;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.TimeUnit;
+import org.assertj.core.api.Assertions;
+import org.influxdb.dto.Point.Builder;
+import org.influxdb.dto.utils.CheckTags;
+import org.junit.Assert;
+import org.junit.jupiter.api.Test;
+import org.junit.platform.runner.JUnitPlatform;
+import org.junit.runner.RunWith;
+
+@RunWith(JUnitPlatform.class)
+public class CheckTagsTest {
+  @Test
+  public void TagNameNewLineTest() {
+    final String tagname = "ma\ndrid";
+    final String tagname1 = "madrid\n";
+    final String tagname2 = "\nmadrid";
+
+    Point point = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag(tagname,"city").addField("a", 1.0).build();
+    Assert.assertFalse(point.getTags().containsKey("madrid"));
+    Assert.assertFalse(point.getTags().containsKey("mad\nrid"));
+    Assert.assertFalse(CheckTags.isTagNameLegal(tagname));
+    Assert.assertFalse(CheckTags.isTagNameLegal(tagname1));
+    Assert.assertFalse(CheckTags.isTagNameLegal(tagname2));
+  }
+  @Test
+  public void TagNameCarriageReturnTest() {
+    final String tagname = "ma\rdrid";
+    final String tagname1 = "madrid\r";
+    final String tagname2 = "\rmadrid";
+    Point point = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag(tagname,"city").addField("a", 1.0).build();
+    Assert.assertFalse(point.getTags().containsKey("madrid"));
+    Assert.assertFalse(point.getTags().containsKey("mad\rrid"));
+    Assert.assertFalse(CheckTags.isTagNameLegal(tagname));
+    Assert.assertFalse(CheckTags.isTagNameLegal(tagname1));
+    Assert.assertFalse(CheckTags.isTagNameLegal(tagname2));
+  }
+  @Test
+  public void TagNameSymbolTest() {
+    final String tagname = "$cost";
+    final String tagname1 = "!cost";
+    Point point = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag(tagname,"$15").addField("a", 1.0).build();
+    Assert.assertFalse(point.getTags().containsKey("cost"));
+    Assert.assertTrue(point.getTags().containsKey("$cost"));
+    Assert.assertTrue(CheckTags.isTagNameLegal(tagname));
+    Assert.assertTrue(CheckTags.isTagNameLegal(tagname1));
+    final HashMap<String, String> map = new HashMap<>();
+    map.put("$cost","$15");
+    map.put("$mortgage","$34,000");
+    map.put("%interest","65%");
+    map.put("@email","[email protected]");
+    Point.Builder point1 = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag(map).addField("a", 1.0);
+    Assertions.assertThat(point1.build().getTags().values().equals(map.values()));
+  }
+  @Test
+  public void BatchPointsTagTest() {
+    final HashMap<String, String> map = new HashMap<>();
+    map.put("$cost","$15");
+    map.put("$mortgage","$34,000");
+    map.put("%interest","65%");
+    map.put("@email","[email protected]");
+    Point point = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).addField("a", 1.0).build();
+    BatchPoints.Builder points = BatchPoints.builder()
+        .tag("$cost","$15").tag("$mortgage","$34,000")
+        .tag("%interest","65%").tag("@email","[email protected]").point(point);
+    Assertions.assertThat(points.build().getPoints().get(0).getTags().equals(map.values()));
+    map.put("#phone","1-555-0101");
+    Assertions.assertThat(!points.build().getPoints().get(0).getTags().equals(map.values()));
+  }
+  @Test
+  public void LegalFullCheckTagTest() {
+    Assert.assertTrue(CheckTags.isLegalFullCheck("test","value"));
+    Assert.assertFalse(CheckTags.isLegalFullCheck("",""));
+    Assert.assertFalse(CheckTags.isLegalFullCheck("test",""));
+    Assert.assertFalse(CheckTags.isLegalFullCheck("","value"));
+    Assert.assertFalse(CheckTags.isLegalFullCheck("ma\ndrid", "city"));
+    Assert.assertFalse(CheckTags.isLegalFullCheck("city","ąćę"));
+    Assert.assertFalse(CheckTags.isLegalFullCheck("","ąćę"));
+  }
+  @Test
+  public void TagNameHyphenTest() {
+    final String tagname = "-cost";
+    final String tagname1 = "bushel-cost";
+    Point point = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag(tagname,"$15").addField("a", 1.0).build();
+    Assert.assertTrue(point.getTags().containsKey("-cost"));
+    Assert.assertFalse(point.getTags().containsKey("cost"));
+    Assert.assertTrue(CheckTags.isTagNameLegal(tagname));
+    Assert.assertTrue(CheckTags.isTagNameLegal(tagname1));
+  }
+  @Test
+  public void TagNameUnderscoreTest() {
+    final String tagname = "_cost_";
+    final String tagname1 = "bushel_cost";
+    Point point = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag(tagname,"$15").addField("a", 1.0).build();
+    Assert.assertTrue(point.getTags().containsKey("_cost_"));
+    Assert.assertTrue(CheckTags.isTagNameLegal(tagname));
+    Assert.assertTrue(CheckTags.isTagNameLegal(tagname1));
+  }
+  @Test
+  public void TagValueASCIITest() {
+    final String tagvalue = "$15";
+    final String tagvalue1 = "$34,000";
+    final String tagvalue2 = "65%";
+    final String tagvalue3 = "[email protected]";
+    final String tagvalue4 = "%SYSTEM_VARARG%";
+    Point.Builder point = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag("cost",tagvalue).addField("a", 1.0);
+    Assertions.assertThat(point.build().lineProtocol()).isEqualTo("test,cost=$15 a=1.0 1");
+    Assert.assertTrue(CheckTags.isTagValueLegal(tagvalue));
+    Assert.assertTrue(CheckTags.isTagValueLegal(tagvalue1));
+    Assert.assertTrue(CheckTags.isTagValueLegal(tagvalue2));
+    Assert.assertTrue(CheckTags.isTagValueLegal(tagvalue3));
+    Assert.assertTrue(CheckTags.isTagValueLegal(tagvalue4));
+    Assert.assertFalse(CheckTags.isTagValueLegal("ąćę"));
+    final HashMap<String, String> map = new HashMap<>();
+    map.put("cost",tagvalue);
+    map.put("mortgage",tagvalue1);
+    map.put("interest",tagvalue2);
+    map.put("email",tagvalue3);
+    Point.Builder point1 = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag(map).addField("a", 1.0);
+    Assertions.assertThat(point1.build().getTags().values().equals(map.values()));
+    map.put("phone","1-555-0101");
+    Assertions.assertThat(!point1.build().getTags().values().equals(map.values()));
+
+  }
+  @Test
+  public void TagsNullOrEmpty(){
+    final String tagname = null;
+    final String tagvalue = null;
+    final String tagvalue1 = "";
+
+    Assert.assertThrows(NullPointerException.class, ()-> {
+      Point.Builder point = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag("cost",tagvalue).addField("a", 1.0);
+    });
+
+    Assert.assertThrows(NullPointerException.class, ()-> {
+      Point.Builder point = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag(tagname,"whistle while you work").addField("a", 1.0);
+    });
+
+    Point.Builder point1 = Point.measurement("test").time(1, TimeUnit.NANOSECONDS).tag("cost",tagvalue1).addField("a", 1.0);
+    Assert.assertTrue(point1.build().getTags().isEmpty());
+
+  }
+}