Merge lp:~mattyw/juju-core/1199432 into lp:~go-bot/juju-core/trunk

[Matthew Williams]

Bug fix for lp:1199432

Currently an empty config supplied to charm.ReadConfig causes a panic, it would be better to return an error. lp:1199432

https://codereview.appspot.com/11542046/

Bug fix for lp:1199432

Currently an empty config supplied to charm.ReadConfig causes a panic, it would be better to return an error. lp:1199432

https://codereview.appspot.com/11542046/

[Matthew Williams (mattyw)]

Reviewers: mp+175856_code.launchpad.net,

Message:
Please take a look.

Description:
Bug fix for lp:1199432

Currently an empty config supplied to charm.ReadConfig causes a panic,
it would be better to return an error. lp:1199432

https://code.launchpad.net/~mattyw/juju-core/1199432/+merge/175856

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/11542046/

Affected files:
   A [revision details]
   M charm/config.go
   M charm/config_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20130719095358-4z29gui2k03po34w
+New revision: <email address hidden>

Index: charm/config.go
=== modified file 'charm/config.go'
--- charm/config.go 2013-07-09 10:32:23 +0000
+++ charm/config.go 2013-07-19 14:49:06 +0000
@@ -102,6 +102,9 @@
   if err := goyaml.Unmarshal(data, &config); err != nil {
    return nil, err
   }
+ if config == nil {
+ return nil, fmt.Errorf("invalid config: empty configuration")
+ }
   for name, option := range config.Options {
    switch option.Type {
    case "string", "int", "float", "boolean":

Index: charm/config_test.go
=== modified file 'charm/config_test.go'
--- charm/config_test.go 2013-07-09 10:32:23 +0000
+++ charm/config_test.go 2013-07-19 14:49:06 +0000
@@ -403,3 +403,11 @@
   assertTypeError("float", "123", "123")
   assertTypeError("int", "true", "true")
  }
+
+// When an empty config is supplied an error should be returned
+func (s *ConfigSuite) TestEmptyConfigReturnsError(c *C) {
+ config := ""
+ result, err := charm.ReadConfig(bytes.NewBuffer([]byte(config)))
+ c.Assert(result, IsNil)
+ c.Assert(err, ErrorMatches, "invalid config: empty configuration")
+}

[William Reade (fwereade)]

LGTM trivial, thanks.

https://codereview.appspot.com/11542046/

=== modified file 'charm/config.go'
--- charm/config.go	2013-07-09 10:32:23 +0000
+++ charm/config.go	2013-07-19 15:02:28 +0000
@@ -102,6 +102,9 @@
 	if err := goyaml.Unmarshal(data, &config); err != nil {
 		return nil, err
 	}
+	if config == nil {
+		return nil, fmt.Errorf("invalid config: empty configuration")
+	}
 	for name, option := range config.Options {
 		switch option.Type {
 		case "string", "int", "float", "boolean":

=== modified file 'charm/config_test.go'
--- charm/config_test.go	2013-07-09 10:32:23 +0000
+++ charm/config_test.go	2013-07-19 15:02:28 +0000
@@ -403,3 +403,11 @@
 	assertTypeError("float", "123", "123")
 	assertTypeError("int", "true", "true")
 }
+
+// When an empty config is supplied an error should be returned
+func (s *ConfigSuite) TestEmptyConfigReturnsError(c *C) {
+	config := ""
+	result, err := charm.ReadConfig(bytes.NewBuffer([]byte(config)))
+	c.Assert(result, IsNil)
+	c.Assert(err, ErrorMatches, "invalid config: empty configuration")
+}