modify PQL parser to handle escapes in string values

This modifies the parser to properly "unquote" incoming strings. So if
a string comes in double or single quoted, we approximately follow Go
rules for removing the quotes and processing escape sequences.

The differences from Go are:
1. we only support backslash, quote, tab and newline escape
sequenences.
2. Single quoted strings are supported and work just like double
quoted strings.
3. The peg parser won't actually accept backquoted strings (I don't
think)

Fixes: #411

pql/ast.go

@@ -134,6 +134,13 @@ func (q *Query) validateArgField(elem *callStackElem) {
 }
 
 func (q *Query) addVal(val interface{}) {
+	if vs, ok := val.(string); ok {
+		vsu, err := Unquote(vs)
+		if err != nil {
+			panic(err)
+		}
+		val = vsu
+	}
 	elem := q.lastCallStackElem()
 	if elem == nil || elem.lastField == "" {
 		panic(fmt.Sprintf("addVal called with '%s' when lastField is empty", val))

pql/parser.go

@@ -18,7 +18,9 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"strconv"
 	"strings"
+	"unicode/utf8"
 
 	"github.com/pkg/errors"
 )
@@ -95,3 +97,79 @@ func (p *parser) Parse() (*Query, error) {
 
 	return &p.Query, nil
 }
+
+// Unquote interprets s as a single-quoted, double-quoted, or
+// backquoted Go string literal, returning the string value that s
+// quotes. It is a copy of stdlib's strconv.Unquote, but modified so
+// that if s is single-quoted, it can still be a string rather than
+// only character literal. This version of Unquote also accepts
+// unquoted strings and passes them back unchanged.
+func Unquote(s string) (string, error) {
+	n := len(s)
+	if n < 2 {
+		return s, nil
+	}
+	quote := s[0]
+	if quote != '"' && quote != '\'' && quote != '`' {
+		return s, nil
+	}
+	if quote != s[n-1] {
+		return "", strconv.ErrSyntax
+	}
+	s = s[1 : n-1]
+
+	if quote == '`' {
+		if contains(s, '`') {
+			return "", strconv.ErrSyntax
+		}
+		if contains(s, '\r') {
+			// -1 because we know there is at least one \r to remove.
+			buf := make([]byte, 0, len(s)-1)
+			for i := 0; i < len(s); i++ {
+				if s[i] != '\r' {
+					buf = append(buf, s[i])
+				}
+			}
+			return string(buf), nil
+		}
+		return s, nil
+	}
+	if quote != '"' && quote != '\'' {
+		return "", strconv.ErrSyntax
+	}
+	if contains(s, '\n') {
+		return "", strconv.ErrSyntax
+	}
+
+	// Is it trivial? Avoid allocation.
+	if !contains(s, '\\') && !contains(s, quote) {
+		switch quote {
+		case '"', '\'':
+			if utf8.ValidString(s) {
+				return s, nil
+			}
+		}
+	}
+
+	var runeTmp [utf8.UTFMax]byte
+	buf := make([]byte, 0, 3*len(s)/2) // Try to avoid more allocations.
+	for len(s) > 0 {
+		c, multibyte, ss, err := strconv.UnquoteChar(s, quote)
+		if err != nil {
+			return "", err
+		}
+		s = ss
+		if c < utf8.RuneSelf || !multibyte {
+			buf = append(buf, byte(c))
+		} else {
+			n := utf8.EncodeRune(runeTmp[:], c)
+			buf = append(buf, runeTmp[:n]...)
+		}
+	}
+	return string(buf), nil
+}
+
+// contains reports whether the string contains the byte c.
+func contains(s string, c byte) bool {
+	return strings.ContainsRune(s, rune(c))
+}

pql/parser_test.go

@@ -16,6 +16,7 @@ package pql_test
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/pilosa/pilosa/v2/pql"
@@ -192,4 +193,70 @@ func TestParser_Parse(t *testing.T) {
 			t.Fatalf("unexpected call: %#v", q.Calls[0])
 		}
 	})
+
+}
+
+func TestUnquote(t *testing.T) {
+	tests := []struct {
+		name   string
+		value  string
+		exp    string
+		expErr string
+	}{
+		{
+			name:  "simple double",
+			value: `"hello"`,
+			exp:   "hello",
+		},
+		{
+			name:  "simple single",
+			value: `'hello'`,
+			exp:   "hello",
+		},
+		{
+			name:  "double with esc",
+			value: `"he\"llo"`,
+			exp:   "he\"llo",
+		},
+		{
+			name:  "single with esc",
+			value: `'he\'llo'`,
+			exp:   "he'llo",
+		},
+		{
+			name:  "single with backslash and esc",
+			value: `'he\\\'llo'`,
+			exp:   `he\'llo`,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got, err := pql.Unquote(test.value)
+			if testErr(t, test.expErr, err) {
+				return
+			}
+			if got != test.exp {
+				t.Errorf("exp: '%s'\ngot: '%s'", test.exp, got)
+			}
+		})
+	}
+
+}
+
+func testErr(t *testing.T, exp string, actual error) (done bool) {
+	t.Helper()
+	if exp == "" && actual == nil {
+		return false
+	}
+	if exp == "" && actual != nil {
+		t.Fatalf("unexpected error: %v", actual)
+	}
+	if exp != "" && actual == nil {
+		t.Fatalf("expected error like '%s'", exp)
+	}
+	if !strings.Contains(actual.Error(), exp) {
+		t.Fatalf("unmatched errs exp/got\n%s\n%v", exp, actual)
+	}
+	return true
 }

