Skip to content

Commit

Permalink
Add warning for users signing URLs without passing an explicit version.
Browse files Browse the repository at this point in the history
  • Loading branch information
tseaver committed Apr 1, 2019
1 parent 6c5cfc7 commit a384692
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 33 deletions.
13 changes: 11 additions & 2 deletions storage/google/cloud/storage/blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@
_READ_LESS_THAN_SIZE = (
"Size {:d} was specified but the file-like object only had " "{:d} bytes remaining."
)
_SIGNED_URL_V2_DEFAULT_MESSAGE = (
"You have generated a signed URL using the default v2 signing "
"implementation. In the future, this will default to v4. "
"You may experience breaking changes if you use longer than 7 day "
"expiration times with v4. To opt-in to the behavior specify version='v2'."
)

_DEFAULT_CHUNKSIZE = 104857600 # 1024 * 1024 B * 100 = 100 MB
_MAX_MULTIPART_SIZE = 8388608 # 8 MB
Expand Down Expand Up @@ -316,7 +322,7 @@ def generate_signed_url(
query_parameters=None,
client=None,
credentials=None,
version="v2",
version=None,
):
"""Generates a signed URL for this blob.
Expand Down Expand Up @@ -421,7 +427,10 @@ def generate_signed_url(
:returns: A signed URL you can use to access the resource
until expiration.
"""
if version not in ("v2", "v4"):
if version is None:
version = "v2"
warnings.warn(DeprecationWarning(_SIGNED_URL_V2_DEFAULT_MESSAGE))
elif version not in ("v2", "v4"):
raise ValueError("'version' must be either 'v2' or 'v4'")

resource = "/{bucket_name}/{quoted_name}".format(
Expand Down
8 changes: 6 additions & 2 deletions storage/google/cloud/storage/bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from google.cloud.storage._signing import generate_signed_url_v4
from google.cloud.storage.acl import BucketACL
from google.cloud.storage.acl import DefaultObjectACL
from google.cloud.storage.blob import _SIGNED_URL_V2_DEFAULT_MESSAGE
from google.cloud.storage.blob import Blob
from google.cloud.storage.notification import BucketNotification
from google.cloud.storage.notification import NONE_PAYLOAD_FORMAT
Expand Down Expand Up @@ -1983,7 +1984,7 @@ def generate_signed_url(
query_parameters=None,
client=None,
credentials=None,
version="v2",
version=None,
):
"""Generates a signed URL for this bucket.
Expand Down Expand Up @@ -2063,7 +2064,10 @@ def generate_signed_url(
:returns: A signed URL you can use to access the resource
until expiration.
"""
if version not in ("v2", "v4"):
if version is None:
version = "v2"
warnings.warn(DeprecationWarning(_SIGNED_URL_V2_DEFAULT_MESSAGE))
elif version not in ("v2", "v4"):
raise ValueError("'version' must be either 'v2' or 'v4'")

resource = "/{bucket_name}".format(bucket_name=self.name)
Expand Down
52 changes: 35 additions & 17 deletions storage/tests/unit/test_blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import os
import tempfile
import unittest
import warnings

import google.cloud.storage.blob
import mock
Expand Down Expand Up @@ -363,7 +364,7 @@ def test_generate_signed_url_w_invalid_version(self):

def _generate_signed_url_helper(
self,
version,
version=None,
blob_name="blob-name",
api_access_endpoint=None,
method="GET",
Expand Down Expand Up @@ -393,24 +394,38 @@ def _generate_signed_url_helper(
client = _Client(connection)
bucket = _Bucket(client)
blob = self._make_one(blob_name, bucket=bucket)
to_patch = "google.cloud.storage.blob.generate_signed_url_{}".format(version)

if version is None:
effective_version = "v2"
else:
effective_version = version

to_patch = "google.cloud.storage.blob.generate_signed_url_{}".format(
effective_version)

with mock.patch(to_patch) as signer:
signed_uri = blob.generate_signed_url(
expiration=expiration,
max_age=max_age,
api_access_endpoint=api_access_endpoint,
method=method,
credentials=credentials,
content_md5=content_md5,
content_type=content_type,
response_type=response_type,
response_disposition=response_disposition,
generation=generation,
headers=headers,
query_parameters=query_parameters,
version=version,
)
with warnings.catch_warnings(record=True) as warned:
signed_uri = blob.generate_signed_url(
expiration=expiration,
max_age=max_age,
api_access_endpoint=api_access_endpoint,
method=method,
credentials=credentials,
content_md5=content_md5,
content_type=content_type,
response_type=response_type,
response_disposition=response_disposition,
generation=generation,
headers=headers,
query_parameters=query_parameters,
version=version,
)

if version is None:
self.assertEqual(len(warned), 1)
self.assertIs(warned[0].category, DeprecationWarning)
else:
self.assertEqual(len(warned), 0)

self.assertEqual(signed_uri, signer.return_value)

Expand All @@ -437,6 +452,9 @@ def _generate_signed_url_helper(
}
signer.assert_called_once_with(expected_creds, **expected_kwargs)

def test_generate_signed_url_no_version_passed_warning(self):
self._generate_signed_url_helper()

def _generate_signed_url_v2_helper(self, **kw):
version = "v2"
self._generate_signed_url_helper(version, **kw)
Expand Down
42 changes: 30 additions & 12 deletions storage/tests/unit/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import datetime
import unittest
import warnings

import mock

Expand Down Expand Up @@ -2583,7 +2584,7 @@ def test_generate_signed_url_w_invalid_version(self):

def _generate_signed_url_helper(
self,
version,
version=None,
bucket_name="bucket-name",
api_access_endpoint=None,
method="GET",
Expand Down Expand Up @@ -2612,19 +2613,33 @@ def _generate_signed_url_helper(
connection = _Connection()
client = _Client(connection)
bucket = self._make_one(name=bucket_name, client=client)
to_patch = "google.cloud.storage.bucket.generate_signed_url_{}".format(version)

if version is None:
effective_version = "v2"
else:
effective_version = version

to_patch = "google.cloud.storage.bucket.generate_signed_url_{}".format(
effective_version)

with mock.patch(to_patch) as signer:
signed_uri = bucket.generate_signed_url(
expiration=expiration,
max_age=max_age,
api_access_endpoint=api_access_endpoint,
method=method,
credentials=credentials,
headers=headers,
query_parameters=query_parameters,
version=version,
)
with warnings.catch_warnings(record=True) as warned:
signed_uri = bucket.generate_signed_url(
expiration=expiration,
max_age=max_age,
api_access_endpoint=api_access_endpoint,
method=method,
credentials=credentials,
headers=headers,
query_parameters=query_parameters,
version=version,
)

if version is None:
self.assertEqual(len(warned), 1)
self.assertIs(warned[0].category, DeprecationWarning)
else:
self.assertEqual(len(warned), 0)

self.assertEqual(signed_uri, signer.return_value)

Expand All @@ -2646,6 +2661,9 @@ def _generate_signed_url_helper(
}
signer.assert_called_once_with(expected_creds, **expected_kwargs)

def test_generate_signed_url_no_version_passed_warning(self):
self._generate_signed_url_helper()

def _generate_signed_url_v2_helper(self, **kw):
version = "v2"
self._generate_signed_url_helper(version, **kw)
Expand Down

0 comments on commit a384692

Please sign in to comment.