Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion v3/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/zmap/zlint/v3
go 1.24.0

require (
github.com/pelletier/go-toml v1.9.5
github.com/pelletier/go-toml/v2 v2.2.4
github.com/sirupsen/logrus v1.9.3
github.com/zmap/zcrypto v0.0.0-20251227215108-5ca1211d486b
golang.org/x/crypto v0.46.0
Expand Down
4 changes: 2 additions & 2 deletions v3/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/pelletier/go-toml v1.9.5 h1:4yBQzkHv+7BHq2PQUZF3Mx0IYxG7LsP222s7Agd3ve8=
github.com/pelletier/go-toml v1.9.5/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c=
github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4=
github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8=
Expand Down
43 changes: 36 additions & 7 deletions v3/lint/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ import (
"reflect"
"strings"

"github.com/pelletier/go-toml"
"github.com/pelletier/go-toml/v2"
)

// Configuration is a ZLint configuration which serves as a target
// to hold the full TOML tree that is a physical ZLint configuration./
type Configuration struct {
tree *toml.Tree
tree map[string]any
}

// MaybeConfigure is a thin wrapper over Configure.
Expand Down Expand Up @@ -93,11 +93,14 @@ func (c Configuration) Configure(lint interface{}, namespace string) error {
// The contents of the provided reader MUST be in a valid TOML format. The caller of this function
// is responsible for closing the reader, if appropriate.
func NewConfig(r io.Reader) (Configuration, error) {
tree, err := toml.LoadReader(r)
if err != nil {
var tree map[string]any
if err := toml.NewDecoder(r).Decode(&tree); err != nil {
return Configuration{}, err
}
return Configuration{tree}, nil
if tree == nil {
tree = map[string]any{}
}
return Configuration{tree: tree}, nil
}

// NewConfigFromFile attempts to instantiate a configuration from the provided filesystem path.
Expand Down Expand Up @@ -167,15 +170,41 @@ func NewEmptyConfig() Configuration {
// If there is no such namespace found in this configuration then provided the namespace specific data encoded
// within `target` is left unmodified. However, configuration of higher scoped fields will still be attempted.
func (c Configuration) deserializeConfigInto(target interface{}, namespace string) error {
if tree := c.tree.Get(namespace); tree != nil {
err := tree.(*toml.Tree).Unmarshal(target)
if tree := getNamespace(c.tree, namespace); tree != nil {
b, err := toml.Marshal(tree)
if err != nil {
return err
}
if err := toml.Unmarshal(b, target); err != nil {
return err
}
}
return c.resolveHigherScopedReferences(target)
}

func getNamespace(doc map[string]any, namespace string) any {
if doc == nil {
return nil
}
if namespace == "" {
return doc
}

cur := any(doc)
for part := range strings.SplitSeq(namespace, ".") {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I can see why it would think to do nonsense like this due to the word namespace.

So namespace is what this TOML library calls an object name.

[this_is_a_namespace]
flip_widget = true

[this_is_another_namespace]
fidget_threshold = 42

Since it's using a map as intermediary, what it really should be looking for are object keys that match the give namespace.

{
    "A": {},
    "B": {
        "C": {
            "some_config": true
         }
    }
}
got := getNamespace(doc, "C")

Should return...

{
     "some_config": true
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks! I recall now that I was not sure what the expectation was; the examples I could find were all a flat config with no nesting.

What's the expectation if there's nesting? Say;

[namespace]
C = hello

[namespace.sub1.sub2]
C = world

Would got := getNamespace(doc, "C") traverse the tree and find the first match (because that looks ambiguous); I gave it a quick try, and it looks like currently it would allow namespace to be namespace.sub1.sub2 to resolve a config.

Let me push a test for what I had in mind.

m, ok := cur.(map[string]any)
if !ok {
return nil
}
next, ok := m[part]
if !ok {
return nil
}
cur = next
}
return cur
}

// resolveHigherScopeReferences takes in an interface{} value and attempts to
// find any field within its inner value that is either a struct or a pointer
// to a struct that is one of our global configurable types. If such a field
Expand Down
89 changes: 70 additions & 19 deletions v3/lint/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,70 @@ import (
"sync"
"testing"

"github.com/pelletier/go-toml"
"github.com/pelletier/go-toml/v2"
)

func TestNestedConfigs(t *testing.T) {
c, err := NewConfigFromString(`
[Test]
A = "Test"
[Test.Sub1]
A = "Test.Sub1"
[Test.Sub1.Sub2]
A = "Test.Sub1.Sub2"
[Test2.Sub3.Sub4]
A = "Test2.Sub3.Sub4"
[Test3.ConfigA.ConfigB]
A = "Test3.ConfigA.ConfigB.A"
`)
if err != nil {
t.Fatal(err)
}
type Test struct {
A string
}
type Test2 struct {
ConfigA struct{ ConfigB Test }
}
t.Run("nested configs", func(t *testing.T) {
for _, ns := range []string{"Test", "Test.Sub1", "Test.Sub1.Sub2", "Test2.Sub3.Sub4"} {
test := Test{}
Comment on lines +27 to +51
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a commit with a tests that passes before and after this PR 😅 - perhaps you could have a peek to see if this test matches the intended behavior 🤗

err = c.Configure(&test, ns)
if err != nil {
t.Fatal(err)
}

if test.A != ns {
t.Fatalf("wanted %q got %q", ns, test.A)
}
}
})
t.Run("nested config from top", func(t *testing.T) {
got := Test2{}
err = c.Configure(&got, "Test3")
if err != nil {
t.Fatal(err)
}

want := Test2{}
want.ConfigA.ConfigB.A = "Test3.ConfigA.ConfigB.A"
if !reflect.DeepEqual(got, want) {
t.Fatalf("wanted %+v got %+v", want, got)
}
})
t.Run("nested config", func(t *testing.T) {
got := Test{}
err = c.Configure(&got, "Test3.ConfigA.ConfigB")
if err != nil {
t.Fatal(err)
}
want := "Test3.ConfigA.ConfigB.A"
if got.A != want {
t.Fatalf("wanted %q got %q", want, got)
}
})
}

func TestInt(t *testing.T) {
type Test struct {
A int
Expand Down Expand Up @@ -383,7 +444,7 @@ func TestSmokeExamplePrinting(t *testing.T) {
go func() {
defer wg.Done()
defer w.Close()
err = toml.NewEncoder(w).Indentation("").CompactComments(true).Encode(mapping)
err = toml.NewEncoder(w).SetIndentSymbol("").SetIndentTables(false).Encode(mapping)
}()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -930,8 +991,7 @@ func TestPrintConfiguration(t *testing.T) {
// I'm not a huge fan of this sort of test since it will have to be updated
// on the slightest change, but it's better than not have a test for printing
// out the configuration file.
want := `
[AppleRootStorePolicyConfig]
want := `[AppleRootStorePolicyConfig]

[CABFBaselineRequirementsConfig]
CrossSignedCa = false
Expand Down Expand Up @@ -1015,10 +1075,9 @@ func TestNewGlobalWithPrivateMembersDontGetPrinted(t *testing.T) {
// I'm not a huge fan of this sort of test since it will have to be updated
// on the slightest change, but it's better than not have a test for printing
// out the configuration file.
want := `
[this_is_a_test]
want := `[this_is_a_test]
A = 1
B = "2"
B = '2'
`
if got != want {
t.Fatalf("wanted '%s' but got '%s'", want, got)
Expand Down Expand Up @@ -1095,12 +1154,8 @@ func TestStripGlobalsFromStructWithPrivates(t *testing.T) {

func TestNewEmptyConfig(t *testing.T) {
c := NewEmptyConfig()
got, err := c.tree.Marshal()
if err != nil {
t.Fatal(err)
}
if got != nil {
t.Fatalf("wanted nil byte slice, got %s", string(got))
if len(c.tree) != 0 {
t.Fatalf("wanted empty config, got %#v", c.tree)
}
}

Expand Down Expand Up @@ -1172,12 +1227,8 @@ func TestEmptyConfigFromEmptyPath(t *testing.T) {
if err != nil {
t.Fatal(err)
}
got, err := c.tree.Marshal()
if err != nil {
t.Fatal(err)
}
if got != nil {
t.Fatalf("wanted nil byte slice, got %s", string(got))
if len(c.tree) != 0 {
t.Fatalf("wanted empty config, got %#v", c.tree)
}
}

Expand Down
4 changes: 2 additions & 2 deletions v3/lint/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"sort"
"strings"

"github.com/pelletier/go-toml"
"github.com/pelletier/go-toml/v2"
)

// FilterOptions is a struct used by Registry.Filter to create a sub registry
Expand Down Expand Up @@ -493,7 +493,7 @@ func (r *registryImpl) defaultConfiguration(globals []GlobalConfiguration) ([]by

}
w := &bytes.Buffer{}
err := toml.NewEncoder(w).Indentation("").CompactComments(true).Encode(configurables)
err := toml.NewEncoder(w).SetIndentSymbol("").SetIndentTables(false).Encode(configurables)
if err != nil {
return nil, err
}
Expand Down