pql/pql.peg

@@ -64,8 +64,8 @@ itema <- ( 'null' &(comma / sp close) { p.addVal(nil) }
          )
 itemb <- ( < IDENT > { p.startCall(buffer[begin:end]) } open allargs comma? close { p.addVal(p.endCall()) }
          / < ([[A-Z]] / [0-9] / '-' / '_' / ':')+ > { p.addVal(buffer[begin:end]) }
-         / < '"'  doublequotedstring '"' > { s, _ := strconv.Unquote(buffer[begin:end]); p.addVal(s) }
-         / '\'' < singlequotedstring > '\'' { p.addVal(buffer[begin:end]) }
+         / < '"'  doublequotedstring '"' > { p.addVal(buffer[begin:end]) }
+         / < '\''  singlequotedstring  '\'' > { p.addVal(buffer[begin:end]) }
          )
 float <- ( < '-'? [0-9]+ ('.'[0-9]*)? > { p.addNumVal(buffer[begin:end], true) }
          / < '-'? '.'[0-9]+ > { p.addNumVal(buffer[begin:end], true) }
@@ -74,8 +74,8 @@ decimal <- ( < '-'? [0-9]+ ('.'[0-9]*)? > { p.addNumVal(buffer[begin:end], false
            / < '-'? '.'[0-9]+ > { p.addNumVal(buffer[begin:end], false) }
            )
 
-doublequotedstring <- ( '\\"' / '\\\\' / [^"] )*
-singlequotedstring <- ( '\\\'' / '\\\\' / [^'] )*
+doublequotedstring <- ( '\\"' / '\\\\' / '\\n' / '\\t' / [^"\\] )*
+singlequotedstring <- ( '\\\'' / '\\\\' / '\\n' / '\\t' /  [^'\\] )*
 
 fieldExpr <- ( [[A-Z]] / '_' ) ( [[A-Z]] / [0-9] / '_' / '-' )*
 field <- <fieldExpr / reserved> { p.addField(buffer[begin:end]) }
@@ -83,12 +83,12 @@ reserved <- ('_row' / '_col' / '_start' / '_end' / '_timestamp' / '_field')
 posfield <- <fieldExpr> { p.addPosStr("_field", buffer[begin:end]) }
 uint <- [1-9] [0-9]* / '0'
 col <- ( <uint> {p.addPosNum("_col", buffer[begin:end])}
-        / '\'' <singlequotedstring> '\'' {p.addPosStr("_col", buffer[begin:end])}
-        / '"' <doublequotedstring> '"' {p.addPosStr("_col", buffer[begin:end])}
+        / < '\'' singlequotedstring '\'' > {p.addPosStr("_col", buffer[begin:end])}
+        / < '"' doublequotedstring '"' > {p.addPosStr("_col", buffer[begin:end])}
         )
 row <- ( <uint> {p.addPosNum("_row", buffer[begin:end])}
-        / '\'' <singlequotedstring> '\'' {p.addPosStr("_row", buffer[begin:end])}
-        / '"' <doublequotedstring> '"' {p.addPosStr("_row", buffer[begin:end])}
+        / < '\'' singlequotedstring '\'' > {p.addPosStr("_row", buffer[begin:end])}
+        / < '"' doublequotedstring '"' > {p.addPosStr("_row", buffer[begin:end])}
         )
 
 open <- '(' sp

server/server_test.go

@@ -1092,6 +1092,50 @@ func TestClusterExhaustingConnections(t *testing.T) {
 	}
 }
 
+func TestQueryingWithQuotesAndStuff(t *testing.T) {
+	m := test.RunCommand(t)
+	defer m.Close()
+
+	client, err := http.NewInternalClient(m.API.Node().URI.HostPort(), http.GetHTTPClient(nil))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Execute Set() commands.
+	if err := client.CreateIndex(context.Background(), "i", pilosa.IndexOptions{Keys: true}); err != nil {
+		t.Fatal(err)
+	}
+	if err := client.CreateFieldWithOptions(context.Background(), "i", "fld", pilosa.FieldOptions{Keys: true}); err != nil {
+		t.Fatal(err)
+	}
+
+	// Test escaped single quote gets set properly
+	if res, err := m.Query(t, "i", "", `Set('bl\'ah', fld=ha)`); err != nil {
+		t.Fatal(err)
+	} else if !strings.Contains(res, "[true]") {
+		t.Errorf("setting escaped single quote result: %s", res)
+	}
+	if res, err := m.Query(t, "i", "", `Row(fld=ha)`); err != nil {
+		t.Fatal(err)
+	} else if !strings.Contains(res, `bl'ah`) {
+		t.Errorf("value with escaped single quote set improperly: %s", res)
+	}
+
+	// Test escaped double quote gets set properly
+	if res, err := m.Query(t, "i", "", `Set("d\"ah", fld=dq)`); err != nil {
+		t.Fatal(err)
+	} else if !strings.Contains(res, "[true]") {
+		t.Errorf("value with escaped double quote set improperly: %s", res)
+	}
+	if res, err := m.Query(t, "i", "", `Row(fld=dq)`); err != nil {
+		t.Fatal(err)
+	} else if !strings.Contains(res, `d\"ah`) {
+		// the backslash is there because JSON needs to escape the
+		// double quote since it uses double quotes
+		t.Errorf("value with escaped double quote set improperly: %s", res)
+	}
+}
+
 func TestClusterExhaustingConnectionsImport(t *testing.T) {
 	if !runStress {
 		t.Skip("stress")