Skip to content

Commit

Permalink
linter: @filter regex support for strings (#1247)
Browse files Browse the repository at this point in the history
  • Loading branch information
Hidanio authored Feb 19, 2025
1 parent 9ca19fc commit 7307e95
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 2 deletions.
19 changes: 19 additions & 0 deletions docs/dynamic_rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,25 @@ function forbiddenIdUsage() {

This rule will match usage if `$id` variable.

Also, originally designed to apply to variables by matching their names, the functionality has been extended so that if a string literal is captured under the given name, the regular expression is applied to its literal value.

When a string literal is captured, the regular expression is applied to the literal’s value.

For example:

```php
function insecureUrl() {
/**
* @warning Use secure URLs
* @filter $url ^http://
*/
callApi("http://example.com");
}
```

In this case, the rule will match because the captured string literal "http://example.com" matches the regular expression ^http://.


#### Underline location (`@location`)

For every warning that NoVerify finds, it underlines the location. However, for dynamic rules, the right place is not always underlined.
Expand Down
9 changes: 7 additions & 2 deletions src/linter/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -1953,8 +1953,13 @@ func (d *rootWalker) checkFilterSet(m *phpgrep.MatchData, sc *meta.Scope, filter
return false
}
if filter.Regexp != nil {
if vr, ok := nn.(*ir.SimpleVar); ok {
if !filter.Regexp.MatchString(vr.Name) {
switch v := nn.(type) {
case *ir.SimpleVar:
if !filter.Regexp.MatchString(v.Name) {
return false
}
case *ir.String:
if !filter.Regexp.MatchString(v.Value) {
return false
}
}
Expand Down
69 changes: 69 additions & 0 deletions src/tests/rules/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,75 @@ function type_type_check(string $animal_name, int $animal_id) {
test.RunRulesTest()
}

func TestFilterLiteralNoWarning(t *testing.T) {
rfile := `<?php
function literalEndpointSafe() {
/**
* @warning Literal endpoint must use HTTPS
* @filter $endpoint ^http://
*/
callApi($endpoint);
}
`
test := linttest.NewSuite(t)
test.RuleFile = rfile
test.AddFile(`<?php
function testLiteralSafe() {
callApi("https://secure.com");
}
`)

test.RunRulesTest()
}

func TestMultipleLiterals(t *testing.T) {
rfile := `<?php
function checkEndpoint() {
/**
* @warning Endpoint must use HTTPS
* @filter $endpoint ^http://
*/
callApi($endpoint);
}
`
test := linttest.NewSuite(t)
test.RuleFile = rfile
test.AddFile(`<?php
function testMultipleEndpoints() {
callApi("http://example.com");
callApi("https://secure.com");
callApi("http://another.com");
}
`)
test.Expect = []string{
"Endpoint must use HTTPS",
"Endpoint must use HTTPS",
}
test.RunRulesTest()
}

func TestFilterVariableNoWarning(t *testing.T) {
rfile := `<?php
function variableEndpointSafe() {
/**
* @warning Variable endpoint must be renamed
* @filter $endpoint ^endpoint$
*/
callApi($endpoint);
}
`
test := linttest.NewSuite(t)
test.RuleFile = rfile
test.AddFile(`<?php
function testVariableSafe() {
$safeEndpoint = "http://example.com";
callApi($safeEndpoint);
}
`)

test.RunRulesTest()
}

func TestRulePathExcludePositive(t *testing.T) {
rfile := `<?php
/**
Expand Down

0 comments on commit 7307e95

Please sign in to comment.