Skip to content

Commit 8ca327e

Browse files
author
Federico Fissore
committed
If a property/txt file contains an invalid line, fail gracefully
Signed-off-by: Federico Fissore <[email protected]>
1 parent a4df772 commit 8ca327e

12 files changed

+104
-40
lines changed

Diff for: ‎src/arduino.cc/builder/constants/constants.go

+2
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ const LIBRARY_SENTENCE = "sentence"
194194
const LIBRARY_URL = "url"
195195
const LIBRARY_VERSION = "version"
196196
const MSG_ARCH_FOLDER_NOT_SUPPORTED = "'arch' folder is no longer supported! See http://goo.gl/gfFJzU for more information"
197+
const MSG_WRONG_PROPERTIES_FILE = "Property line '{0}' in file {1} is invalid"
198+
const MSG_WRONG_PROPERTIES = "Property line '{0}' is invalid"
197199
const MSG_BOARD_UNKNOWN = "Board {0} (platform {1}, package {2}) is unknown"
198200
const MSG_BOOTLOADER_FILE_MISSING = "Bootloader file specified but missing: {0}"
199201
const MSG_BUILD_OPTIONS_CHANGED = "Build options changed, rebuilding all"

Diff for: ‎src/arduino.cc/builder/hardware_loader.go

+16-13
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ package builder
3131

3232
import (
3333
"arduino.cc/builder/constants"
34+
"arduino.cc/builder/i18n"
3435
"arduino.cc/builder/props"
3536
"arduino.cc/builder/types"
3637
"arduino.cc/builder/utils"
@@ -41,6 +42,8 @@ import (
4142
type HardwareLoader struct{}
4243

4344
func (s *HardwareLoader) Run(context map[string]interface{}) error {
45+
logger := context[constants.CTX_LOGGER].(i18n.Logger)
46+
4447
packages := &types.Packages{}
4548
packages.Packages = make(map[string]*types.Package)
4649
packages.Properties = make(map[string]string)
@@ -60,7 +63,7 @@ func (s *HardwareLoader) Run(context map[string]interface{}) error {
6063
return utils.Errorf(context, constants.MSG_MUST_BE_A_FOLDER, folder)
6164
}
6265

63-
hardwarePlatformTxt, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PLATFORM_TXT))
66+
hardwarePlatformTxt, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PLATFORM_TXT), logger)
6467
if err != nil {
6568
return utils.WrapError(err)
6669
}
@@ -81,7 +84,7 @@ func (s *HardwareLoader) Run(context map[string]interface{}) error {
8184
}
8285

8386
targetPackage := getOrCreatePackage(packages, packageId)
84-
err = loadPackage(targetPackage, subfolderPath)
87+
err = loadPackage(targetPackage, subfolderPath, logger)
8588
if err != nil {
8689
return utils.WrapError(err)
8790
}
@@ -107,8 +110,8 @@ func getOrCreatePackage(packages *types.Packages, packageId string) *types.Packa
107110
return &targetPackage
108111
}
109112

110-
func loadPackage(targetPackage *types.Package, folder string) error {
111-
packagePlatformTxt, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PLATFORM_TXT))
113+
func loadPackage(targetPackage *types.Package, folder string, logger i18n.Logger) error {
114+
packagePlatformTxt, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PLATFORM_TXT), logger)
112115
if err != nil {
113116
return utils.WrapError(err)
114117
}
@@ -139,7 +142,7 @@ func loadPackage(targetPackage *types.Package, folder string) error {
139142
}
140143

141144
platform := getOrCreatePlatform(platforms, platformId)
142-
err = loadPlatform(platform, targetPackage.PackageId, subfolderPath)
145+
err = loadPlatform(platform, targetPackage.PackageId, subfolderPath, logger)
143146
if err != nil {
144147
return utils.WrapError(err)
145148
}
@@ -163,7 +166,7 @@ func getOrCreatePlatform(platforms map[string]*types.Platform, platformId string
163166
return &targetPlatform
164167
}
165168

