Merge lp:~axwalk/juju-core/sshstorage-quote-meta into lp:~go-bot/juju-core/trunk

Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 1937
Proposed branch: lp:~axwalk/juju-core/sshstorage-quote-meta
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 30 lines (+11/-1)
2 files modified
environs/sshstorage/storage.go (+1/-1)
environs/sshstorage/storage_test.go (+10/-0)
To merge this branch: bzr merge lp:~axwalk/juju-core/sshstorage-quote-meta
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+188505@code.launchpad.net

Commit message

environs/sshstorage: quote escapes in storage dir

https://codereview.appspot.com/14195043/

Description of the change

environs/sshstorage: quote escapes in storage dir

https://codereview.appspot.com/14195043/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+188505_code.launchpad.net,

Message:
Please take a look.

Description:
environs/sshstorage: quote escapes in storage dir

https://code.launchpad.net/~axwalk/juju-core/sshstorage-quote-meta/+merge/188505

(do not edit description out of merge proposal)

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

Affected files (+13, -1 lines):
   A [revision details]
   M environs/sshstorage/storage.go
   M environs/sshstorage/storage_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-20130930183118-qlwtuzv7j6d2iy73
+New revision: <email address hidden>

Index: environs/sshstorage/storage.go
=== modified file 'environs/sshstorage/storage.go'
--- environs/sshstorage/storage.go 2013-09-24 01:55:30 +0000
+++ environs/sshstorage/storage.go 2013-10-01 03:33:39 +0000
@@ -137,7 +137,7 @@
   command = fmt.Sprintf(
    "SHELL=/bin/bash flock %s %s -c %s",
    flockmode,
- s.remotepath,
+ utils.ShQuote(s.remotepath),
    utils.ShQuote(command),
   )
   var encoded string

Index: environs/sshstorage/storage_test.go
=== modified file 'environs/sshstorage/storage_test.go'
--- environs/sshstorage/storage_test.go 2013-09-23 03:07:31 +0000
+++ environs/sshstorage/storage_test.go 2013-10-01 03:33:39 +0000
@@ -341,3 +341,13 @@
   _, err := NewSSHStorage("example.com", storageDir,
filepath.Join(tmpdir, "subdir2"))
   c.Assert(err, gc.ErrorMatches, ".*install: cannot create
directory.*Permission denied.*")
  }
+
+func (s *storageSuite) TestPathCharacters(c *gc.C) {
+ storageDirBase := c.MkDir()
+ storageDir := filepath.Join(storageDirBase, "'")
+ tmpdir := filepath.Join(storageDirBase, `"`)
+ c.Assert(os.Mkdir(storageDir, 0755), gc.IsNil)
+ c.Assert(os.Mkdir(tmpdir, 0755), gc.IsNil)
+ _, err := NewSSHStorage("example.com", storageDir, tmpdir)
+ c.Assert(err, gc.IsNil)
+}

Revision history for this message
Tim Penhey (thumper) wrote :

LGTM - although why people put weird characters in their config stuns
me.

https://codereview.appspot.com/14195043/

Revision history for this message
Tim Penhey (thumper) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'environs/sshstorage/storage.go'
2--- environs/sshstorage/storage.go 2013-09-24 01:55:30 +0000
3+++ environs/sshstorage/storage.go 2013-10-01 03:45:33 +0000
4@@ -137,7 +137,7 @@
5 command = fmt.Sprintf(
6 "SHELL=/bin/bash flock %s %s -c %s",
7 flockmode,
8- s.remotepath,
9+ utils.ShQuote(s.remotepath),
10 utils.ShQuote(command),
11 )
12 var encoded string
13
14=== modified file 'environs/sshstorage/storage_test.go'
15--- environs/sshstorage/storage_test.go 2013-09-23 03:07:31 +0000
16+++ environs/sshstorage/storage_test.go 2013-10-01 03:45:33 +0000
17@@ -341,3 +341,13 @@
18 _, err := NewSSHStorage("example.com", storageDir, filepath.Join(tmpdir, "subdir2"))
19 c.Assert(err, gc.ErrorMatches, ".*install: cannot create directory.*Permission denied.*")
20 }
21+
22+func (s *storageSuite) TestPathCharacters(c *gc.C) {
23+ storageDirBase := c.MkDir()
24+ storageDir := filepath.Join(storageDirBase, "'")
25+ tmpdir := filepath.Join(storageDirBase, `"`)
26+ c.Assert(os.Mkdir(storageDir, 0755), gc.IsNil)
27+ c.Assert(os.Mkdir(tmpdir, 0755), gc.IsNil)
28+ _, err := NewSSHStorage("example.com", storageDir, tmpdir)
29+ c.Assert(err, gc.IsNil)
30+}

Subscribers

People subscribed via source and target branches

to status/vote changes: