Feedback

confluentinc · Apr 6, 2020 · 55820d6 · 55820d6
1 parent 4e255f0
commit 55820d6
Show file tree

Hide file tree

Showing 2 changed files with 134 additions and 12 deletions.
diff --git a/ksqldb-cli/src/main/java/io/confluent/ksql/cli/console/UnclosedQuoteChecker.java b/ksqldb-cli/src/main/java/io/confluent/ksql/cli/console/UnclosedQuoteChecker.java
@@ -15,8 +15,6 @@
 
 package io.confluent.ksql.cli.console;
 
-import static java.util.Objects.requireNonNull;
-
 /**
  * Checks to see if the line is in the middle of a quote.  Unlike
  * org.jline.reader.impl.DefaultParser, this is comment aware.
@@ -27,11 +25,14 @@ public final class UnclosedQuoteChecker {
   private UnclosedQuoteChecker() {}
 
   public static boolean isUnclosedQuote(final String line) {
-    requireNonNull(line, "line");
     int quoteStart = -1;
-    for (int i = 0; line != null && i < line.length(); ++i) {
+    for (int i = 0; i < line.length(); ++i) {
       if (quoteStart < 0 && isQuoteChar(line, i)) {
         quoteStart = i;
+      } else if (quoteStart >= 0 && isTwoQuoteStart(line, i) && !isEscaped(line, i)) {
+        // Together, two quotes are effectively an escaped quote and don't act as a quote character.
+        // Skip the next quote char, since it's coupled with the first.
+        i++;
       } else if (quoteStart >= 0 && isQuoteChar(line, i) && !isEscaped(line, i)) {
         quoteStart = -1;
       }
@@ -59,9 +60,20 @@ private static boolean isEscaped(final String line, final int ind) {
       return false;
     }
     final char c = line.charAt(ind - 1);
-    if (c == '\\') {
+    if (c == '\\' && !isEscaped(line, ind - 1)) {
       return true;
     }
     return false;
   }
+
+  // Technically, it is sufficient to ensure that quotes are paired up to answer the "unclosed"
+  // question.  It's still clearer to differentiate between the two-quote escaped quote and two
+  // back-to-back strings (e.g. 'ab''cd' is really evaluated as "ab'cd" rather than 'ab' and 'cd'),
+  // so we explicitly check for that case here.
+  private static boolean isTwoQuoteStart(final String line, final int ind) {
+    if (ind + 1 >= line.length()) {
+      return false;
+    }
+    return isQuoteChar(line, ind) && isQuoteChar(line, ind + 1);
+  }
 }
diff --git a/ksqldb-cli/src/test/java/io/confluent/ksql/cli/console/UnclosedQuoteCheckerTest.java b/ksqldb-cli/src/test/java/io/confluent/ksql/cli/console/UnclosedQuoteCheckerTest.java
@@ -15,7 +15,9 @@
 
 package io.confluent.ksql.cli.console;
 
-import org.junit.Assert;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
 import org.junit.Test;
 
 public class UnclosedQuoteCheckerTest {
@@ -26,7 +28,25 @@ public void shouldFindUnclosedQuote() {
     final String line = "some line 'this is in a quote";
 
     // Then:
-    Assert.assertTrue(UnclosedQuoteChecker.isUnclosedQuote(line));
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(true));
+  }
+
+  @Test
+  public void shouldFindUnclosedQuote_commentCharsInside() {
+    // Given:
+    final String line = "some line 'this is in a quote -- not a comment";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(true));
+  }
+
+  @Test
+  public void shouldNotFindUnclosedQuote_commentCharsInside() {
+    // Given:
+    final String line = "some line 'this is in a quote -- not a comment'";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
   }
 
   @Test
@@ -35,7 +55,25 @@ public void shouldFindUnclosedQuote_escaped() {
     final String line = "some line 'this is in a quote\\'";
 
     // Then:
-    Assert.assertTrue(UnclosedQuoteChecker.isUnclosedQuote(line));
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(true));
+  }
+
+  @Test
+  public void shouldFindUnclosedQuote_escapedEscape() {
+    // Given:
+    final String line = "some line 'this is in a quote\\\\'";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
+  }
+
+  @Test
+  public void shouldFindUnclosedQuote_twoQuote() {
+    // Given:
+    final String line = "some line 'this is in a quote''";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(true));
   }
 
   @Test
@@ -44,7 +82,7 @@ public void shouldNotFindUnclosedQuote_endsQuote() {
     final String line = "some line 'this is in a quote'";
 
     // Then:
-    Assert.assertFalse(UnclosedQuoteChecker.isUnclosedQuote(line));
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
   }
 
   @Test
@@ -53,7 +91,70 @@ public void shouldNotFindUnclosedQuote_endsNonQuote() {
     final String line = "some line 'this is in a quote' more";
 
     // Then:
-    Assert.assertFalse(UnclosedQuoteChecker.isUnclosedQuote(line));
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
+  }
+
+  @Test
+  public void shouldNotFindUnclosedQuote_containsDoubleQuote() {
+    // Given:
+    final String line = "some line 'this is \"in\" a quote'";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
+  }
+
+  @Test
+  public void shouldNotFindUnclosedQuote_containsUnclosedDoubleQuote() {
+    // Given:
+    final String line = "some line 'this is \"in a quote'";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
+  }
+
+  @Test
+  public void shouldNotFindUnclosedQuote_escaped() {
+    // Given:
+    final String line = "some line 'this is in a quote\\''";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
+  }
+
+  @Test
+  public void shouldNotFindUnclosedQuote_twoQuote() {
+    // Given:
+    final String line = "some line 'this is in a quote'''";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
+  }
+
+  @Test
+  public void shouldFindUnclosedQuote_manyQuote() {
+    // Given:
+    final String line = "some line 'this is in a quote''''";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(true));
+  }
+
+  @Test
+  public void shouldNotFindUnclosedQuote_manyQuote() {
+    // Given:
+    final String line = "some line 'this is in a quote'''''";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
+  }
+
+  @Test
+  public void shouldNotFindUnclosedQuote_escapedAndMultipleQuotes() {
+    // Given:
+    final String line = "some line 'this is in a quote\\''''";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
   }
 
   @Test
@@ -62,7 +163,7 @@ public void shouldNotFindUnclosedQuote_inComment() {
     final String line = "some line -- 'this is in a comment";
 
     // Then:
-    Assert.assertFalse(UnclosedQuoteChecker.isUnclosedQuote(line));
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
   }
 
   @Test
@@ -71,6 +172,15 @@ public void shouldNotFindUnclosedQuote_onlyComment() {
     final String line = "some line -- this is a comment";
 
     // Then:
-    Assert.assertFalse(UnclosedQuoteChecker.isUnclosedQuote(line));
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
+  }
+
+  @Test
+  public void shouldNotFindUnclosedQuote_commentAfterQuote() {
+    // Given:
+    final String line = "some line 'quoted text' -- this is a comment";
+
+    // Then:
+    assertThat(UnclosedQuoteChecker.isUnclosedQuote(line), is(false));
   }
 }