Repository: ysugimoto/falco
Pull request #91 (2022-07-28 21:01:59)

Fix lint of `log` statement
Fastly doesn't allow literals to be passed to log staments but
falco was ok with it. We fix it and add some tests to solidify
the behavior.

Fiddle: https://fiddle.fastlydemo.net/fiddle/1d772f20

Signed-off-by: Sotiris Nanopoulos <sotiris.nanopoulos@reddit.com>


Patch:
diff --git a/Makefile b/Makefile
index 87d666a..eed7f5c 100644
--- a/Makefile
+++ b/Makefile
@@ -6,7 +6,7 @@ generate:
 	cd ./__generator__/ && go generate .
 
 test: generate
-	go list ./... | xargs go test
+	go test ./...
 
 linux:
 	GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build \
diff --git a/__generator__/predefined.yml b/__generator__/predefined.yml
index 02f912d..45a37ba 100644
--- a/__generator__/predefined.yml
+++ b/__generator__/predefined.yml
@@ -716,6 +716,11 @@ fastly_info.is_h2:
   on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
   get: BOOL
 
+fastly_info.is_h3:
+  reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/fastly-info-is-h3/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: BOOL
+
 quic.cc.cwnd:
   reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/quic-cc-cwnd/"
   on: [RECV, HASH, DELIVER, LOG]
@@ -836,6 +841,11 @@ tls.client.iana_chosen_cipher_id:
   on: [RECV, HASH, DELIVER, LOG]
   get: INTEGER
 
+tls.client.ja3_md5:
+  reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/tls-client-ja3-md5/"
+  on: [RECV, HASH, DELIVER, LOG]
+  get: STRING
+
 tls.client.protocol:
   reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/tls-client-protocol/"
   on: [RECV, HASH, DELIVER, LOG]
@@ -866,6 +876,16 @@ tls.client.tlsexts_sha:
   on: [RECV, HASH, DELIVER, LOG]
   get: STRING
 
+transport.bw_estimate:
+  reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/transport-bw-estimate/"
+  on: [RECV, HASH, DELIVER, LOG]
+  get: INTEGER
+
+transport.type:
+  reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/transport-type/"
+  on: [RECV, HASH, DELIVER, LOG]
+  get: STRING
+
 client.bot.name:
   reference: "https://developer.fastly.com/reference/vcl/variables/client-request/client-bot-name/"
   on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
diff --git a/context/predefined.go b/context/predefined.go
index 503aaa0..5a80a21 100644
--- a/context/predefined.go
+++ b/context/predefined.go
@@ -1836,6 +1836,16 @@ func predefinedVariables() Variables {
 						Reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/fastly-info-is-h2/",
 					},
 				},
