Skip to content

Commit

Permalink
Quote 'key.name' properly when used in URL path / query string.
Browse files Browse the repository at this point in the history
Fixes #354.
  • Loading branch information
tseaver committed Nov 11, 2014
1 parent 95e80e1 commit 093c7d6
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 8 deletions.
19 changes: 11 additions & 8 deletions gcloud/storage/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import mimetypes
import os
from StringIO import StringIO
import urllib

from gcloud.storage._helpers import _PropertyMixin
from gcloud.storage._helpers import _scalar_property
Expand Down Expand Up @@ -117,7 +118,7 @@ def path(self):
elif not self.name:
raise ValueError('Cannot determine path without a key name.')

return self.bucket.path + '/o/' + self.name
return self.bucket.path + '/o/' + urllib.quote(self.name, safe='')

@property
def public_url(self):
Expand All @@ -126,11 +127,10 @@ def public_url(self):
:rtype: `string`
:returns: The public URL for this key.
"""
return '{storage_base_url}/{bucket_name}/{key_name}'.format(
return '{storage_base_url}/{bucket_name}/{quoted_name}'.format(
storage_base_url='http://commondatastorage.googleapis.com',
key_name=self.name,
bucket_name=self.bucket.name,
)
quoted_name=urllib.quote(self.name, safe=''))

def generate_signed_url(self, expiration, method='GET'):
"""Generates a signed URL for this key.
Expand All @@ -153,10 +153,9 @@ def generate_signed_url(self, expiration, method='GET'):
:returns: A signed URL you can use to access the resource
until expiration.
"""
resource = '/{bucket_name}/{key_name}'.format(
key_name=self.name,
resource = '/{bucket_name}/{quoted_name}'.format(
bucket_name=self.bucket.name,
)
quoted_name=urllib.quote(self.name, safe=''))
return self.connection.generate_signed_url(resource=resource,
expiration=expiration,
method=method)
Expand Down Expand Up @@ -285,9 +284,13 @@ def upload_from_file(self, file_obj, rewind=False, size=None,
'X-Upload-Content-Length': total_bytes,
}

query_params = {
'uploadType': 'resumable',
'name': urllib.quote_plus(self.name),
}
upload_url = self.connection.build_api_url(
path=self.bucket.path + '/o',
query_params={'uploadType': 'resumable', 'name': self.name},
query_params=query_params,
api_base_url=self.connection.API_BASE_URL + '/upload')

response, _ = self.connection.make_request(
Expand Down
75 changes: 75 additions & 0 deletions gcloud/storage/test_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ def test_path_normal(self):
key = self._makeOne(bucket, KEY)
self.assertEqual(key.path, '/b/name/o/%s' % KEY)

def test_path_w_slash_in_name(self):
KEY = 'parent/child'
connection = _Connection()
bucket = _Bucket(connection)
key = self._makeOne(bucket, KEY)
self.assertEqual(key.path, '/b/name/o/parent%2Fchild')

def test_public_url(self):
KEY = 'key'
connection = _Connection()
Expand All @@ -87,6 +94,15 @@ def test_public_url(self):
'http://commondatastorage.googleapis.com/name/%s' %
KEY)

def test_public_url_w_slash_in_name(self):
KEY = 'parent/child'
connection = _Connection()
bucket = _Bucket(connection)
key = self._makeOne(bucket, KEY)
self.assertEqual(
key.public_url,
'http://commondatastorage.googleapis.com/name/parent%2Fchild')

def test_generate_signed_url_w_default_method(self):
KEY = 'key'
EXPIRATION = '2014-10-16T20:34:37Z'
Expand All @@ -99,6 +115,19 @@ def test_generate_signed_url_w_default_method(self):
self.assertEqual(connection._signed,
[('/name/key', EXPIRATION, {'method': 'GET'})])

def test_generate_signed_url_w_slash_in_name(self):
KEY = 'parent/child'
EXPIRATION = '2014-10-16T20:34:37Z'
connection = _Connection()
bucket = _Bucket(connection)
key = self._makeOne(bucket, KEY)
self.assertEqual(key.generate_signed_url(EXPIRATION),
'http://example.com/abucket/akey?Signature=DEADBEEF'
'&Expiration=2014-10-16T20:34:37Z')
self.assertEqual(connection._signed,
[('/name/parent%2Fchild',
EXPIRATION, {'method': 'GET'})])

def test_generate_signed_url_w_explicit_method(self):
KEY = 'key'
EXPIRATION = '2014-10-16T20:34:37Z'
Expand Down Expand Up @@ -238,6 +267,52 @@ def test_upload_from_file(self):
self.assertEqual(rq[2]['data'], DATA[5:])
self.assertEqual(rq[2]['headers'], {'Content-Range': 'bytes 5-5/6'})

def test_upload_from_file_w_slash_in_name(self):
from tempfile import NamedTemporaryFile
from urlparse import parse_qsl
from urlparse import urlsplit
KEY = 'parent/child'
UPLOAD_URL = 'http://example.com/upload/name/parent%2Fchild'
DATA = 'ABCDEF'
loc_response = {'location': UPLOAD_URL}
chunk1_response = {}
chunk2_response = {}
connection = _Connection(
(loc_response, ''),
(chunk1_response, ''),
(chunk2_response, ''),
)
bucket = _Bucket(connection)
key = self._makeOne(bucket, KEY)
key.CHUNK_SIZE = 5
with NamedTemporaryFile() as fh:
fh.write(DATA)
fh.flush()
key.upload_from_file(fh, rewind=True)
rq = connection._requested
self.assertEqual(len(rq), 3)
self.assertEqual(rq[0]['method'], 'POST')
uri = rq[0]['url']
scheme, netloc, path, qs, _ = urlsplit(uri)
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'resumable', 'name': 'parent%2Fchild'})
self.assertEqual(rq[0]['headers'],
{'X-Upload-Content-Length': 6,
'X-Upload-Content-Type': 'application/unknown'})
self.assertEqual(rq[1]['method'], 'POST')
self.assertEqual(rq[1]['url'], UPLOAD_URL)
self.assertEqual(rq[1]['content_type'], 'text/plain')
self.assertEqual(rq[1]['data'], DATA[:5])
self.assertEqual(rq[1]['headers'], {'Content-Range': 'bytes 0-4/6'})
self.assertEqual(rq[2]['method'], 'POST')
self.assertEqual(rq[2]['url'], UPLOAD_URL)
self.assertEqual(rq[2]['content_type'], 'text/plain')
self.assertEqual(rq[2]['data'], DATA[5:])
self.assertEqual(rq[2]['headers'], {'Content-Range': 'bytes 5-5/6'})

def test_upload_from_filename(self):
from tempfile import NamedTemporaryFile
from urlparse import parse_qsl
Expand Down

0 comments on commit 093c7d6

Please sign in to comment.