diff --git a/pkg/blobserver/s3/s3.go b/pkg/blobserver/s3/s3.go index ac12588b2..a4672f28c 100644 --- a/pkg/blobserver/s3/s3.go +++ b/pkg/blobserver/s3/s3.go @@ -79,6 +79,9 @@ func newFromConfig(_ blobserver.Loader, config jsonconfig.Obj) (blobserver.Stora if err := config.Validate(); err != nil { return nil, err } + if !s3.IsValidBucket(sto.bucket) { + return nil, fmt.Errorf("Not a valid bucket name: %v", sto.bucket) + } if !skipStartupCheck { // TODO: skip this check if a file // ~/.camli/.configcheck/sha1-("IS GOOD: s3: sha1(access key + diff --git a/pkg/misc/amazon/s3/client.go b/pkg/misc/amazon/s3/client.go index 88f6121b9..80d08e900 100644 --- a/pkg/misc/amazon/s3/client.go +++ b/pkg/misc/amazon/s3/client.go @@ -57,6 +57,10 @@ func (c *Client) httpClient() *http.Client { return http.DefaultClient } +func (c *Client) blobURL(bucket, key string) string { + return fmt.Sprintf("https://%s/%s/%s", c.hostname(), bucket, key) +} + func newReq(url_ string) *http.Request { req, err := http.NewRequest("GET", url_, nil) if err != nil { @@ -94,8 +98,8 @@ func parseListAllMyBuckets(r io.Reader) ([]*Bucket, error) { } // Returns 0, os.ErrNotExist if not on S3, otherwise reterr is real. -func (c *Client) Stat(name, bucket string) (size int64, reterr error) { - req := newReq("https://" + bucket + "." + c.hostname() + "/" + name) +func (c *Client) Stat(key, bucket string) (size int64, reterr error) { + req := newReq(c.blobURL(bucket, key)) req.Method = "HEAD" c.Auth.SignRequest(req) res, err := c.httpClient().Do(req) @@ -111,11 +115,11 @@ func (c *Client) Stat(name, bucket string) (size int64, reterr error) { case http.StatusOK: return strconv.ParseInt(res.Header.Get("Content-Length"), 10, 64) } - return 0, fmt.Errorf("s3: Unexpected status code %d statting object %v", res.StatusCode, name) + return 0, fmt.Errorf("s3: Unexpected status code %d statting object %v", res.StatusCode, key) } -func (c *Client) PutObject(name, bucket string, md5 hash.Hash, size int64, body io.Reader) error { - req := newReq("https://" + bucket + "." + c.hostname() + "/" + name) +func (c *Client) PutObject(key, bucket string, md5 hash.Hash, size int64, body io.Reader) error { + req := newReq(c.blobURL(bucket, key)) req.Method = "PUT" req.ContentLength = size if md5 != nil { @@ -172,8 +176,8 @@ func (c *Client) ListBucket(bucket string, startAt string, maxKeys int) (items [ } var bres listBucketResults - url_ := fmt.Sprintf("https://%s.%s/?marker=%s&max-keys=%d", - bucket, c.hostname(), url.QueryEscape(marker), fetchN) + url_ := fmt.Sprintf("https://%s/%s/?marker=%s&max-keys=%d", + c.hostname(), bucket, url.QueryEscape(marker), fetchN) // Try the enumerate three times, since Amazon likes to close // https connections a lot, and Go sucks at dealing with it: @@ -233,8 +237,7 @@ func (c *Client) ListBucket(bucket string, startAt string, maxKeys int) (items [ } func (c *Client) Get(bucket, key string) (body io.ReadCloser, size int64, err error) { - url_ := fmt.Sprintf("https://%s.%s/%s", bucket, c.hostname(), key) - req := newReq(url_) + req := newReq(c.blobURL(bucket, key)) c.Auth.SignRequest(req) var res *http.Response res, err = c.httpClient().Do(req) @@ -258,8 +261,7 @@ func (c *Client) Get(bucket, key string) (body io.ReadCloser, size int64, err er } func (c *Client) Delete(bucket, key string) error { - url_ := fmt.Sprintf("https://%s.%s/%s", bucket, c.hostname(), key) - req := newReq(url_) + req := newReq(c.blobURL(bucket, key)) req.Method = "DELETE" c.Auth.SignRequest(req) res, err := c.httpClient().Do(req) @@ -275,3 +277,42 @@ func (c *Client) Delete(bucket, key string) error { } return fmt.Errorf("Amazon HTTP error on DELETE: %d", res.StatusCode) } + +// IsValid reports whether bucket is a valid bucket name, per Amazon's naming restrictions. +// +// See http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html +func IsValidBucket(bucket string) bool { + l := len(bucket) + if l < 3 || l > 63 { + return false + } + + valid := false + prev := byte('.') + for i := 0; i < len(bucket); i++ { + c := bucket[i] + switch { + default: + return false + case 'a' <= c && c <= 'z': + valid = true + case '0' <= c && c <= '9': + // Is allowed, but bucketname can't be just numbers. + // Therefore, don't set valid to true + case c == '-': + if prev == '.' { + return false + } + case c == '.': + if prev == '.' || prev == '-' { + return false + } + } + prev = c + } + + if prev == '-' || prev == '.' { + return false + } + return valid +} diff --git a/pkg/misc/amazon/s3/client_test.go b/pkg/misc/amazon/s3/client_test.go index a6194cdfe..4e517650f 100644 --- a/pkg/misc/amazon/s3/client_test.go +++ b/pkg/misc/amazon/s3/client_test.go @@ -69,3 +69,27 @@ func TestParseBuckets(t *testing.T) { dump(want) } } + +func TestValidBucketNames(t *testing.T) { + m := []struct { + in string + want bool + }{ + {"myawsbucket", true}, + {"my.aws.bucket", true}, + {"my-aws-bucket.1", true}, + {"my---bucket.1", true}, + {".myawsbucket", false}, + {"-myawsbucket", false}, + {"myawsbucket.", false}, + {"myawsbucket-", false}, + {"my..awsbucket", false}, + } + + for _, bt := range m { + got := IsValidBucket(bt.in) + if got != bt.want { + t.Errorf("func(%q) = %v; want %v", bt.in, got, bt.want) + } + } +}