Skip to content

Commit

Permalink
fix presigned_url generation issue (#954)
Browse files Browse the repository at this point in the history
update tests as well and also update
select object example with new API
changes.
  • Loading branch information
harshavardhana authored Aug 10, 2020
1 parent ea6712b commit d4f0bde
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 49 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pythonpackage-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,5 @@ jobs:
chmod +x /tmp/minio
mkdir -p /tmp/minio-config/certs/
cp tests/certs/* /tmp/minio-config/certs/
/tmp/minio -C /tmp/minio-config server /tmp/fs/ &
/tmp/minio -C /tmp/minio-config server /tmp/fs{1...4} &
python tests/functional/tests.py
2 changes: 1 addition & 1 deletion .github/workflows/pythonpackage-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ jobs:
New-Item -ItemType Directory -Path "$env:temp/minio-config/certs/"
Copy-Item -Path tests\certs\* -Destination "$env:temp/minio-config/certs/"
Invoke-WebRequest -Uri https://dl.minio.io/server/minio/release/windows-amd64/minio.exe -OutFile $HOME/minio.exe
Start-Process -NoNewWindow -FilePath "$HOME/minio.exe" -ArgumentList "-C", "$env:temp/minio-config", "server", "$env:temp/fs"
Start-Process -NoNewWindow -FilePath "$HOME/minio.exe" -ArgumentList "-C", "$env:temp/minio-config", "server", "$env:temp/fs{1...4}"
$env:SSL_CERT_FILE = "$env:temp/minio-config/certs/public.crt"
python tests/functional/tests.py
24 changes: 12 additions & 12 deletions docs/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -786,22 +786,22 @@ options = SelectObjectOptions(
expression=" select * from s3object",
input_serialization=InputSerialization(
compression_type="NONE",
csv=CSVInput(FileHeaderInfo="USE",
RecordDelimiter="\n",
FieldDelimiter=",",
QuoteCharacter='"',
QuoteEscapeCharacter='"',
Comments="#",
AllowQuotedRecordDelimiter="FALSE",
csv=CSVInput(file_header_info="USE",
record_delimiter="\n",
field_delimiter=",",
quote_character='"',
quote_escape_character='"',
comments="#",
allow_quoted_record_delimiter="FALSE",
),
),

output_serialization=OutputSerialization(
csv=CSVOutput(QuoteFields="ASNEEDED",
RecordDelimiter="\n",
FieldDelimiter=",",
QuoteCharacter='"',
QuoteEscapeCharacter='"',)
csv=CSVOutput(quote_fields="ASNEEDED",
record_delimiter="\n",
field_delimiter=",",
quote_character='"',
quote_escape_character='"',)
),
request_progress=RequestProgress(
enabled="FALSE"
Expand Down
28 changes: 14 additions & 14 deletions examples/select_object_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,28 @@
input_serialization=InputSerialization(
compression_type="NONE",
csv=CSVInput(
FileHeaderInfo="USE",
RecordDelimiter="\n",
FieldDelimiter=",",
QuoteCharacter='"',
QuoteEscapeCharacter='"',
Comments="#",
AllowQuotedRecordDelimiter="FALSE",
file_header_info="USE",
record_delimiter="\n",
field_delimiter=",",
quote_character='"',
quote_escape_character='"',
comments="#",
allow_quoted_record_delimiter="FALSE",
),
# If input is JSON
# json=JSONInput(Type="DOCUMENT")
# json=JSONInput(json_type="DOCUMENT")
),

output_serialization=OutputSerialization(
csv=CSVOutput(
QuoteFields="ASNEEDED",
RecordDelimiter="\n",
FieldDelimiter=",",
QuoteCharacter='"',
QuoteEscapeCharacter='"',
quote_fields="ASNEEDED",
record_delimiter="\n",
field_delimiter=",",
quote_character='"',
quote_escape_character='"',
),

# json = JSONOutput(RecordDelimiter="\n")
# json = JSONOutput(record_delimiter="\n")
),
request_progress=RequestProgress(
enabled="False"
Expand Down
8 changes: 5 additions & 3 deletions minio/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,21 +84,23 @@


# Note earlier versions of minio.compat exposed urllib.quote as urlencode
def quote(resource):
def quote(resource, safe='/', encoding=None, errors=None):
"""
This implementation of urllib.quote supports all unicode characters
:param: resource: Resource value to be url encoded.
"""
return _quote(
resource.encode('utf-8') if isinstance(resource, str) else resource,
safe=safe, encoding=encoding,
errors=errors,
)


def queryencode(query):
def queryencode(query, safe='/', encoding=None, errors=None):
"""
This implementation of queryencode supports all unicode characters
:param: query: Query value to be url encoded.
"""
return quote(query).replace('/', '%2F')
return quote(query, safe, encoding, errors).replace('/', '%2F')
30 changes: 12 additions & 18 deletions minio/signer.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import hmac
from datetime import datetime

from .compat import queryencode, urlsplit
from .compat import parse_qs, queryencode, urlencode, urlsplit
from .error import InvalidArgumentError
from .fold_case_dict import FoldCaseDict
from .helpers import get_sha256_hexdigest
Expand Down Expand Up @@ -126,38 +126,32 @@ def presign_v4(method, url, credentials,
if response_headers:
query.update(response_headers)

provided_query = parse_qs(parsed_url.query)
query.update(provided_query)

# URL components.
url_components = [parsed_url.geturl()]
ordered_query = collections.OrderedDict(sorted(query.items()))
query_components = []
for component_key in ordered_query:
single_component = [component_key, '=']
if ordered_query[component_key]:
single_component.append(queryencode(ordered_query[component_key]))
query_components.append(''.join(single_component))

query_string = '&'.join(query_components)
if query_string:
url_components.append('?')
url_components.append(query_string)
new_url = ''.join(url_components)
# new url constructor block ends.
new_parsed_url = urlsplit(new_url)
ordered_query.update(query)

query_str = urlencode(ordered_query, doseq=True, quote_via=queryencode)
new_url = parsed_url.scheme + '://' + parsed_url.netloc + \
parsed_url.path + '?' + query_str
new_parsed_url = urlsplit(new_url)
canonical_request = generate_canonical_request(method,
new_parsed_url,
headers_to_sign,
signed_headers,
content_hash_hex)
print(canonical_request)
string_to_sign = generate_string_to_sign(request_date, region,
canonical_request,
_PRESIGNED_SERVICE_NAME)
signing_key = generate_signing_key(request_date, region,
credentials.get().secret_key)
signature = hmac.new(signing_key, string_to_sign.encode('utf-8'),
hashlib.sha256).hexdigest()
new_parsed_url = urlsplit(new_url + "&X-Amz-Signature="+signature)
return new_parsed_url.geturl()

return new_parsed_url.geturl()+"&X-Amz-Signature="+signature


def get_signed_headers(headers):
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/minio_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ def test_get_target_url_works(self):
'objectName',
'us-west-2', None),
'https://bucket-name.s3-us-west-2.amazonaws.com/objectName')
eq_(get_target_url('http://localhost:9000',
'bucket-name',
'objectName',
'us-east-1',
{'versionId': 'uuid'}),
'http://localhost:9000/bucket-name/objectName?versionId=uuid')

@raises(TypeError)
def test_minio_requires_string(self):
Expand Down
9 changes: 9 additions & 0 deletions tests/unit/sign_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,15 @@ def test_presigned_invalid_expires(self):
presign_v4('GET', 'http://localhost:9000/hello',
credentials, region=None, headers={}, expires=0)

def test_presigned_versioned_id(self):
credentials = Credentials(
provider=Static("minio", "minio123"),
)
url = presign_v4('GET', 'http://localhost:9000/bucket-name/objectName?versionId=uuid',
credentials, region='us-east-1', headers={}, request_date=dt)

eq_(url, 'http://localhost:9000/bucket-name/objectName?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=minio%2F20150620%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20150620T010203Z&X-Amz-Expires=604800&X-Amz-SignedHeaders=host&versionId=uuid&X-Amz-Signature=3ce13e2ca929fafa20581a05730e4e9435f2a5e20ec7c5a082d175692fb0a663')


class SignV4Test(TestCase):
def test_signv4(self):
Expand Down

0 comments on commit d4f0bde

Please sign in to comment.