166-
func loadPlatform(targetPlatform *types.Platform, packageId string, folder string) error {
169+
func loadPlatform(targetPlatform *types.Platform, packageId string, folder string, logger i18n.Logger) error {
167170
_, err := os.Stat(filepath.Join(folder, constants.FILE_BOARDS_TXT))
168171
if err != nil && !os.IsNotExist(err) {
169172
return utils.WrapError(err)
@@ -175,26 +178,26 @@ func loadPlatform(targetPlatform *types.Platform, packageId string, folder strin
175178

176179
targetPlatform.Folder = folder
177180

178-
err = loadBoards(targetPlatform.Boards, packageId, targetPlatform.PlatformId, folder)
181+
err = loadBoards(targetPlatform.Boards, packageId, targetPlatform.PlatformId, folder, logger)
179182
if err != nil {
180183
return utils.WrapError(err)
181184
}
182185

183186
assignDefaultBoardToPlatform(targetPlatform)
184187

185-
platformTxt, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PLATFORM_TXT))
188+
platformTxt, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PLATFORM_TXT), logger)
186189
if err != nil {
187190
return utils.WrapError(err)
188191
}
189192

190-
localPlatformProperties, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PLATFORM_LOCAL_TXT))
193+
localPlatformProperties, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PLATFORM_LOCAL_TXT), logger)
191194
if err != nil {
192195
return utils.WrapError(err)
193196
}
194197

195198
targetPlatform.Properties = utils.MergeMapsOfStrings(make(map[string]string), targetPlatform.Properties, platformTxt, localPlatformProperties)
196199

197-
programmersProperties, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PROGRAMMERS_TXT))
200+
programmersProperties, err := props.SafeLoad(filepath.Join(folder, constants.FILE_PROGRAMMERS_TXT), logger)
198201
if err != nil {
199202
return utils.WrapError(err)
200203
}
@@ -213,13 +216,13 @@ func assignDefaultBoardToPlatform(targetPlatform *types.Platform) {
213216
}
214217
}
215218

