From 1d4016bd87e5ac6eef8b0075da3b28d151014c5c Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Sat, 1 Aug 2020 00:27:48 -0400 Subject: [PATCH 01/20] #684: add tag checking for invalid characters --- .../java/org/influxdb/InfluxDBException.java | 12 +++++ .../java/org/influxdb/dto/BatchPoints.java | 14 ++++++ src/main/java/org/influxdb/dto/Point.java | 10 +++- .../org/influxdb/dto/utils/CheckTags.java | 47 +++++++++++++++++++ 4 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/influxdb/dto/utils/CheckTags.java diff --git a/src/main/java/org/influxdb/InfluxDBException.java b/src/main/java/org/influxdb/InfluxDBException.java index 9f826f8eb..27e901f84 100644 --- a/src/main/java/org/influxdb/InfluxDBException.java +++ b/src/main/java/org/influxdb/InfluxDBException.java @@ -52,6 +52,16 @@ public boolean isRetryWorth() { static final String USER_NOT_AUTHORIZED_ERROR = "user is not authorized to write to database"; static final String AUTHORIZATION_FAILED_ERROR = "authorization failed"; static final String USERNAME_REQUIRED_ERROR = "username required"; + static final String TAGGING_INVALID_ERROR = "tag name or value failed regex check"; + + public static final class TaggingInvalidException extends InfluxDBException{ + private TaggingInvalidException(final String message){ + super(message); + } + public boolean isRetryWorth() { + return false; + } + } public static final class DatabaseNotFoundException extends InfluxDBException { private DatabaseNotFoundException(final String message) { @@ -152,6 +162,8 @@ private static InfluxDBException buildExceptionFromErrorMessage(final String err if (errorMessage.contains(CACHE_MAX_MEMORY_SIZE_EXCEEDED_ERROR)) { return new CacheMaxMemorySizeExceededException(errorMessage); } + if (errorMessage.contains(TAGGING_INVALID_ERROR)) + return new TaggingInvalidException(errorMessage); if (errorMessage.contains(USER_REQUIRED_ERROR) || errorMessage.contains(USER_NOT_AUTHORIZED_ERROR) || errorMessage.contains(AUTHORIZATION_FAILED_ERROR) diff --git a/src/main/java/org/influxdb/dto/BatchPoints.java b/src/main/java/org/influxdb/dto/BatchPoints.java index e32774628..26c841e67 100644 --- a/src/main/java/org/influxdb/dto/BatchPoints.java +++ b/src/main/java/org/influxdb/dto/BatchPoints.java @@ -10,6 +10,10 @@ import java.util.concurrent.TimeUnit; import org.influxdb.InfluxDB.ConsistencyLevel; +import org.influxdb.InfluxDBException; +import org.influxdb.dto.utils.CheckTags; + +import static org.influxdb.dto.utils.CheckTags.*; /** * {Purpose of This Type}. @@ -29,6 +33,7 @@ public class BatchPoints { BatchPoints() { // Only visible in the Builder + } /** @@ -90,7 +95,16 @@ public Builder retentionPolicy(final String policy) { * @return the Builder instance. */ public Builder tag(final String tagName, final String value) { + Objects.requireNonNull(tagName, "tagName"); + Objects.requireNonNull(value, "value"); + if (!tagName.isEmpty() + && !value.isEmpty() + && CheckTags.isTagNameLegal(tagName) + && CheckTags.isTagValueLegal(value)) this.tags.put(tagName, value); + else { + throw InfluxDBException.buildExceptionForErrorState("tag name or value failed regex check"); + } return this; } diff --git a/src/main/java/org/influxdb/dto/Point.java b/src/main/java/org/influxdb/dto/Point.java index 422884e8a..6b91e5fdd 100755 --- a/src/main/java/org/influxdb/dto/Point.java +++ b/src/main/java/org/influxdb/dto/Point.java @@ -15,9 +15,11 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; import org.influxdb.BuilderException; +import org.influxdb.InfluxDBException; 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,9 +118,15 @@ 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 (!tagName.isEmpty() + && !value.isEmpty() + && CheckTags.isTagNameLegal(tagName) + && CheckTags.isTagValueLegal(value)) { tags.put(tagName, value); } + else { + throw InfluxDBException.buildExceptionForErrorState("tag name or value failed regex check"); + } return this; } diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java new file mode 100644 index 000000000..4cc0dd0e3 --- /dev/null +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -0,0 +1,47 @@ +package org.influxdb.dto.utils; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * {Purpose of This Type}. + * + * {Other Notes Relating to This Type (Optional)} + * + * @author BrentonPoke + * + */ +public final class CheckTags { + static final String nameRegex = "([a-zA-Z0-9-_]+)"; + static final String valueRegex = "[[:ascii:]]+"; + static final Pattern namePattern = Pattern.compile(nameRegex, Pattern.MULTILINE); + static final Pattern valuePattern = Pattern.compile(valueRegex, Pattern.MULTILINE); + + /** + * 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(String name){ + final Matcher matcher = namePattern.matcher(name); + return matcher.groupCount() == 1 && 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(String value){ + final Matcher matcher = valuePattern.matcher(value); + return matcher.matches(); + } + +} \ No newline at end of file From ef277d0415ade792f7ad1665979d4eb205fc93ef Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Sat, 1 Aug 2020 01:23:11 -0400 Subject: [PATCH 02/20] #684: forgot that posix-styled stuff doesn't work here --- src/main/java/org/influxdb/dto/Point.java | 7 ++++--- src/main/java/org/influxdb/dto/utils/CheckTags.java | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/influxdb/dto/Point.java b/src/main/java/org/influxdb/dto/Point.java index 6b91e5fdd..48fccfd26 100755 --- a/src/main/java/org/influxdb/dto/Point.java +++ b/src/main/java/org/influxdb/dto/Point.java @@ -124,9 +124,6 @@ public Builder tag(final String tagName, final String value) { && CheckTags.isTagValueLegal(value)) { tags.put(tagName, value); } - else { - throw InfluxDBException.buildExceptionForErrorState("tag name or value failed regex check"); - } return this; } @@ -139,7 +136,11 @@ public Builder tag(final String tagName, final String value) { */ public Builder tag(final Map tagsToAdd) { for (Entry tag : tagsToAdd.entrySet()) { + if(CheckTags.isTagNameLegal(tag.getKey()) + && CheckTags.isTagValueLegal(tag.getValue())) tag(tag.getKey(), tag.getValue()); + else + continue; } return this; } diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index 4cc0dd0e3..f53383db3 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -13,7 +13,7 @@ */ public final class CheckTags { static final String nameRegex = "([a-zA-Z0-9-_]+)"; - static final String valueRegex = "[[:ascii:]]+"; + static final String valueRegex = "[\\x00-\\x7F]+"; static final Pattern namePattern = Pattern.compile(nameRegex, Pattern.MULTILINE); static final Pattern valuePattern = Pattern.compile(valueRegex, Pattern.MULTILINE); @@ -28,7 +28,7 @@ public final class CheckTags { public static Boolean isTagNameLegal(String name){ final Matcher matcher = namePattern.matcher(name); - return matcher.groupCount() == 1 && matcher.matches(); + return matcher.matches(); } /** From f5aac7a2b6d1aeef8a48ca631e8b2f5ac10c7508 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Sat, 1 Aug 2020 01:53:21 -0400 Subject: [PATCH 03/20] cleaning out throwing of exception. --- src/main/java/org/influxdb/dto/BatchPoints.java | 3 --- src/main/java/org/influxdb/dto/utils/CheckTags.java | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/org/influxdb/dto/BatchPoints.java b/src/main/java/org/influxdb/dto/BatchPoints.java index 26c841e67..4a2dea742 100644 --- a/src/main/java/org/influxdb/dto/BatchPoints.java +++ b/src/main/java/org/influxdb/dto/BatchPoints.java @@ -102,9 +102,6 @@ public Builder tag(final String tagName, final String value) { && CheckTags.isTagNameLegal(tagName) && CheckTags.isTagValueLegal(value)) this.tags.put(tagName, value); - else { - throw InfluxDBException.buildExceptionForErrorState("tag name or value failed regex check"); - } return this; } diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index f53383db3..3c354de34 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -8,7 +8,7 @@ * * {Other Notes Relating to This Type (Optional)} * - * @author BrentonPoke + * @author Brenton Poke * */ public final class CheckTags { From 226eecea7cc4bc8f2c2a521fcd09b9c545bd17f9 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Sun, 9 Aug 2020 19:30:51 -0400 Subject: [PATCH 04/20] attempting to fix checkstyle errors --- .../java/org/influxdb/InfluxDBException.java | 7 ++++--- .../java/org/influxdb/dto/BatchPoints.java | 9 +++----- src/main/java/org/influxdb/dto/Point.java | 9 +++----- .../org/influxdb/dto/utils/CheckTags.java | 21 ++++++++----------- 4 files changed, 19 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/influxdb/InfluxDBException.java b/src/main/java/org/influxdb/InfluxDBException.java index 27e901f84..e803d34d7 100644 --- a/src/main/java/org/influxdb/InfluxDBException.java +++ b/src/main/java/org/influxdb/InfluxDBException.java @@ -54,8 +54,8 @@ public boolean isRetryWorth() { static final String USERNAME_REQUIRED_ERROR = "username required"; static final String TAGGING_INVALID_ERROR = "tag name or value failed regex check"; - public static final class TaggingInvalidException extends InfluxDBException{ - private TaggingInvalidException(final String message){ + public static final class TaggingInvalidException extends InfluxDBException { + private TaggingInvalidException(final String message) { super(message); } public boolean isRetryWorth() { @@ -162,8 +162,9 @@ private static InfluxDBException buildExceptionFromErrorMessage(final String err if (errorMessage.contains(CACHE_MAX_MEMORY_SIZE_EXCEEDED_ERROR)) { return new CacheMaxMemorySizeExceededException(errorMessage); } - if (errorMessage.contains(TAGGING_INVALID_ERROR)) + if (errorMessage.contains(TAGGING_INVALID_ERROR)) { return new TaggingInvalidException(errorMessage); + } if (errorMessage.contains(USER_REQUIRED_ERROR) || errorMessage.contains(USER_NOT_AUTHORIZED_ERROR) || errorMessage.contains(AUTHORIZATION_FAILED_ERROR) diff --git a/src/main/java/org/influxdb/dto/BatchPoints.java b/src/main/java/org/influxdb/dto/BatchPoints.java index 4a2dea742..5ac219965 100644 --- a/src/main/java/org/influxdb/dto/BatchPoints.java +++ b/src/main/java/org/influxdb/dto/BatchPoints.java @@ -10,11 +10,8 @@ import java.util.concurrent.TimeUnit; import org.influxdb.InfluxDB.ConsistencyLevel; -import org.influxdb.InfluxDBException; import org.influxdb.dto.utils.CheckTags; -import static org.influxdb.dto.utils.CheckTags.*; - /** * {Purpose of This Type}. * @@ -33,7 +30,6 @@ public class BatchPoints { BatchPoints() { // Only visible in the Builder - } /** @@ -100,8 +96,9 @@ public Builder tag(final String tagName, final String value) { if (!tagName.isEmpty() && !value.isEmpty() && CheckTags.isTagNameLegal(tagName) - && CheckTags.isTagValueLegal(value)) - this.tags.put(tagName, value); + && CheckTags.isTagValueLegal(value)) { + this.tags.put(tagName, value); + } return this; } diff --git a/src/main/java/org/influxdb/dto/Point.java b/src/main/java/org/influxdb/dto/Point.java index 48fccfd26..4ceb24ccf 100755 --- a/src/main/java/org/influxdb/dto/Point.java +++ b/src/main/java/org/influxdb/dto/Point.java @@ -15,7 +15,6 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; import org.influxdb.BuilderException; -import org.influxdb.InfluxDBException; import org.influxdb.annotation.Column; import org.influxdb.annotation.Measurement; import org.influxdb.annotation.TimeColumn; @@ -136,11 +135,9 @@ public Builder tag(final String tagName, final String value) { */ public Builder tag(final Map tagsToAdd) { for (Entry tag : tagsToAdd.entrySet()) { - if(CheckTags.isTagNameLegal(tag.getKey()) - && CheckTags.isTagValueLegal(tag.getValue())) - tag(tag.getKey(), tag.getValue()); - else - continue; + if (CheckTags.isTagNameLegal(tag.getKey()) && CheckTags.isTagValueLegal(tag.getValue())) { + tag(tag.getKey(), tag.getValue()); + } } return this; } diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index 3c354de34..30a490f00 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -12,11 +12,10 @@ * */ public final class CheckTags { - static final String nameRegex = "([a-zA-Z0-9-_]+)"; - static final String valueRegex = "[\\x00-\\x7F]+"; - static final Pattern namePattern = Pattern.compile(nameRegex, Pattern.MULTILINE); - static final Pattern valuePattern = Pattern.compile(valueRegex, Pattern.MULTILINE); - + static final String NAMEREGEX = "([a-zA-Z0-9-_]+)"; + static final String VALUEREGEX = "[\\x00-\\x7F]+"; + static final Pattern NAMEPATTERN = Pattern.compile(NAMEREGEX, Pattern.MULTILINE); + static final Pattern VALUEPATTERN = Pattern.compile(VALUEREGEX, Pattern.MULTILINE); /** * Check a single tag's name according to the corresponding regular expression. * @@ -25,9 +24,8 @@ public final class CheckTags { * @return Boolean indicating that the tag name is legal * */ - - public static Boolean isTagNameLegal(String name){ - final Matcher matcher = namePattern.matcher(name); + public static Boolean isTagNameLegal(final String name) { + final Matcher matcher = NAMEPATTERN.matcher(name); return matcher.matches(); } @@ -39,9 +37,8 @@ public static Boolean isTagNameLegal(String name){ * @return Boolean indicating that the tag value is legal * */ - public static Boolean isTagValueLegal(String value){ - final Matcher matcher = valuePattern.matcher(value); + public static Boolean isTagValueLegal(final String value) { + final Matcher matcher = VALUEPATTERN.matcher(value); return matcher.matches(); } - -} \ No newline at end of file +} From 04e1a21967ca02e941b784178b0fc90e947e001f Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Sun, 9 Aug 2020 19:40:35 -0400 Subject: [PATCH 05/20] attempting to fix checkstyle errors pt. 2 --- src/main/java/org/influxdb/InfluxDBException.java | 2 +- src/main/java/org/influxdb/dto/utils/CheckTags.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/influxdb/InfluxDBException.java b/src/main/java/org/influxdb/InfluxDBException.java index e803d34d7..eedfa2865 100644 --- a/src/main/java/org/influxdb/InfluxDBException.java +++ b/src/main/java/org/influxdb/InfluxDBException.java @@ -53,7 +53,7 @@ public boolean isRetryWorth() { static final String AUTHORIZATION_FAILED_ERROR = "authorization failed"; static final String USERNAME_REQUIRED_ERROR = "username required"; static final String TAGGING_INVALID_ERROR = "tag name or value failed regex check"; - + public static final class TaggingInvalidException extends InfluxDBException { private TaggingInvalidException(final String message) { super(message); diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index 30a490f00..68a8b1597 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -16,6 +16,8 @@ public final class CheckTags { static final String VALUEREGEX = "[\\x00-\\x7F]+"; static final Pattern NAMEPATTERN = Pattern.compile(NAMEREGEX, Pattern.MULTILINE); static final Pattern VALUEPATTERN = Pattern.compile(VALUEREGEX, Pattern.MULTILINE); + + private CheckTags() { } /** * Check a single tag's name according to the corresponding regular expression. * @@ -28,7 +30,6 @@ 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. * From 507fbb2e9cf0f796c565c49c486bbf08f05a63a7 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Sun, 9 Aug 2020 22:07:44 -0400 Subject: [PATCH 06/20] Adding unit tests for coverage --- .../java/org/influxdb/dto/CheckTagsTest.java | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 src/test/java/org/influxdb/dto/CheckTagsTest.java diff --git a/src/test/java/org/influxdb/dto/CheckTagsTest.java b/src/test/java/org/influxdb/dto/CheckTagsTest.java new file mode 100644 index 000000000..b8594d839 --- /dev/null +++ b/src/test/java/org/influxdb/dto/CheckTagsTest.java @@ -0,0 +1,54 @@ +package org.influxdb.dto; + +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 = "mad\ndrid"; + final String tagname1 = "maddrid\n"; + final String tagname2 = "\nmaddrid"; + 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"; + Assert.assertFalse(CheckTags.isTagNameLegal(tagname)); + Assert.assertFalse(CheckTags.isTagNameLegal(tagname1)); + } + @Test + public void TagNameHyphenTest() { + final String tagname = "-cost"; + final String tagname1 = "bushel-cost"; + Assert.assertTrue(CheckTags.isTagNameLegal(tagname)); + Assert.assertTrue(CheckTags.isTagNameLegal(tagname1)); + } + @Test + public void TagNameUnderscoreTest() { + final String tagname = "_cost_"; + final String tagname1 = "bushel_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 = "startrek@cbs.com"; + final String tagvalue4 = "%SYSTEM_VARARG%"; + 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)); + } +} From f25944969d3824fd98046a57c95ff63a3e722bad Mon Sep 17 00:00:00 2001 From: Brenton Date: Tue, 11 Aug 2020 14:06:12 -0400 Subject: [PATCH 07/20] removing unused exception --- src/main/java/org/influxdb/InfluxDBException.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/org/influxdb/InfluxDBException.java b/src/main/java/org/influxdb/InfluxDBException.java index eedfa2865..d76d46d78 100644 --- a/src/main/java/org/influxdb/InfluxDBException.java +++ b/src/main/java/org/influxdb/InfluxDBException.java @@ -52,7 +52,6 @@ public boolean isRetryWorth() { static final String USER_NOT_AUTHORIZED_ERROR = "user is not authorized to write to database"; static final String AUTHORIZATION_FAILED_ERROR = "authorization failed"; static final String USERNAME_REQUIRED_ERROR = "username required"; - static final String TAGGING_INVALID_ERROR = "tag name or value failed regex check"; public static final class TaggingInvalidException extends InfluxDBException { private TaggingInvalidException(final String message) { @@ -162,9 +161,6 @@ private static InfluxDBException buildExceptionFromErrorMessage(final String err if (errorMessage.contains(CACHE_MAX_MEMORY_SIZE_EXCEEDED_ERROR)) { return new CacheMaxMemorySizeExceededException(errorMessage); } - if (errorMessage.contains(TAGGING_INVALID_ERROR)) { - return new TaggingInvalidException(errorMessage); - } if (errorMessage.contains(USER_REQUIRED_ERROR) || errorMessage.contains(USER_NOT_AUTHORIZED_ERROR) || errorMessage.contains(AUTHORIZATION_FAILED_ERROR) From 4b9157deeceee2bbc03b1d5cd91b1c27e61c2784 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 04:42:41 -0400 Subject: [PATCH 08/20] revised per feedback on pullrequst --- src/main/java/org/influxdb/dto/Point.java | 7 +++- .../org/influxdb/dto/utils/CheckTags.java | 4 +- .../java/org/influxdb/dto/CheckTagsTest.java | 40 ++++++++++++++++++- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/influxdb/dto/Point.java b/src/main/java/org/influxdb/dto/Point.java index 4ceb24ccf..fa3d6cb2f 100755 --- a/src/main/java/org/influxdb/dto/Point.java +++ b/src/main/java/org/influxdb/dto/Point.java @@ -135,8 +135,11 @@ public Builder tag(final String tagName, final String value) { */ public Builder tag(final Map tagsToAdd) { for (Entry tag : tagsToAdd.entrySet()) { - if (CheckTags.isTagNameLegal(tag.getKey()) && CheckTags.isTagValueLegal(tag.getValue())) { - tag(tag.getKey(), tag.getValue()); + if (!tag.getKey().isEmpty() + && !tag.getValue().isEmpty() + && CheckTags.isTagNameLegal(tag.getKey()) + && CheckTags.isTagValueLegal(tag.getValue())) { + tags.put(tag.getKey(), tag.getValue()); } } return this; diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index 68a8b1597..1dde7462a 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -4,7 +4,7 @@ import java.util.regex.Pattern; /** - * {Purpose of This Type}. + * Utility class intended for internal use. Checks the format of tag names and values to see if they conform to a regular expression. * * {Other Notes Relating to This Type (Optional)} * @@ -12,7 +12,7 @@ * */ public final class CheckTags { - static final String NAMEREGEX = "([a-zA-Z0-9-_]+)"; + static final String NAMEREGEX = "([^\\\\n\\\\r]+)"; static final String VALUEREGEX = "[\\x00-\\x7F]+"; static final Pattern NAMEPATTERN = Pattern.compile(NAMEREGEX, Pattern.MULTILINE); static final Pattern VALUEPATTERN = Pattern.compile(VALUEREGEX, Pattern.MULTILINE); diff --git a/src/test/java/org/influxdb/dto/CheckTagsTest.java b/src/test/java/org/influxdb/dto/CheckTagsTest.java index b8594d839..5aad368a4 100644 --- a/src/test/java/org/influxdb/dto/CheckTagsTest.java +++ b/src/test/java/org/influxdb/dto/CheckTagsTest.java @@ -1,5 +1,7 @@ package org.influxdb.dto; +import java.util.concurrent.TimeUnit; +import org.assertj.core.api.Assertions; import org.influxdb.dto.utils.CheckTags; import org.junit.Assert; import org.junit.jupiter.api.Test; @@ -13,6 +15,10 @@ public void TagNameNewLineTest() { final String tagname = "mad\ndrid"; final String tagname1 = "maddrid\n"; final String tagname2 = "\nmaddrid"; + + 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)); @@ -21,13 +27,19 @@ public void TagNameNewLineTest() { public void TagNameSymbolTest() { final String tagname = "$cost"; final String tagname1 = "!cost"; - Assert.assertFalse(CheckTags.isTagNameLegal(tagname)); - Assert.assertFalse(CheckTags.isTagNameLegal(tagname1)); + 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)); } @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)); } @@ -35,6 +47,8 @@ public void TagNameHyphenTest() { 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)); } @@ -45,10 +59,32 @@ public void TagValueASCIITest() { final String tagvalue2 = "65%"; final String tagvalue3 = "startrek@cbs.com"; 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("ąćę")); + + } + @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()); + } } From a684bd9e7d2ba81991cd87cef0dee4543f87a884 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 04:52:30 -0400 Subject: [PATCH 09/20] missing a carriage return test --- .../java/org/influxdb/dto/CheckTagsTest.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/influxdb/dto/CheckTagsTest.java b/src/test/java/org/influxdb/dto/CheckTagsTest.java index 5aad368a4..f9236b647 100644 --- a/src/test/java/org/influxdb/dto/CheckTagsTest.java +++ b/src/test/java/org/influxdb/dto/CheckTagsTest.java @@ -12,9 +12,9 @@ public class CheckTagsTest { @Test public void TagNameNewLineTest() { - final String tagname = "mad\ndrid"; - final String tagname1 = "maddrid\n"; - final String tagname2 = "\nmaddrid"; + 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")); @@ -24,6 +24,18 @@ public void TagNameNewLineTest() { 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"; From 6d475c5817837fbf1160766cf3d4e6a87d2c4267 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 06:34:33 -0400 Subject: [PATCH 10/20] testBufferLimitGreaterThanActions is still failing for unkown reasons --- src/main/java/org/influxdb/dto/utils/CheckTags.java | 4 ++-- src/test/java/org/influxdb/BatchOptionsTest.java | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index 1dde7462a..d32a0ab4b 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -12,9 +12,9 @@ * */ public final class CheckTags { - static final String NAMEREGEX = "([^\\\\n\\\\r]+)"; + static final String NAMEREGEX = "[^\r\n]+"; static final String VALUEREGEX = "[\\x00-\\x7F]+"; - static final Pattern NAMEPATTERN = Pattern.compile(NAMEREGEX, Pattern.MULTILINE); + static final Pattern NAMEPATTERN = Pattern.compile(NAMEREGEX); static final Pattern VALUEPATTERN = Pattern.compile(VALUEREGEX, Pattern.MULTILINE); private CheckTags() { } diff --git a/src/test/java/org/influxdb/BatchOptionsTest.java b/src/test/java/org/influxdb/BatchOptionsTest.java index 351321b68..cadf800d2 100644 --- a/src/test/java/org/influxdb/BatchOptionsTest.java +++ b/src/test/java/org/influxdb/BatchOptionsTest.java @@ -382,6 +382,7 @@ protected void check(InvocationOnMock invocation) { result = spy.query(new Query("select * from measurement2", dbName)); //assert all 6 point written because of retry capable CACHE_MAX_MEMORY_SIZE_EXCEEDED_ERROR and RetryCapableBatchWriter did retry + System.out.println(result.getResults()); Assertions.assertEquals(6, result.getResults().get(0).getSeries().get(0).getValues().size()); } finally { From 3622a59d05a48e46e3f43734a6171b91108b665a Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 06:35:52 -0400 Subject: [PATCH 11/20] reverting prinout --- src/test/java/org/influxdb/BatchOptionsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/influxdb/BatchOptionsTest.java b/src/test/java/org/influxdb/BatchOptionsTest.java index cadf800d2..351321b68 100644 --- a/src/test/java/org/influxdb/BatchOptionsTest.java +++ b/src/test/java/org/influxdb/BatchOptionsTest.java @@ -382,7 +382,6 @@ protected void check(InvocationOnMock invocation) { result = spy.query(new Query("select * from measurement2", dbName)); //assert all 6 point written because of retry capable CACHE_MAX_MEMORY_SIZE_EXCEEDED_ERROR and RetryCapableBatchWriter did retry - System.out.println(result.getResults()); Assertions.assertEquals(6, result.getResults().get(0).getSeries().get(0).getValues().size()); } finally { From 37309e15104e22f98cd4661bc2dc1d6236e1031b Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 07:02:27 -0400 Subject: [PATCH 12/20] checkstyle doesn't like my comment --- src/main/java/org/influxdb/dto/utils/CheckTags.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index d32a0ab4b..464eb501f 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -4,7 +4,8 @@ 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. + * Utility class intended for internal use. + * Checks the format of tag names and values to see if they conform to a regular expression. * * {Other Notes Relating to This Type (Optional)} * From fced434e4138f05d10e32ee7636bd02c9fb4df58 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 07:03:05 -0400 Subject: [PATCH 13/20] let's remove that, too --- src/main/java/org/influxdb/dto/utils/CheckTags.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index 464eb501f..606ede438 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -7,8 +7,6 @@ * Utility class intended for internal use. * Checks the format of tag names and values to see if they conform to a regular expression. * - * {Other Notes Relating to This Type (Optional)} - * * @author Brenton Poke * */ From 89c3ece2dec691d4df3c45a9ebf56aa1e20cd001 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 07:24:07 -0400 Subject: [PATCH 14/20] cleaning up an unused exception class i made --- src/main/java/org/influxdb/InfluxDBException.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/main/java/org/influxdb/InfluxDBException.java b/src/main/java/org/influxdb/InfluxDBException.java index d76d46d78..9f826f8eb 100644 --- a/src/main/java/org/influxdb/InfluxDBException.java +++ b/src/main/java/org/influxdb/InfluxDBException.java @@ -53,15 +53,6 @@ public boolean isRetryWorth() { static final String AUTHORIZATION_FAILED_ERROR = "authorization failed"; static final String USERNAME_REQUIRED_ERROR = "username required"; - public static final class TaggingInvalidException extends InfluxDBException { - private TaggingInvalidException(final String message) { - super(message); - } - public boolean isRetryWorth() { - return false; - } - } - public static final class DatabaseNotFoundException extends InfluxDBException { private DatabaseNotFoundException(final String message) { super(message); From b38fd60004f1f85c4cf356bdd9bab0f58ecc539e Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 08:57:13 -0400 Subject: [PATCH 15/20] attempting better code coverage --- .../java/org/influxdb/dto/CheckTagsTest.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/influxdb/dto/CheckTagsTest.java b/src/test/java/org/influxdb/dto/CheckTagsTest.java index f9236b647..51844909b 100644 --- a/src/test/java/org/influxdb/dto/CheckTagsTest.java +++ b/src/test/java/org/influxdb/dto/CheckTagsTest.java @@ -1,5 +1,6 @@ package org.influxdb.dto; +import java.util.HashMap; import java.util.concurrent.TimeUnit; import org.assertj.core.api.Assertions; import org.influxdb.dto.utils.CheckTags; @@ -44,6 +45,13 @@ public void TagNameSymbolTest() { Assert.assertTrue(point.getTags().containsKey("$cost")); Assert.assertTrue(CheckTags.isTagNameLegal(tagname)); Assert.assertTrue(CheckTags.isTagNameLegal(tagname1)); + final HashMap map = new HashMap<>(); + map.put("$cost","$15"); + map.put("$mortgage","$34,000"); + map.put("%interest","65%"); + map.put("@email","startrek@cbs.com"); + 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 TagNameHyphenTest() { @@ -79,7 +87,14 @@ public void TagValueASCIITest() { Assert.assertTrue(CheckTags.isTagValueLegal(tagvalue3)); Assert.assertTrue(CheckTags.isTagValueLegal(tagvalue4)); Assert.assertFalse(CheckTags.isTagValueLegal("ąćę")); - + final HashMap 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())); + } @Test public void TagsNullOrEmpty(){ From 0725aefd5c9fa79e2c277af1ace420db8f2e78f4 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 11:24:10 -0400 Subject: [PATCH 16/20] attempting to raise code coverage. --- .../java/org/influxdb/dto/CheckTagsTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/java/org/influxdb/dto/CheckTagsTest.java b/src/test/java/org/influxdb/dto/CheckTagsTest.java index 51844909b..d6d62be04 100644 --- a/src/test/java/org/influxdb/dto/CheckTagsTest.java +++ b/src/test/java/org/influxdb/dto/CheckTagsTest.java @@ -3,6 +3,7 @@ import java.util.HashMap; 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; @@ -54,6 +55,21 @@ public void TagNameSymbolTest() { Assertions.assertThat(point1.build().getTags().values().equals(map.values())); } @Test + public void BatchPointsTagTest() { + final HashMap map = new HashMap<>(); + map.put("$cost","$15"); + map.put("$mortgage","$34,000"); + map.put("%interest","65%"); + map.put("@email","startrek@cbs.com"); + 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","startrek@cbs.com").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 TagNameHyphenTest() { final String tagname = "-cost"; final String tagname1 = "bushel-cost"; @@ -94,6 +110,8 @@ public void TagValueASCIITest() { 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 From 735ffb472e8d81397ab1cc68451e238dfaf38e54 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 12:25:19 -0400 Subject: [PATCH 17/20] refactor --- src/main/java/org/influxdb/dto/BatchPoints.java | 5 +---- src/main/java/org/influxdb/dto/Point.java | 10 ++-------- src/main/java/org/influxdb/dto/utils/CheckTags.java | 8 ++++++++ src/test/java/org/influxdb/dto/CheckTagsTest.java | 11 +++++++++++ 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/influxdb/dto/BatchPoints.java b/src/main/java/org/influxdb/dto/BatchPoints.java index 5ac219965..1c42cbebe 100644 --- a/src/main/java/org/influxdb/dto/BatchPoints.java +++ b/src/main/java/org/influxdb/dto/BatchPoints.java @@ -93,10 +93,7 @@ public Builder retentionPolicy(final String policy) { public Builder tag(final String tagName, final String value) { Objects.requireNonNull(tagName, "tagName"); Objects.requireNonNull(value, "value"); - if (!tagName.isEmpty() - && !value.isEmpty() - && CheckTags.isTagNameLegal(tagName) - && CheckTags.isTagValueLegal(value)) { + if (CheckTags.isLegalFullCheck(tagName,value)) { this.tags.put(tagName, value); } return this; diff --git a/src/main/java/org/influxdb/dto/Point.java b/src/main/java/org/influxdb/dto/Point.java index fa3d6cb2f..36dffce1e 100755 --- a/src/main/java/org/influxdb/dto/Point.java +++ b/src/main/java/org/influxdb/dto/Point.java @@ -117,10 +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() - && CheckTags.isTagNameLegal(tagName) - && CheckTags.isTagValueLegal(value)) { + if (CheckTags.isLegalFullCheck(tagName,value)) { tags.put(tagName, value); } return this; @@ -135,10 +132,7 @@ public Builder tag(final String tagName, final String value) { */ public Builder tag(final Map tagsToAdd) { for (Entry tag : tagsToAdd.entrySet()) { - if (!tag.getKey().isEmpty() - && !tag.getValue().isEmpty() - && CheckTags.isTagNameLegal(tag.getKey()) - && CheckTags.isTagValueLegal(tag.getValue())) { + if (CheckTags.isLegalFullCheck(tag.getKey(),tag.getValue())) { tags.put(tag.getKey(), tag.getValue()); } } diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index 606ede438..9ba3bc6ad 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -1,5 +1,7 @@ package org.influxdb.dto.utils; +import java.util.Map; +import java.util.Map.Entry; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -41,4 +43,10 @@ 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); + } } diff --git a/src/test/java/org/influxdb/dto/CheckTagsTest.java b/src/test/java/org/influxdb/dto/CheckTagsTest.java index d6d62be04..3785b3303 100644 --- a/src/test/java/org/influxdb/dto/CheckTagsTest.java +++ b/src/test/java/org/influxdb/dto/CheckTagsTest.java @@ -1,6 +1,8 @@ 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; @@ -70,6 +72,15 @@ public void BatchPointsTagTest() { 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","ąćę")); + } + @Test public void TagNameHyphenTest() { final String tagname = "-cost"; final String tagname1 = "bushel-cost"; From 78725e39a34a5dd3dc0c03ca8fc8bbc22cedd711 Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 12:31:48 -0400 Subject: [PATCH 18/20] fixing checkstyle --- src/main/java/org/influxdb/dto/BatchPoints.java | 2 +- src/main/java/org/influxdb/dto/Point.java | 4 ++-- src/main/java/org/influxdb/dto/utils/CheckTags.java | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/influxdb/dto/BatchPoints.java b/src/main/java/org/influxdb/dto/BatchPoints.java index 1c42cbebe..5602732f2 100644 --- a/src/main/java/org/influxdb/dto/BatchPoints.java +++ b/src/main/java/org/influxdb/dto/BatchPoints.java @@ -93,7 +93,7 @@ public Builder retentionPolicy(final String policy) { public Builder tag(final String tagName, final String value) { Objects.requireNonNull(tagName, "tagName"); Objects.requireNonNull(value, "value"); - if (CheckTags.isLegalFullCheck(tagName,value)) { + if (CheckTags.isLegalFullCheck(tagName, value)) { this.tags.put(tagName, value); } return this; diff --git a/src/main/java/org/influxdb/dto/Point.java b/src/main/java/org/influxdb/dto/Point.java index 36dffce1e..b8900c70d 100755 --- a/src/main/java/org/influxdb/dto/Point.java +++ b/src/main/java/org/influxdb/dto/Point.java @@ -117,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 (CheckTags.isLegalFullCheck(tagName,value)) { + if (CheckTags.isLegalFullCheck(tagName, value)) { tags.put(tagName, value); } return this; @@ -132,7 +132,7 @@ public Builder tag(final String tagName, final String value) { */ public Builder tag(final Map tagsToAdd) { for (Entry tag : tagsToAdd.entrySet()) { - if (CheckTags.isLegalFullCheck(tag.getKey(),tag.getValue())) { + if (CheckTags.isLegalFullCheck(tag.getKey(), tag.getValue())) { tags.put(tag.getKey(), tag.getValue()); } } diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index 9ba3bc6ad..1ab24a6af 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -1,7 +1,5 @@ package org.influxdb.dto.utils; -import java.util.Map; -import java.util.Map.Entry; import java.util.regex.Matcher; import java.util.regex.Pattern; From 0e8c7aa07ff5bd49fec59ba93f1590a1244ff18c Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Sat, 15 Aug 2020 02:07:48 -0400 Subject: [PATCH 19/20] adding another case --- src/test/java/org/influxdb/dto/CheckTagsTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/influxdb/dto/CheckTagsTest.java b/src/test/java/org/influxdb/dto/CheckTagsTest.java index 3785b3303..d492b7e5c 100644 --- a/src/test/java/org/influxdb/dto/CheckTagsTest.java +++ b/src/test/java/org/influxdb/dto/CheckTagsTest.java @@ -79,6 +79,7 @@ public void LegalFullCheckTagTest() { 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() { From 7b6c2cf40f1ea64cf8379949be6194c02234679b Mon Sep 17 00:00:00 2001 From: Brenton Poke Date: Fri, 14 Aug 2020 04:52:30 -0400 Subject: [PATCH 20/20] missing a carriage return test testBufferLimitGreaterThanActions is still failing for unkown reasons reverting prinout checkstyle doesn't like my comment let's remove that, too cleaning up an unused exception class i made attempting better code coverage attempting to raise code coverage. refactor fixing checkstyle adding another case --- .../java/org/influxdb/InfluxDBException.java | 9 --- .../java/org/influxdb/dto/BatchPoints.java | 5 +- src/main/java/org/influxdb/dto/Point.java | 10 +-- .../org/influxdb/dto/utils/CheckTags.java | 15 +++-- .../java/org/influxdb/dto/CheckTagsTest.java | 65 +++++++++++++++++-- 5 files changed, 74 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/influxdb/InfluxDBException.java b/src/main/java/org/influxdb/InfluxDBException.java index d76d46d78..9f826f8eb 100644 --- a/src/main/java/org/influxdb/InfluxDBException.java +++ b/src/main/java/org/influxdb/InfluxDBException.java @@ -53,15 +53,6 @@ public boolean isRetryWorth() { static final String AUTHORIZATION_FAILED_ERROR = "authorization failed"; static final String USERNAME_REQUIRED_ERROR = "username required"; - public static final class TaggingInvalidException extends InfluxDBException { - private TaggingInvalidException(final String message) { - super(message); - } - public boolean isRetryWorth() { - return false; - } - } - public static final class DatabaseNotFoundException extends InfluxDBException { private DatabaseNotFoundException(final String message) { super(message); diff --git a/src/main/java/org/influxdb/dto/BatchPoints.java b/src/main/java/org/influxdb/dto/BatchPoints.java index 5ac219965..5602732f2 100644 --- a/src/main/java/org/influxdb/dto/BatchPoints.java +++ b/src/main/java/org/influxdb/dto/BatchPoints.java @@ -93,10 +93,7 @@ public Builder retentionPolicy(final String policy) { public Builder tag(final String tagName, final String value) { Objects.requireNonNull(tagName, "tagName"); Objects.requireNonNull(value, "value"); - if (!tagName.isEmpty() - && !value.isEmpty() - && CheckTags.isTagNameLegal(tagName) - && CheckTags.isTagValueLegal(value)) { + if (CheckTags.isLegalFullCheck(tagName, value)) { this.tags.put(tagName, value); } return this; diff --git a/src/main/java/org/influxdb/dto/Point.java b/src/main/java/org/influxdb/dto/Point.java index fa3d6cb2f..b8900c70d 100755 --- a/src/main/java/org/influxdb/dto/Point.java +++ b/src/main/java/org/influxdb/dto/Point.java @@ -117,10 +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() - && CheckTags.isTagNameLegal(tagName) - && CheckTags.isTagValueLegal(value)) { + if (CheckTags.isLegalFullCheck(tagName, value)) { tags.put(tagName, value); } return this; @@ -135,10 +132,7 @@ public Builder tag(final String tagName, final String value) { */ public Builder tag(final Map tagsToAdd) { for (Entry tag : tagsToAdd.entrySet()) { - if (!tag.getKey().isEmpty() - && !tag.getValue().isEmpty() - && CheckTags.isTagNameLegal(tag.getKey()) - && CheckTags.isTagValueLegal(tag.getValue())) { + if (CheckTags.isLegalFullCheck(tag.getKey(), tag.getValue())) { tags.put(tag.getKey(), tag.getValue()); } } diff --git a/src/main/java/org/influxdb/dto/utils/CheckTags.java b/src/main/java/org/influxdb/dto/utils/CheckTags.java index 1dde7462a..1ab24a6af 100644 --- a/src/main/java/org/influxdb/dto/utils/CheckTags.java +++ b/src/main/java/org/influxdb/dto/utils/CheckTags.java @@ -4,17 +4,16 @@ 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. - * - * {Other Notes Relating to This Type (Optional)} + * 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 = "([^\\\\n\\\\r]+)"; + static final String NAMEREGEX = "[^\r\n]+"; static final String VALUEREGEX = "[\\x00-\\x7F]+"; - static final Pattern NAMEPATTERN = Pattern.compile(NAMEREGEX, Pattern.MULTILINE); + static final Pattern NAMEPATTERN = Pattern.compile(NAMEREGEX); static final Pattern VALUEPATTERN = Pattern.compile(VALUEREGEX, Pattern.MULTILINE); private CheckTags() { } @@ -42,4 +41,10 @@ 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); + } } diff --git a/src/test/java/org/influxdb/dto/CheckTagsTest.java b/src/test/java/org/influxdb/dto/CheckTagsTest.java index 5aad368a4..d492b7e5c 100644 --- a/src/test/java/org/influxdb/dto/CheckTagsTest.java +++ b/src/test/java/org/influxdb/dto/CheckTagsTest.java @@ -1,7 +1,11 @@ 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; @@ -12,9 +16,9 @@ public class CheckTagsTest { @Test public void TagNameNewLineTest() { - final String tagname = "mad\ndrid"; - final String tagname1 = "maddrid\n"; - final String tagname2 = "\nmaddrid"; + 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")); @@ -24,6 +28,18 @@ public void TagNameNewLineTest() { 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"; @@ -32,6 +48,38 @@ public void TagNameSymbolTest() { Assert.assertTrue(point.getTags().containsKey("$cost")); Assert.assertTrue(CheckTags.isTagNameLegal(tagname)); Assert.assertTrue(CheckTags.isTagNameLegal(tagname1)); + final HashMap map = new HashMap<>(); + map.put("$cost","$15"); + map.put("$mortgage","$34,000"); + map.put("%interest","65%"); + map.put("@email","startrek@cbs.com"); + 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 map = new HashMap<>(); + map.put("$cost","$15"); + map.put("$mortgage","$34,000"); + map.put("%interest","65%"); + map.put("@email","startrek@cbs.com"); + 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","startrek@cbs.com").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() { @@ -67,7 +115,16 @@ public void TagValueASCIITest() { Assert.assertTrue(CheckTags.isTagValueLegal(tagvalue3)); Assert.assertTrue(CheckTags.isTagValueLegal(tagvalue4)); Assert.assertFalse(CheckTags.isTagValueLegal("ąćę")); - + final HashMap 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(){