Skip to content

Commit

Permalink
Fix some python 2/3 compatibility issues
Browse files Browse the repository at this point in the history
Change-Id: Ib5b67ddcc761aae2731cd6c7d74ba4bd3ff59f5d
  • Loading branch information
Jon Wayne Parrott committed Mar 7, 2018
1 parent 8e3e8c7 commit 6c417f2
Showing 1 changed file with 16 additions and 13 deletions.
29 changes: 16 additions & 13 deletions cdn/snippets.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,8 @@
import hashlib
import hmac

# Python 2 & 3 compatibility
try:
from urllib.parse import urlsplit, parse_qs
except ImportError:
from urlparse import urlsplit, parse_qs
from six.moves import urllib


# [BEGIN sign_url]
def sign_url(url, key_name, key, expiration_time):
Expand All @@ -50,25 +47,32 @@ def sign_url(url, key_name, key, expiration_time):
"""
stripped_url = url.strip()
parsed_url = urlsplit(stripped_url)
query_params = parse_qs(parsed_url.query, keep_blank_values=True)
parsed_url = urllib.parse.urlsplit(stripped_url)
query_params = urllib.parse.parse_qs(
parsed_url.query, keep_blank_values=True)
epoch = datetime.datetime.utcfromtimestamp(0)
expiration_timestamp = int((expiration_time - epoch).total_seconds())
decoded_key = base64.urlsafe_b64decode(key)

url_to_sign = '{url}{separator}Expires={expires}&KeyName={key_name}'.format(
url_pattern = u'{url}{separator}Expires={expires}&KeyName={key_name}'

url_to_sign = url_pattern.format(
url=stripped_url,
separator='&' if query_params else '?',
expires=expiration_timestamp,
key_name=key_name)

signature = base64.urlsafe_b64encode(
hmac.new(decoded_key, url_to_sign, hashlib.sha1).digest())
digest = hmac.new(
decoded_key, url_to_sign.encode('utf-8'), hashlib.sha1).digest()
signature = base64.urlsafe_b64encode(digest)

return '{url}&Signature={signature}'.format(
signed_url = u'{url}&Signature={signature}'.format(
url=url_to_sign, signature=signature)

print(signed_url)

This comment has been minimized.

Copy link
@ahmetb

ahmetb Mar 7, 2018

Contributor

don't print here. return from here. users will likely want a str out of this, not a printed statement

This comment has been minimized.

Copy link
@theacodes

theacodes Mar 7, 2018

Contributor

That's against our general style guide for samples, in which samples should always print their output.

This comment has been minimized.

Copy link
@ahmetb

ahmetb Mar 7, 2018

Contributor

@jonparrott in that case I'd say we should create a print_signed_url method to print it.

the way we expect people to use this snippet is that they'll copy/paste sign_url method to their code and just use the returned str. It's unlikely that someone will actually print a sensitive info like this to the stdout in their applications.

This comment has been minimized.

Copy link
@theacodes

theacodes Mar 7, 2018

Contributor

That's not how we've decided as a team to do this. See go/cdpe-sample-rubric under the "Processes the result" setting. This sample would score "Needs work" if all it did was return the string.

This comment has been minimized.

Copy link
@ahmetb

ahmetb Mar 8, 2018

Contributor

at any rate I don't care that much, anyone probably can change it from a print to return fairly easily. but this was what I was talking about:

def sign_url(...)
    # algorithm

def print_signed_url(...)
    print sign_url(...)

This comment has been minimized.

Copy link
@theacodes

theacodes Mar 8, 2018

Contributor

That's too much indirection, which is also not great as per our rubic.

# [END sign_url]


if __name__ == '__main__':
parser = argparse.ArgumentParser(
description=__doc__,
Expand All @@ -95,5 +99,4 @@ def sign_url(url, key_name, key, expiration_time):
args = parser.parse_args()

if args.command == 'sign-url':
print(sign_url(args.url, args.key_name, args.key, args.expiration_time))

sign_url(args.url, args.key_name, args.key, args.expiration_time)

0 comments on commit 6c417f2

Please sign in to comment.