216-
func loadBoards(boards map[string]*types.Board, packageId string, platformId string, folder string) error {
217-
properties, err := props.Load(filepath.Join(folder, constants.FILE_BOARDS_TXT))
219+
func loadBoards(boards map[string]*types.Board, packageId string, platformId string, folder string, logger i18n.Logger) error {
220+
properties, err := props.Load(filepath.Join(folder, constants.FILE_BOARDS_TXT), logger)
218221
if err != nil {
219222
return utils.WrapError(err)
220223
}
221224

222-
localProperties, err := props.SafeLoad(filepath.Join(folder, constants.FILE_BOARDS_LOCAL_TXT))
225+
localProperties, err := props.SafeLoad(filepath.Join(folder, constants.FILE_BOARDS_LOCAL_TXT), logger)
223226
if err != nil {
224227
return utils.WrapError(err)
225228
}

Diff for: ‎src/arduino.cc/builder/libraries_loader.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func makeLibrary(libraryFolder string, debugLevel int, logger i18n.Logger) (*typ
113113
}
114114

115115
func makeNewLibrary(libraryFolder string, debugLevel int, logger i18n.Logger) (*types.Library, error) {
116-
properties, err := props.Load(filepath.Join(libraryFolder, constants.LIBRARY_PROPERTIES))
116+
properties, err := props.Load(filepath.Join(libraryFolder, constants.LIBRARY_PROPERTIES), logger)
117117
if err != nil {
118118
return nil, utils.WrapError(err)
119119
}

Diff for: ‎src/arduino.cc/builder/platform_keys_rewrite_loader.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ package builder
3131

3232
import (
3333
"arduino.cc/builder/constants"
34+
"arduino.cc/builder/i18n"
3435
"arduino.cc/builder/props"
3536
"arduino.cc/builder/types"
3637
"arduino.cc/builder/utils"
@@ -43,6 +44,7 @@ import (
4344
type PlatformKeysRewriteLoader struct{}
4445

4546
func (s *PlatformKeysRewriteLoader) Run(context map[string]interface{}) error {
47+
logger := context[constants.CTX_LOGGER].(i18n.Logger)
4648
folders := context[constants.CTX_HARDWARE_FOLDERS].([]string)
4749

4850
platformKeysRewriteTxtPath, err := findPlatformKeysRewriteTxt(folders)
@@ -56,7 +58,7 @@ func (s *PlatformKeysRewriteLoader) Run(context map[string]interface{}) error {
5658
platformKeysRewrite := types.PlatforKeysRewrite{}
5759
platformKeysRewrite.Rewrites = []types.PlatforKeyRewrite{}
5860

59-
txt, err := props.Load(platformKeysRewriteTxtPath)
61+
txt, err := props.Load(platformKeysRewriteTxtPath, logger)
6062
keys := utils.KeysOfMapOfString(txt)
6163
sort.Strings(keys)
6264

Diff for: ‎src/arduino.cc/builder/props/properties.go

+21-8
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ package props
3131

3232
import (
3333
"arduino.cc/builder/constants"
34+
"arduino.cc/builder/i18n"
3435
"arduino.cc/builder/utils"
36+
"github.com/go-errors/errors"
3537
"io/ioutil"
3638
"os"
3739
"regexp"
@@ -54,7 +56,7 @@ func init() {
5456
}
5557
}
5658

57-
func Load(filepath string) (map[string]string, error) {
59+
func Load(filepath string, logger i18n.Logger) (map[string]string, error) {
5860
bytes, err := ioutil.ReadFile(filepath)
5961
if err != nil {
6062
return nil, utils.WrapError(err)
@@ -67,42 +69,53 @@ func Load(filepath string) (map[string]string, error) {
6769
properties := make(map[string]string)
6870

6971
for _, line := range strings.Split(text, "\n") {
70-
loadSingleLine(properties, line)
72+
err := loadSingleLine(properties, line)
73+
if err != nil {
74+
return nil, utils.ErrorfWithLogger(logger, constants.MSG_WRONG_PROPERTIES_FILE, line, filepath)
75+
}
7176
}
7277

7378
return properties, nil
7479
}
7580

76-
func LoadFromSlice(lines []string) map[string]string {
81+
func LoadFromSlice(lines []string, logger i18n.Logger) (map[string]string, error) {
7782
properties := make(map[string]string)
7883

7984
for _, line := range lines {
80-
loadSingleLine(properties, line)
85+
err := loadSingleLine(properties, line)
86+
if err != nil {
87+
return nil, utils.ErrorfWithLogger(logger, constants.MSG_WRONG_PROPERTIES, line)
88+
}
8189
}
8290

83-
return properties
91+
return properties, nil
8492
}
8593

86-
func loadSingleLine(properties map[string]string, line string) {
94+
func loadSingleLine(properties map[string]string, line string) error {
8795
line = strings.TrimSpace(line)
8896

8997
if len(line) > 0 && line[0] != '#' {
9098
lineParts := strings.SplitN(line, "=", 2)
99+
if len(lineParts) != 2 {
100+
return errors.New("")
101+
}
91102
key := strings.TrimSpace(lineParts[0])
92103
value := strings.TrimSpace(lineParts[1])
93104

94105
key = strings.Replace(key, "."+OSNAME, constants.EMPTY_STRING, 1)
95106
properties[key] = value
96107
}
108+
109+
return nil
97110
}
98111

99-
func SafeLoad(filepath string) (map[string]string, error) {
112+
func SafeLoad(filepath string, logger i18n.Logger) (map[string]string, error) {
100113
_, err := os.Stat(filepath)
101114
if os.IsNotExist(err) {
102115
return make(map[string]string), nil
103116
}
104117

105-
properties, err := Load(filepath)
118+
properties, err := Load(filepath, logger)
106119
if err != nil {
107120
return nil, utils.WrapError(err)
108121
}

Diff for: ‎src/arduino.cc/builder/set_custom_build_properties.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ package builder
3131

3232
import (
3333
"arduino.cc/builder/constants"
34+
"arduino.cc/builder/i18n"
3435
"arduino.cc/builder/props"
3536
"arduino.cc/builder/utils"
3637
)
@@ -42,8 +43,12 @@ func (s *SetCustomBuildProperties) Run(context map[string]interface{}) error {
4243
return nil
4344
}
4445

46+
logger := context[constants.CTX_LOGGER].(i18n.Logger)
4547
buildProperties := context[constants.CTX_BUILD_PROPERTIES].(map[string]string)
46-
customBuildProperties := props.LoadFromSlice(context[constants.CTX_CUSTOM_BUILD_PROPERTIES].([]string))
48+
customBuildProperties, err := props.LoadFromSlice(context[constants.CTX_CUSTOM_BUILD_PROPERTIES].([]string), logger)
49+
if err != nil {
50+
return utils.WrapError(err)
51+
}
4752

4853
for key, value := range customBuildProperties {
4954
buildProperties[key] = value

Diff for: ‎src/arduino.cc/builder/test/hardware_loader_test.go

+27-9
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,15 @@ func TestLoadHardware(t *testing.T) {
4444
context := make(map[string]interface{})
4545
context[constants.CTX_HARDWARE_FOLDERS] = []string{"downloaded_hardware", filepath.Join("..", "hardware"), "hardware"}
4646

47-
loader := builder.HardwareLoader{}
48-
err := loader.Run(context)
49-
NoError(t, err)
47+
commands := []types.Command{
48+
&builder.SetupHumanLoggerIfMissing{},
49+
&builder.HardwareLoader{},
50+
}
51+
52+
for _, command := range commands {
53+
err := command.Run(context)
54+
NoError(t, err)
55+
}
5056

5157
packages := context[constants.CTX_HARDWARE].(*types.Packages)
5258
require.Equal(t, 1, len(packages.Packages))
@@ -153,9 +159,15 @@ func TestLoadHardwareWithBoardManagerFolderStructure(t *testing.T) {
153159
context := make(map[string]interface{})
154160
context[constants.CTX_HARDWARE_FOLDERS] = []string{"downloaded_board_manager_stuff"}
155161

156-
loader := builder.HardwareLoader{}
157-
err := loader.Run(context)
158-
NoError(t, err)
162+
commands := []types.Command{
163+
&builder.SetupHumanLoggerIfMissing{},
164+
&builder.HardwareLoader{},
165+
}
166+
167+
for _, command := range commands {
168+
err := command.Run(context)
169+
NoError(t, err)
170+
}
159171

160172
packages := context[constants.CTX_HARDWARE].(*types.Packages)
161173
require.Equal(t, 3, len(packages.Packages))
@@ -196,9 +208,15 @@ func TestLoadLotsOfHardware(t *testing.T) {
196208

197209
context[constants.CTX_HARDWARE_FOLDERS] = []string{"downloaded_board_manager_stuff", "downloaded_hardware", filepath.Join("..", "hardware"), "hardware", "user_hardware"}
198210

199-
loader := builder.HardwareLoader{}
200-
err := loader.Run(context)
201-
NoError(t, err)
211+
commands := []types.Command{
212+
&builder.SetupHumanLoggerIfMissing{},
213+
&builder.HardwareLoader{},
214+
}
215+
216+
for _, command := range commands {
217+
err := command.Run(context)
218+
NoError(t, err)
219+
}
202220

203221
packages := context[constants.CTX_HARDWARE].(*types.Packages)
204222

Diff for: ‎src/arduino.cc/builder/test/helper_tools_downloader.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ package test
3232
import (
3333
"arduino.cc/builder/constants"
3434
"arduino.cc/builder/gohasissues"
35+
"arduino.cc/builder/i18n"
3536
"arduino.cc/builder/props"
3637
"arduino.cc/builder/utils"
3738
"encoding/json"
@@ -338,7 +339,7 @@ func coreAlreadyDownloadedAndUnpacked(targetPath string, core Core) (bool, error
338339
if os.IsNotExist(err) {
339340
return false, nil
340341
}
341-
platform, err := props.Load(filepath.Join(corePath, "platform.txt"))
342+
platform, err := props.Load(filepath.Join(corePath, "platform.txt"), i18n.HumanLogger{})
342343
if err != nil {
343344
return false, utils.WrapError(err)
344345
}

Diff for: ‎src/arduino.cc/builder/test/platform_keys_rewrite_loader_test.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,15 @@ func TestLoadPlatformKeysRewrite(t *testing.T) {
4242
context := make(map[string]interface{})
4343
context[constants.CTX_HARDWARE_FOLDERS] = []string{"downloaded_hardware", filepath.Join("..", "hardware"), "hardware"}
4444

45-
loader := builder.PlatformKeysRewriteLoader{}
46-
err := loader.Run(context)
47-
NoError(t, err)
45+
commands := []types.Command{
46+
&builder.SetupHumanLoggerIfMissing{},
47+
&builder.PlatformKeysRewriteLoader{},
48+
}
49+
50+
for _, command := range commands {
51+
err := command.Run(context)
52+
NoError(t, err)
53+
}
4854

4955
platformKeysRewrite := context[constants.CTX_PLATFORM_KEYS_REWRITE].(types.PlatforKeysRewrite)
5056

Diff for: ‎src/arduino.cc/builder/test/properties_test.go

+10-3
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ package test
3131

3232
import (
3333
"arduino.cc/builder/constants"
34+
"arduino.cc/builder/i18n"
3435
"arduino.cc/builder/props"
3536
"github.com/stretchr/testify/require"
3637
"path/filepath"
@@ -39,7 +40,7 @@ import (
3940
)
4041

4142
func TestPropertiesBoardsTxt(t *testing.T) {
42-
properties, err := props.Load(filepath.Join("props", "boards.txt"))
43+
properties, err := props.Load(filepath.Join("props", "boards.txt"), i18n.HumanLogger{})
4344

4445
NoError(t, err)
4546

@@ -52,7 +53,7 @@ func TestPropertiesBoardsTxt(t *testing.T) {
5253
}
5354

5455
func TestPropertiesTestTxt(t *testing.T) {
55-
properties, err := props.Load(filepath.Join("props", "test.txt"))
56+
properties, err := props.Load(filepath.Join("props", "test.txt"), i18n.HumanLogger{})
5657

5758
NoError(t, err)
5859

@@ -119,7 +120,7 @@ func TestDeleteUnexpandedPropsFromString2(t *testing.T) {
119120
}
120121

121122
func TestPropertiesRedBeearLabBoardsTxt(t *testing.T) {
122-
properties, err := props.Load(filepath.Join("props", "redbearlab_boards.txt"))
123+
properties, err := props.Load(filepath.Join("props", "redbearlab_boards.txt"), i18n.HumanLogger{})
123124

124125
NoError(t, err)
125126

@@ -131,3 +132,9 @@ func TestPropertiesRedBeearLabBoardsTxt(t *testing.T) {
131132
ethernet := props.SubTree(properties, "blend")
132133
require.Equal(t, "arduino:arduino", ethernet[constants.BUILD_PROPERTIES_BUILD_CORE])
133134
}
135+
136+
func TestPropertiesBroken(t *testing.T) {
137+
_, err := props.Load(filepath.Join("props", "broken.txt"), i18n.HumanLogger{})
138+
139+
require.Error(t, err)
140+
}

Diff for: ‎src/arduino.cc/builder/test/props/broken.txt

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
key=value
2+
brokenkey value

0 commit comments

Comments
 (0)