particle-os/debian-forge-composer
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

store: Fix SourceConfig.RepoConfig() function call

Browse source 
The SourceConfig pointer may be a loop variable that gets reused. This
results in unexpected behavior when the value pointed to is overwritten
by the loop calling this function.

Includes a test to make sure this is fixed.

So, DO NOT point to unsafe variables. Make a new pointer using
common.ToPtr where it is passed by value and returns a pointer to that
new value.

NOTE: This is NOT caught by golangci-lint. There may be other places
where this happens, but I have gone through the potential looking code
in osbuild-composer and images and not found any (other than a couple
places already noted with G601 tags as not a problem).
This commit is contained in:
Brian C. Lane 2023-10-26 16:43:31 -07:00 • committed by Ondřej Budai
parent a65f17e73e
commit b786178077
2 changed files with 42 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

4
internal/store/store.go
View file

@ -624,9 +624,9 @@ func (s *SourceConfig) RepoConfig(name string) rpmmd.RepoConfig {
repo.Name = name
repo.IgnoreSSL = common.ToPtr(!s.CheckSSL)
repo.CheckGPG = &s.CheckGPG
repo.CheckGPG = common.ToPtr(s.CheckGPG)
repo.RHSM = s.RHSM
repo.CheckRepoGPG = &s.CheckRepoGPG
repo.CheckRepoGPG = common.ToPtr(s.CheckRepoGPG)
repo.GPGKeys = s.GPGKeys
var urls []string

40
internal/store/store_test.go
View file

@ -489,6 +489,46 @@ func (suite *storeTest) TestRepoConfigMirrorlist() {
suite.Equal(expectedRepo, actualRepo)
}
// Test multiple SourceConfigs with different CheckGPG and CheckRepoGPG settings
func (suite *storeTest) TestSourceConfigGPGKeysTrueFalse() {
// We only care about the GPG bools
sources := map[string]SourceConfig{
"source-with-true": {Name: "source-with-true", CheckGPG: true, CheckRepoGPG: true},
"source-with-false": {Name: "source-with-false", CheckGPG: false, CheckRepoGPG: false},
}
// source is reused inside the loop, which can result in unexpected changes in go < 1.22
// https://go.dev/blog/loopvar-preview
var repos []rpmmd.RepoConfig
for id, source := range sources {
repos = append(repos, source.RepoConfig(id))
}
// First repo should be true, second should be false
suite.True(*repos[0].CheckGPG)
suite.True(*repos[0].CheckRepoGPG)
suite.False(*repos[1].CheckGPG)
suite.False(*repos[1].CheckRepoGPG)
// We only care about the GPG bools
// Test with false then true
sources = map[string]SourceConfig{
"source-with-false": {Name: "source-with-false", CheckGPG: false, CheckRepoGPG: false},
"source-with-true": {Name: "source-with-true", CheckGPG: true, CheckRepoGPG: true},
}
repos = []rpmmd.RepoConfig{}
for id, source := range sources {
repos = append(repos, source.RepoConfig(id))
}
// First repo should be false, second should be true
suite.False(*repos[0].CheckGPG)
suite.False(*repos[0].CheckRepoGPG)
suite.True(*repos[1].CheckGPG)
suite.True(*repos[1].CheckRepoGPG)
}
func TestStore(t *testing.T) {
suite.Run(t, new(storeTest))
}