Repository: ysugimoto/falco
Pull request #67 (2022-05-11 15:04:44)

Respect Ratelimit variables in context
**What does this PR do?**
Right now Falco doesn't know about the penaltybox and ratecounter variables that are declared. 
The linter will throw the error "undefined variables" for them.
This PR aims to fix that.

Also this PR adds support for predefined variables that Fastly provides for the ratelimiter :)


Patch:
diff --git a/.gitignore b/.gitignore
index b75848c..14eb5f3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -16,3 +16,5 @@
 falco
 dist/*
 playground
+
+.vscode
diff --git a/__generator__/builtin.yml b/__generator__/builtin.yml
index 7016cee..8b2b4b6 100644
--- a/__generator__/builtin.yml
+++ b/__generator__/builtin.yml
@@ -1221,14 +1221,14 @@ ratelimit.check_rate:
   reference: "https://developer.fastly.com/reference/vcl/functions/rate-limiting/ratelimit-check-rate/"
   on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
   arguments:
-    - [STRING, ID, INTEGER, INTEGER, INTEGER, ID, TIME]
+    - [STRING, ID, INTEGER, INTEGER, INTEGER, ID, RTIME]
   return: BOOL
 
 ratelimit.check_rates:
   reference: "https://developer.fastly.com/reference/vcl/functions/rate-limiting/ratelimit-check-rates/"
   on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
   arguments:
-    - [STRING, ID, INTEGER, INTEGER, INTEGER, ID, INTEGER, INTEGER, INTEGER, ID, TIME]
+    - [STRING, ID, INTEGER, INTEGER, INTEGER, ID, INTEGER, INTEGER, INTEGER, ID, RTIME]
   return: BOOL
 
 ratelimit.penaltybox_add:
diff --git a/__generator__/predefined.yml b/__generator__/predefined.yml
index 6038398..02f912d 100644
--- a/__generator__/predefined.yml
+++ b/__generator__/predefined.yml
@@ -2093,3 +2093,48 @@ LF:
   reference: "https://developer.fastly.com/reference/vcl/types/string/"
   on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
   get: STRING
+
+ratecounter.%any%.bucket.10s:
+  reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-10s/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: INTEGER
+
+ratecounter.%any%.bucket.20s:
+  reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-20s/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: INTEGER
+
+ratecounter.%any%.bucket.30s:
+  reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-30s/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: INTEGER
+
+ratecounter.%any%.bucket.40s:
+  reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-40s/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: INTEGER
+
+ratecounter.%any%.bucket.50s:
+  reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-50s/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: INTEGER
+
+ratecounter.%any%.bucket.60s:
+  reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-60s/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: INTEGER
+
+ratecounter.%any%.rate.1s:
+  reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-rate-1s/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: FLOAT
+
+ratecounter.%any%.rate.10s:
+  reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-rate-10s/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: FLOAT
+
+ratecounter.%any%.rate.60s:
+  reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-rate-60s/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: FLOAT
diff --git a/context/builtin.go b/context/builtin.go
index 7c7d5c2..9b5f77a 100644
--- a/context/builtin.go
+++ b/context/builtin.go
@@ -1204,7 +1204,7 @@ func builtinFunctions() Functions {
 					Value: &BuiltinFunction{
 						Return: types.BoolType,
 						Arguments: [][]types.Type{
-							[]types.Type{types.StringType, types.IDType, types.IntegerType, types.IntegerType, types.IntegerType, types.IDType, types.TimeType},
+							[]types.Type{types.StringType, types.IDType, types.IntegerType, types.IntegerType, types.IntegerType, types.IDType, types.RTimeType},
 						},
 						Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
 						Reference: "https://developer.fastly.com/reference/vcl/functions/rate-limiting/ratelimit-check-rate/",
@@ -1215,7 +1215,7 @@ func builtinFunctions() Functions {
 					Value: &BuiltinFunction{
 						Return: types.BoolType,
 						Arguments: [][]types.Type{
-							[]types.Type{types.StringType, types.IDType, types.IntegerType, types.IntegerType, types.IntegerType, types.IDType, types.IntegerType, types.IntegerType, types.IntegerType, types.IDType, types.TimeType},
+							[]types.Type{types.StringType, types.IDType, types.IntegerType, types.IntegerType, types.IntegerType, types.IDType, types.IntegerType, types.IntegerType, types.IntegerType, types.IDType, types.RTimeType},
 						},
 						Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
 						Reference: "https://developer.fastly.com/reference/vcl/functions/rate-limiting/ratelimit-check-rates/",
diff --git a/context/context.go b/context/context.go
index 1c45cbe..dbd2352 100644
--- a/context/context.go
+++ b/context/context.go
@@ -185,6 +185,31 @@ func (c *Context) GetRegexGroupVariable(name string) (types.Type, error) {
 	return types.StringType, nil
 }
 
+// Get ratecounter variable
+func (c *Context) GetRatecounterVariable(name string) (types.Type, error) {
+	// Ratecounter variables have the shape: ratecounter.{Variable Name}.[bucket/rate].Time
+	nameComponents := strings.Split(name, ".")
+	if len(nameComponents) != 4 {
+		return types.NullType, fmt.Errorf(`undefined variable "%s"`, name)
+	}
+
+	ratecounterVariableName := nameComponents[1]
+	// Check first if this ratecounter is defined in the first place.
+	if _, ok := c.Ratecounters[ratecounterVariableName]; !ok {
+		return types.NullType, fmt.Errorf(`undefined variable "%s"`, name)
+	}
+
+	// nameComponents[0] should be "ratecounter"
+	// nameComponents[1] should be the variable name
+	// nameComponents[2] should be either "bucket" or "rate"
+	// nameComponents[3] should be the time (10s, 60s, etc)
+	if v, ok := c.Variables[nameComponents[0]].Items["%any%"].Items[nameComponents[2]].Items[nameComponents[3]]; ok {
+		return v.Value.Get, nil
+	}
+
+	return types.NullType, fmt.Errorf(`undefined variable "%s"`, name)
+}
+
 func (c *Context) AddAcl(name string, acl *types.Acl) error {
 	// check existence
 	if _, duplicated := c.Acls[name]; duplicated {
@@ -203,11 +228,6 @@ func (c *Context) AddBackend(name string, backend *types.Backend) error {
 
 	// Additionally, assign some backend name related predefined variable
 	c.Variables["backend"].Items[name] = dynamicBackend()
-	c.Variables["ratecounter"] = &Object{
-		Items: map[string]*Object{
-			name: dynamicRateCounter(),
-		},
-	}
 
 	return nil
 }
@@ -301,6 +321,12 @@ func (c *Context) Get(name string) (types.Type, error) {
 		return c.GetRegexGroupVariable(name)
 	}
 
+	// If program want to access to ratecounter variables like "ratecounter.{Name}.bucket.10s",
+	// proxy to dedicated getter.
+	if first == "ratecounter" {
+		return c.GetRatecounterVariable(name)
+	}
+
 	obj, ok := c.Variables[first]
 	if !ok {
 		return types.NullType, fmt.Errorf(`undefined variable "%s"`, name)
diff --git a/context/predefined.go b/context/predefined.go
index f14d9d1..503aaa0 100644
--- a/context/predefined.go
+++ b/context/predefined.go
@@ -2748,6 +2748,112 @@ func predefinedVariables() Variables {
 				},
 			},
 		},
+		"ratecounter": &Object{
+			Items: map[string]*Object{
+				"%any%": &Object{
+					Items: map[string]*Object{
+						"bucket": &Object{
+							Items: map[string]*Object{
+								"10s": &Object{
+									Items: map[string]*Object{},
+									Value: &Accessor{
+										Get:       types.IntegerType,
+										Set:       types.NeverType,
+										Unset:     false,
+										Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+										Reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-10s/",
+									},
+								},
+								"20s": &Object{
+									Items: map[string]*Object{},
+									Value: &Accessor{
+										Get:       types.IntegerType,
+										Set:       types.NeverType,
+										Unset:     false,
+										Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+										Reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-20s/",
+									},
+								},
+								"30s": &Object{
+									Items: map[string]*Object{},
+									Value: &Accessor{
+										Get:       types.IntegerType,
+										Set:       types.NeverType,
+										Unset:     false,
+										Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+										Reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-30s/",
+									},
+								},
+								"40s": &Object{
+									Items: map[string]*Object{},
+									Value: &Accessor{
+										Get:       types.IntegerType,
+										Set:       types.NeverType,
+										Unset:     false,
+										Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+										Reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-40s/",
+									},
+								},
+								"50s": &Object{
+									Items: map[string]*Object{},
+									Value: &Accessor{
+										Get:       types.IntegerType,
+										Set:       types.NeverType,
+										Unset:     false,
+										Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+										Reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-50s/",
+									},
+								},
+								"60s": &Object{
+									Items: map[string]*Object{},
+									Value: &Accessor{
+										Get:       types.IntegerType,
+										Set:       types.NeverType,
+										Unset:     false,
+										Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+										Reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-bucket-60s/",
+									},
+								},
+							},
+						},
+						"rate": &Object{
+							Items: map[string]*Object{
+								"10s": &Object{
+									Items: map[string]*Object{},
+									Value: &Accessor{
+										Get:       types.FloatType,
+										Set:       types.NeverType,
+										Unset:     false,
+										Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+										Reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-rate-10s/",
+									},
+								},
+								"1s": &Object{
+									Items: map[string]*Object{},
+									Value: &Accessor{
+										Get:       types.FloatType,
+										Set:       types.NeverType,
+										Unset:     false,
+										Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+										Reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-rate-1s/",
+									},
+								},
+								"60s": &Object{
+									Items: map[string]*Object{},
+									Value: &Accessor{
+										Get:       types.FloatType,
+										Set:       types.NeverType,
+										Unset:     false,
+										Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+										Reference: "https://developer.fastly.com/reference/vcl/variables/rate-limiting/ratecounter-rate-60s/",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 		"req": &Object{
 			Items: map[string]*Object{
 				"backend": &Object{
diff --git a/linter/linter.go b/linter/linter.go
index f7fcd0e..b323891 100644
--- a/linter/linter.go
+++ b/linter/linter.go
@@ -47,6 +47,8 @@ func (l *Linter) Lint(node ast.Node, ctx *context.Context, isMain bool) types.Ty
 		l.lintUnusedAcls(ctx)
 		l.lintUnusedBackends(ctx)
 		l.lintUnusedSubroutines(ctx)
+		l.lintUnusedPenaltyboxes(ctx)
+		l.lintUnusedRatecounters(ctx)
 	}
 
 	return types.NeverType
@@ -96,6 +98,24 @@ func (l *Linter) lintUnusedSubroutines(ctx *context.Context) {
 	}
 }
 
+func (l *Linter) lintUnusedPenaltyboxes(ctx *context.Context) {
+	for _, p := range ctx.Penaltyboxes {
+		if p.IsUsed {
+			continue
+		}
+		l.Error(UnusedDeclaration(p.Decl.GetMeta(), p.Decl.Name.Value, "penaltybox").Match(UNUSED_DECLARATION))
+	}
+}
+
+func (l *Linter) lintUnusedRatecounters(ctx *context.Context) {
+	for _, rc := range ctx.Ratecounters {
+		if rc.IsUsed {
+			continue
+		}
+		l.Error(UnusedDeclaration(rc.Decl.GetMeta(), rc.Decl.Name.Value, "ratecounter").Match(UNUSED_DECLARATION))
+	}
+}
+
 func (l *Linter) lintUnusedVariables(ctx *context.Context) {
 	v, ok := ctx.Variables["var"]
 	if !ok {
@@ -293,7 +313,7 @@ func (l *Linter) factoryRootStatements(vcl *ast.VCL, ctx *context.Context) []ast
 			}
 			statements = append(statements, stmt)
 		case *ast.PenaltyboxDeclaration:
-			if err := ctx.AddPenaltybox(t.Name.Value, &types.Penaltybox{Penaltybox: t}); err != nil {
+			if err := ctx.AddPenaltybox(t.Name.Value, &types.Penaltybox{Decl: t}); err != nil {
 				e := &LintError{
 					Severity: ERROR,
 					Token:    t.Name.GetMeta().Token,
@@ -303,7 +323,7 @@ func (l *Linter) factoryRootStatements(vcl *ast.VCL, ctx *context.Context) []ast
 			}
 			statements = append(statements, stmt)
 		case *ast.RatecounterDeclaration:
-			if err := ctx.AddRatecounter(t.Name.Value, &types.Ratecounter{Ratecounter: t}); err != nil {
+			if err := ctx.AddRatecounter(t.Name.Value, &types.Ratecounter{Decl: t}); err != nil {
 				e := &LintError{
 					Severity: ERROR,
 					Token:    t.Name.GetMeta().Token,
@@ -727,7 +747,7 @@ func (l *Linter) lintSetStatement(stmt *ast.SetStatement, ctx *context.Context)
 	// Fastly has various assignment operators and required correspond types for each operator
 	// https://developer.fastly.com/reference/vcl/operators/#assignment-operators
 	//
-	// Above document is not enough to explain for other types... actually more complex type coparison may occur.
+	// Above document is not enough to explain for other types... actually more complex type comparison may occur.
 	// We investigated type comparison and summarized.
 	// See: https://docs.google.com/spreadsheets/d/16xRPugw9ubKA1nXHIc5ysVZKokLLhysI-jAu3qbOFJ8/edit#gid=0
 	switch stmt.Operator.Operator {
@@ -1041,6 +1061,16 @@ func (l *Linter) lintIdent(exp *ast.Ident, ctx *context.Context) types.Type {
 			// mark table is used
 			t.IsUsed = true
 			return types.TableType
+		} else if p, ok := ctx.Penaltyboxes[exp.Value]; ok {
+			// mark penaltybox is used
+			p.IsUsed = true
+			// Fastly treats these variables as type IDs
+			return types.IDType
+		} else if rc, ok := ctx.Ratecounters[exp.Value]; ok {
+			// mark ratecounter is used
+			rc.IsUsed = true
+			// Fastly treats these variables as type IDs
+			return types.IDType
 		} else if _, ok := ctx.Identifiers[exp.Value]; ok {
 			return types.IDType
 		}
diff --git a/types/types.go b/types/types.go
index 3a958a1..5a9ad09 100644
--- a/types/types.go
+++ b/types/types.go
@@ -226,17 +226,19 @@ func (d *Director) Token() token.Token { return d.Decl.Token }
 func (d *Director) String() string     { return d.Decl.String() }
 
 type Penaltybox struct {
-	Penaltybox *ast.PenaltyboxDeclaration
+	Decl   *ast.PenaltyboxDeclaration
+	IsUsed bool // mark this penaltybox is called at least once
 }
 
 func (pb *Penaltybox) Type() Type         { return PenaltyboxType }
-func (pb *Penaltybox) Token() token.Token { return pb.Penaltybox.Token }
-func (pb *Penaltybox) String() string     { return pb.Penaltybox.String() }
+func (pb *Penaltybox) Token() token.Token { return pb.Decl.Token }
+func (pb *Penaltybox) String() string     { return pb.Decl.String() }
 
 type Ratecounter struct {
-	Ratecounter *ast.RatecounterDeclaration
+	Decl   *ast.RatecounterDeclaration
+	IsUsed bool // mark this ratecounter is called at least once
 }
 
 func (rc *Ratecounter) Type() Type         { return RatecounterType }
-func (rc *Ratecounter) Token() token.Token { return rc.Ratecounter.Token }
-func (rc *Ratecounter) String() string     { return rc.Ratecounter.String() }
+func (rc *Ratecounter) Token() token.Token { return rc.Decl.Token }
+func (rc *Ratecounter) String() string     { return rc.Decl.String() }


Test patch:
diff --git a/linter/linter_test.go b/linter/linter_test.go
index bc0a5b2..5d1a9e7 100644
--- a/linter/linter_test.go
+++ b/linter/linter_test.go
@@ -1541,6 +1541,45 @@ penaltybox ip_pb {}
 penaltybox ip_pb {
 	set var.bar = "baz";
 }
+`
+		assertError(t, input)
+	})
+
+	t.Run("penaltybox variable should be pass if it is defined", func(t *testing.T) {
+		input := `
+penaltybox ip_pb {}
+ratecounter counter_60 {}
+
+sub test_sub{
+	declare local var.ratelimit_exceeded BOOL;
+	set var.ratelimit_exceeded = ratelimit.check_rate(
+		digest.hash_sha256("123"),
+		counter_60,
+		1,
+		60,
+		135,
+		ip_pb,
+		2m);
+}
+`
+		assertNoError(t, input)
+	})
+
+	t.Run("penaltybox variable should be defined", func(t *testing.T) {
+		input := `
+ratecounter counter_60 {}
+
+sub test_sub{
+	declare local var.ratelimit_exceeded BOOL;
+	set var.ratelimit_exceeded = ratelimit.check_rate(
+		digest.hash_sha256("123"),
+		counter_60,
+		1,
+		60,
+		135,
+		ip_pb,
+		2m);
+}
 `
 		assertError(t, input)
 	})
@@ -1583,6 +1622,78 @@ ratecounter req_counter {}
 ratecounter req_counter {
 	set var.bar = "baz";
 }
+`
+		assertError(t, input)
+	})
+
+	t.Run("ratecounter variable should be pass if it is defined", func(t *testing.T) {
+		input := `
+penaltybox ip_pb {}
+ratecounter counter_60 {}
+
+sub test_sub{
+	declare local var.ratelimit_exceeded BOOL;
+	set var.ratelimit_exceeded = ratelimit.check_rate(
+		digest.hash_sha256("123"),
+		counter_60,
+		1,
+		60,
+		135,
+		ip_pb,
+		2m);
+}
+`
+		assertNoError(t, input)
+	})
+
+	t.Run("ratecounter variable should be defined", func(t *testing.T) {
+		input := `
+penaltybox ip_pb {}
+
+sub test_sub{
+	declare local var.ratelimit_exceeded BOOL;
+	set var.ratelimit_exceeded = ratelimit.check_rate(
+		digest.hash_sha256("123"),
+		counter_60,
+		1,
+		60,
+		135,
+		ip_pb,
+		2m);
+}
+`
+		assertError(t, input)
+	})
+
+	t.Run("ratecounter bucket variables should pass if the ratecounter is defined", func(t *testing.T) {
+		input := `
+ratecounter counter_60 {}
+
+sub test_sub{
+	set req.http.X-ERL:tls_bucket_10s = std.itoa(ratecounter.counter_60.bucket.10s);
+}
+`
+		assertNoError(t, input)
+	})
+
+	t.Run("ratecounter bucket variables should not pass if the ratecounter is not defined", func(t *testing.T) {
+		input := `
+ratecounter counter_60 {}
+
+sub test_sub{
+	set req.http.X-ERL:tls_rate_10s = std.itoa(ratecounter.counter.bucket.10s);
+}
+`
+		assertError(t, input)
+	})
+
+	t.Run("ratecounter bucket variables should exist", func(t *testing.T) {
+		input := `
+ratecounter counter_60 {}
+
+sub test_sub{
+	set req.http.X-ERL:tls_bucket_10s = std.itoa(ratecounter.counter_60.bucket.100s);
+}
 `
 		assertError(t, input)
 	})