Skip to content

Commit

Permalink
dockerfile/linter: check for nil linter in linter functions
Browse files Browse the repository at this point in the history
During parsing, the linter can be nil and linter rules won't succeed at
running due to a nil dereference. Instead of checking for nil
everywhere, this modifies the linter to automatically disable itself if
the linter is nil.

Fixes a nil dereference panic happening when parsing `ENV` and `LABEL`
commands without a linter introduced by
6cfa459.

Co-authored-by: Paweł Gronowski <[email protected]>
Signed-off-by: Jonathan A. Sternberg <[email protected]>
  • Loading branch information
vvoland authored and jsternberg committed Jun 5, 2024
1 parent f9b730c commit 89043ad
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 9 deletions.
12 changes: 5 additions & 7 deletions frontend/dockerfile/instructions/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,8 @@ func ParseInstructionWithLinter(node *parser.Node, lint *linter.Linter) (v inter
case command.Env:
return parseEnv(req, lint)
case command.Maintainer:
if lint != nil {
msg := linter.RuleMaintainerDeprecated.Format()
lint.Run(&linter.RuleMaintainerDeprecated, node.Location(), msg)
}
msg := linter.RuleMaintainerDeprecated.Format()
lint.Run(&linter.RuleMaintainerDeprecated, node.Location(), msg)
return parseMaintainer(req)
case command.Label:
return parseLabel(req, lint)
Expand All @@ -92,11 +90,11 @@ func ParseInstructionWithLinter(node *parser.Node, lint *linter.Linter) (v inter
case command.Copy:
return parseCopy(req)
case command.From:
if lint != nil && !isLowerCaseStageName(req.args) {
if !isLowerCaseStageName(req.args) {
msg := linter.RuleStageNameCasing.Format(req.args[2])
lint.Run(&linter.RuleStageNameCasing, node.Location(), msg)
}
if lint != nil && !doesFromCaseMatchAsCase(req) {
if !doesFromCaseMatchAsCase(req) {
msg := linter.RuleFromAsCasing.Format(req.command, req.args[1])
lint.Run(&linter.RuleFromAsCasing, node.Location(), msg)
}
Expand Down Expand Up @@ -209,7 +207,7 @@ func parseKvps(args []string, cmdName string, location []parser.Range, lint *lin
return nil, errBlankCommandNames(cmdName)
}
name, value, sep := args[j], args[j+1], args[j+2]
if sep == "" {
if lint != nil && sep == "" {
msg := linter.RuleLegacyKeyValueFormat.Format(cmdName)
lint.Run(&linter.RuleLegacyKeyValueFormat, location, msg)
}
Expand Down
24 changes: 24 additions & 0 deletions frontend/dockerfile/instructions/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,30 @@ func TestParseOptInterval(t *testing.T) {
require.NoError(t, err)
}

func TestNilLinter(t *testing.T) {
for cmd := range command.Commands {
cmd := cmd
t.Run(cmd, func(t *testing.T) {
t.Parallel()

for _, tc := range []string{
cmd + " foo=bar",
cmd + " a",
cmd + " a b",
cmd + " a b c",
cmd + " 0 0",
} {
t.Run(tc, func(t *testing.T) {
ast, err := parser.Parse(strings.NewReader("FROM busybox\n" + tc))
if err == nil {
_, _, _ = Parse(ast.AST, nil)
}
})
}
})
}
}

func TestCommentsDetection(t *testing.T) {
dt := `# foo sets foo
ARG foo=bar
Expand Down
4 changes: 2 additions & 2 deletions frontend/dockerfile/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func New(config *Config) *Linter {
}

func (lc *Linter) Run(rule LinterRuleI, location []parser.Range, txt ...string) {
if lc.Warn == nil || lc.SkipAll {
if lc == nil || lc.Warn == nil || lc.SkipAll {
return
}
rulename := rule.RuleName()
Expand All @@ -50,7 +50,7 @@ func (lc *Linter) Run(rule LinterRuleI, location []parser.Range, txt ...string)
}

func (lc *Linter) Error() error {
if !lc.ReturnAsError {
if lc == nil || !lc.ReturnAsError {
return nil
}
if len(lc.CalledRules) == 0 {
Expand Down

0 comments on commit 89043ad

Please sign in to comment.