+				"is_h3": &Object{
+					Items: map[string]*Object{},
+					Value: &Accessor{
+						Get:       types.BoolType,
+						Set:       types.NeverType,
+						Unset:     false,
+						Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+						Reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/fastly-info-is-h3/",
+					},
+				},
 				"state": &Object{
 					Items: map[string]*Object{},
 					Value: &Accessor{
@@ -4041,6 +4051,16 @@ func predefinedVariables() Variables {
 								Reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/tls-client-iana-chosen-cipher-id/",
 							},
 						},
+						"ja3_md5": &Object{
+							Items: map[string]*Object{},
+							Value: &Accessor{
+								Get:       types.StringType,
+								Set:       types.NeverType,
+								Unset:     false,
+								Scopes:    RECV | HASH | DELIVER | LOG,
+								Reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/tls-client-ja3-md5/",
+							},
+						},
 						"protocol": &Object{
 							Items: map[string]*Object{},
 							Value: &Accessor{
@@ -4105,6 +4125,30 @@ func predefinedVariables() Variables {
 				},
 			},
 		},
+		"transport": &Object{
+			Items: map[string]*Object{
+				"bw_estimate": &Object{
+					Items: map[string]*Object{},
+					Value: &Accessor{
+						Get:       types.IntegerType,
+						Set:       types.NeverType,
+						Unset:     false,
+						Scopes:    RECV | HASH | DELIVER | LOG,
+						Reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/transport-bw-estimate/",
+					},
+				},
+				"type": &Object{
+					Items: map[string]*Object{},
+					Value: &Accessor{
+						Get:       types.StringType,
+						Set:       types.NeverType,
+						Unset:     false,
+						Scopes:    RECV | HASH | DELIVER | LOG,
+						Reference: "https://developer.fastly.com/reference/vcl/variables/client-connection/transport-type/",
+					},
+				},
+			},
+		},
 		"waf": &Object{
 			Items: map[string]*Object{
 				"anomaly_score": &Object{
diff --git a/linter/helper.go b/linter/helper.go
index 651671d..ce80d48 100644
--- a/linter/helper.go
+++ b/linter/helper.go
@@ -437,3 +437,20 @@ func isProbeMakingTheBackendStartAsUnhealthy(prober ast.BackendProbeObject) erro
 
 	return nil
 }
+
+func isTypeLiteral(node ast.Node) bool {
+	switch node.(type) {
+	case *ast.IP:
+		return true
+	case *ast.Boolean:
+		return true
+	case *ast.Integer:
+		return true
+	case *ast.String:
+		return true
+	case *ast.Float:
+		return true
+	default:
+		return false
+	}
+}
diff --git a/linter/linter.go b/linter/linter.go
index 63276c9..df6acf9 100644
--- a/linter/linter.go
+++ b/linter/linter.go
@@ -1083,6 +1083,20 @@ func (l *Linter) lintErrorStatement(stmt *ast.ErrorStatement, ctx *context.Conte
 }
 
 func (l *Linter) lintLogStatement(stmt *ast.LogStatement, ctx *context.Context) types.Type {
+	if isTypeLiteral(stmt.Value) {
+		switch stmt.Value.(type) {
+		case *ast.String:
+			return types.NeverType
+		default:
+			l.Error(&LintError{
+				Severity: ERROR,
+				Token:    stmt.GetMeta().Token,
+				Message:  "Only string literals can be passed to log directly.",
+			})
+			return types.NeverType
+		}
+	}
+
 	l.lint(stmt.Value, ctx)
 	return types.NeverType
 }
diff --git a/parser/statement_parser.go b/parser/statement_parser.go
index 4c023a6..d1b7c2e 100644
--- a/parser/statement_parser.go
+++ b/parser/statement_parser.go
@@ -252,9 +252,19 @@ func (p *Parser) parseCallStatement() (*ast.CallStatement, error) {
 	}
 	stmt.Subroutine = p.parseIdent()
 
+	// Parse functions with ()
+	if p.peekTokenIs(token.LEFT_PAREN) {
+		p.nextToken() // point to the token to check if it is RIGHT_PAREN
+		if !p.peekTokenIs(token.RIGHT_PAREN) {
+			return nil, errors.WithStack(UnexpectedToken(p.curToken))
+		}
+		p.nextToken() // point to RIGHT_PAREN
+	}
+
 	if !p.peekTokenIs(token.SEMICOLON) {
 		return nil, errors.WithStack(MissingSemicolon(p.curToken))
 	}
+
 	stmt.Meta.Trailing = p.trailing()
 	p.nextToken() // point to SEMICOLON
 


Test patch:
diff --git a/linter/linter_test.go b/linter/linter_test.go
index 2e4e2a3..8e8550a 100644
--- a/linter/linter_test.go
+++ b/linter/linter_test.go
@@ -1,6 +1,7 @@
 package linter
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/ysugimoto/falco/context"
@@ -1895,7 +1896,7 @@ func TestLintGotoStatement(t *testing.T) {
 	sub some_function {
 		goto foo;
 	}
-	
+
 	sub another_function {
 		foo:
 	}
@@ -1921,7 +1922,7 @@ func TestLintFunctionStatement(t *testing.T) {
 	sub foo {
 		log "123";
 	}
-	
+
 	sub bar {
 		foo();
 	}
@@ -1936,7 +1937,7 @@ func TestLintFunctionStatement(t *testing.T) {
 		log "123";
 		return true;
 	}
-	
+
 	sub bar {
 		foo();
 	}
@@ -1945,3 +1946,55 @@ func TestLintFunctionStatement(t *testing.T) {
 		assertError(t, input)
 	})
 }
+
+func TestLintLogStatementr(t *testing.T) {
+
+	tests := []struct {
+		name        string
+		logStatment string
+		shouldError bool
+	}{
+		{
+			name:        "log variable",
+			logStatment: "log req.restarts;",
+		},
+		{
+			name:        "log string literal",
+			logStatment: "log \"foo\";",
+		},
+		{
+			name:        "log int",
+			logStatment: "log 42;",
+			shouldError: true,
+		},
+		{
+			name:        "log bool",
+			logStatment: "log true;",
+			shouldError: true,
+		},
+		// IP literal fails due to parsing error
+		// but it should also fail as a lint error
+		// as well.
+		{
+			name:        "log float",
+			logStatment: "log 0.1;",
+			shouldError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			input := fmt.Sprintf(`
+	sub foo {
+		%s
+	}`, tt.logStatment)
+			if tt.shouldError {
+				assertError(t, input)
+			} else {
+				assertNoError(t, input)
+			}
+
+		})
+	}
+
+}
diff --git a/parser/statement_parser_test.go b/parser/statement_parser_test.go
index 0584dd6..d44625b 100644
--- a/parser/statement_parser_test.go
+++ b/parser/statement_parser_test.go
@@ -860,39 +860,76 @@ sub vcl_recv {
 }
 
 func TestCallStatement(t *testing.T) {
-	input := `// Subroutine
-sub vcl_recv {
-	// Leading comment
-	call feature_mod_recv; // Trailing comment
-}`
-	expect := &ast.VCL{
-		Statements: []ast.Statement{
-			&ast.SubroutineDeclaration{
-				Meta: ast.New(T, 0, comments("// Subroutine")),
-				Name: &ast.Ident{
-					Meta:  ast.New(T, 0),
-					Value: "vcl_recv",
+	t.Run("without parentheses", func(t *testing.T) {
+		input := `// Subroutine
+		sub vcl_recv {
+			// Leading comment
+			call feature_mod_recv; // Trailing comment
+		}`
+		expect := &ast.VCL{
+			Statements: []ast.Statement{
+				&ast.SubroutineDeclaration{
+					Meta: ast.New(T, 0, comments("// Subroutine")),
+					Name: &ast.Ident{
+						Meta:  ast.New(T, 0),
+						Value: "vcl_recv",
+					},
+					Block: &ast.BlockStatement{
+						Meta: ast.New(T, 1),
+						Statements: []ast.Statement{
+							&ast.CallStatement{
+								Meta: ast.New(T, 1, comments("// Leading comment"), comments("// Trailing comment")),
+								Subroutine: &ast.Ident{
+									Meta:  ast.New(T, 1),
+									Value: "feature_mod_recv",
+								},
+							},
+						},
+					},
 				},
-				Block: &ast.BlockStatement{
-					Meta: ast.New(T, 1),
-					Statements: []ast.Statement{
-						&ast.CallStatement{
-							Meta: ast.New(T, 1, comments("// Leading comment"), comments("// Trailing comment")),
-							Subroutine: &ast.Ident{
-								Meta:  ast.New(T, 1),
-								Value: "feature_mod_recv",
+			},
+		}
+		vcl, err := New(lexer.NewFromString(input)).ParseVCL()
+		if err != nil {
+			t.Errorf("%+v", err)
+		}
+		assert(t, vcl, expect)
+	})
+	t.Run("with parentheses", func(t *testing.T) {
+		input := `// Subroutine
+		sub vcl_recv {
+			// Leading comment
+			call feature_mod_recv(); // Trailing comment
+		}`
+		expect := &ast.VCL{
+			Statements: []ast.Statement{
+				&ast.SubroutineDeclaration{
+					Meta: ast.New(T, 0, comments("// Subroutine")),
+					Name: &ast.Ident{
+						Meta:  ast.New(T, 0),
+						Value: "vcl_recv",
+					},
+					Block: &ast.BlockStatement{
+						Meta: ast.New(T, 1),
+						Statements: []ast.Statement{
+							&ast.CallStatement{
+								Meta: ast.New(T, 1, comments("// Leading comment"), comments("// Trailing comment")),
+								Subroutine: &ast.Ident{
+									Meta:  ast.New(T, 1),
+									Value: "feature_mod_recv",
+								},
 							},
 						},
 					},
 				},
 			},
-		},
-	}
-	vcl, err := New(lexer.NewFromString(input)).ParseVCL()
-	if err != nil {
-		t.Errorf("%+v", err)
-	}
-	assert(t, vcl, expect)
+		}
+		vcl, err := New(lexer.NewFromString(input)).ParseVCL()
+		if err != nil {
+			t.Errorf("%+v", err)
+		}
+		assert(t, vcl, expect)
+	})
 }
 
 func TestDeclareStatement(t *testing.T) {