Repository: ysugimoto/falco
Pull request #26 (2021-07-31 07:40:32)

re:implement regex validity linting
re-implement of https://github.com/ysugimoto/falco/pull/24

I've found lexer problem of escape sequence so fix that and finally, a regular expression can lint.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4b5fccd..257669e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## v0.3.0
+
+- fix: add statement linter [#22](https://github.com/ysugimoto/falco/pull/22) (@ysugimoto)
+- impl: Support edge dictionary [#20](https://github.com/ysugimoto/falco/pull/20) (@ysugimoto)
+
 ## v0.2.3
 
 - fix: prevent to die linter on return statement [#19](https://github.com/ysugimoto/falco/pull/19) (@ysugimoto)
diff --git a/lexer/lexer.go b/lexer/lexer.go
index bd8e5d3..12a41d0 100644
--- a/lexer/lexer.go
+++ b/lexer/lexer.go
@@ -388,6 +388,7 @@ func (l *Lexer) readString() string {
 			if l.peekChar() != 0x5C {
 				isEscape = true
 			}
+			rs = append(rs, l.char)
 			l.readChar()
 			continue
 		}
diff --git a/linter/linter.go b/linter/linter.go
index 7d479ef..b0ab1bd 100644
--- a/linter/linter.go
+++ b/linter/linter.go
@@ -3,6 +3,7 @@ package linter
 import (
 	"fmt"
 	"net"
+	"regexp"
 	"strings"
 
 	"github.com/ysugimoto/falco/ast"
@@ -1195,6 +1196,17 @@ func (l *Linter) lintInfixExpression(exp *ast.InfixExpression, ctx *context.Cont
 		} else if !expectType(right, types.StringType, types.IPType, types.AclType) {
 			l.Error(InvalidTypeExpression(exp.GetMeta(), right, types.StringType, types.IPType, types.AclType).Match(OPERATOR_CONDITIONAL))
 		}
+		// And, if right expression is STRING, regex must be valid
+		if v, ok := exp.Right.(*ast.String); ok {
+			if _, err := regexp.Compile(strings.ReplaceAll(v.Value, "\\", "\\\\")); err != nil {
+				err := &LintError{
+					Severity: ERROR,
+					Token:    exp.Right.GetMeta().Token,
+					Message:  "regex string is invalid, " + err.Error(),
+				}
+				l.Error(err)
+			}
+		}
 		return types.BoolType
 	case "+":
 		// Plus operator behaves string concatenation.


Test patch:
diff --git a/lexer/lexer_test.go b/lexer/lexer_test.go
index c2daec5..0e86564 100644
--- a/lexer/lexer_test.go
+++ b/lexer/lexer_test.go
@@ -558,7 +558,7 @@ func TestComplecatedStatement(t *testing.T) {
 		{Type: token.COMMA, Literal: ",", Line: 1, Position: 37},
 		{Type: token.STRING, Literal: `^.*?"exp"\s*:\s*(\d+).*?$`, Line: 1, Position: 39},
 		{Type: token.COMMA, Literal: ",", Line: 1, Position: 68},
-		{Type: token.STRING, Literal: `1`, Line: 1, Position: 70},
+		{Type: token.STRING, Literal: `\1`, Line: 1, Position: 70},
 		{Type: token.RIGHT_PAREN, Literal: ")", Line: 1, Position: 74},
 		{Type: token.SEMICOLON, Literal: ";", Line: 1, Position: 75},
 		{Type: token.EOF, Literal: "", Line: 1, Position: 76},
@@ -574,6 +574,22 @@ func TestComplecatedStatement(t *testing.T) {
 	}
 }
 
+func TestEscapeSequence(t *testing.T) {
+	input := `"\1"`
+	expects := []token.Token{
+		{Type: token.STRING, Literal: "\\1", Line: 1, Position: 1},
+		{Type: token.EOF, Literal: "", Line: 1, Position: 5},
+	}
+	l := NewFromString(input)
+	for i, tt := range expects {
+		tok := l.NextToken()
+
+		if diff := cmp.Diff(tt, tok, cmpopts.IgnoreFields(token.Token{}, "Offset")); diff != "" {
+			t.Errorf(`Tests[%d] failed, diff= %s`, i, diff)
+		}
+	}
+}
+
 func TestPeekToken(t *testing.T) {
 	input := `set var.expires`
 	l := NewFromString(input)
diff --git a/linter/linter_test.go b/linter/linter_test.go
index 711e3b1..3d8f1ed 100644
--- a/linter/linter_test.go
+++ b/linter/linter_test.go
@@ -1238,3 +1238,27 @@ sub vcl_recv {
 }`
 	assertNoError(t, input)
 }
+
+func TestRegexExpressionIsInvalid(t *testing.T) {
+	t.Run("pass", func(t *testing.T) {
+		input := `
+sub vcl_recv {
+	#Fastly recv
+	if (req.url ~ "^/([^\?]*)?(\?.*)?$") {
+		restart;
+	}
+}`
+		assertNoError(t, input)
+	})
+
+	t.Run("error: invalid regex", func(t *testing.T) {
+		input := `
+sub vcl_recv {
+	#Fastly recv
+	if (req.url ~ "^/([^\?]*)?(\?.*?$") {
+		restart;
+	}
+}`
+		assertError(t, input)
+	})
+}