Skip to content

Enable streaming upload in S3 : put_object#658

Open
oconnore wants to merge 2 commits into
JuliaCloud:masterfrom
oconnore:streaming-upload
Open

Enable streaming upload in S3 : put_object#658
oconnore wants to merge 2 commits into
JuliaCloud:masterfrom
oconnore:streaming-upload

Conversation

@oconnore

Copy link
Copy Markdown

There were two problems:

  1. Request did not allow an IO
  2. Signing did not permit an unsigned payload
    (https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html)

There were two problems:

1) Request did not allow an IO
2) Signing did not permit an unsigned payload
    (https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html)

@ararslan ararslan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the long silence here. Could you add some tests to ensure this works as intended?

Comment thread src/utilities/sign.jl
Comment on lines +67 to +72
if !haskey(request.headers, "x-amz-content-sha256")
content_hash = bytes2hex(digest(MD_SHA256, request.content))
request.headers["x-amz-content-sha256"] = content_hash
else
content_hash = "UNSIGNED-PAYLOAD"
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can write this a bit more concisely like so:

Suggested change
if !haskey(request.headers, "x-amz-content-sha256")
content_hash = bytes2hex(digest(MD_SHA256, request.content))
request.headers["x-amz-content-sha256"] = content_hash
else
content_hash = "UNSIGNED-PAYLOAD"
end
content_hash = get!(request.headers, "x-amz-content-sha256") do
bytes2hex(digest(MD_SHA256, request.content))
end

Comment thread src/utilities/request.jl
request.headers["x-amz-content-sha256"] = "UNSIGNED-PAYLOAD"
else
request.headers["Content-MD5"] = base64encode(
digest(MD_MD5, request.content))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repository uses the Blue style guide, which puts closing parentheses on the following line in cases like this

Suggested change
digest(MD_MD5, request.content))
digest(MD_MD5, request.content)
)

Supposedly automatic formatting suggestions are set up but that appears to not have occurred here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants