fix: use URI.parse instead of URI.new! in normalize_url function to allow unescaped characters in the path

[fahchen]

This PR depends on wojtekmach/req#433.

[wojtekmach]

Thank you for the PR. I tested this by modifying a test:

diff --git a/test/req_s3_test.exs b/test/req_s3_test.exs
index 5cb6380..b55cdb9 100644
--- a/test/req_s3_test.exs
+++ b/test/req_s3_test.exs
@@ -210,7 +210,7 @@ defmodule ReqS3Test do

     options = [
       bucket: bucket,
-      key: "key1",
+      key: "foo bar",
       content_type: "text/plain"
     ]

@@ -228,7 +228,7 @@ defmodule ReqS3Test do

     %{status: 200, body: ^body, headers: %{"content-type" => ["text/plain"]}} =
       Req.get!(
-        "#{form.url}/key1",
+        "#{form.url}/foo%20bar",
         aws_sigv4: [
           service: :s3,
           access_key_id: @access_key_id,

and I still got a SignatureDoesNotMatch. Could you double-check?

You can run integration tests like this:

$ BUCKET_NAME= REQ_AWS_ACCESS_KEY_ID= REQ_AWS_SECRET_ACCESS_KEY= mix test.all

I used latest Req main.

[fahchen]

@wojtekmach Thank you for testing it, I also got the same error using Req.get!/2 with aws_sigv4 options, but ReqS3.presign_url/1 works as expected. I’ll dig deeper to resolve this issue.

url = ReqS3.presign_url(bucket: bucket, key: "foo bar")

%{
  status: 200,
  body: ^body,
  headers: %{"content-type" => ["text/plain"]}
} = Req.get!(url)

[fahchen]

@wojtekmach I have drafted a PR (wojtekmach/req#434) to address the issue.

The problem arises due to double encoding if the path is already encoded beforehand, as Req.Util will encode it again here:
https://github.com/wojtekmach/req/blob/2a802826b1f3e65bb13a5a1da037ce2d8734e619/lib/req/utils.ex#L74

For example, if we pass s3://bucket.s3.amazonaws.com/foo%20bar, the path gets encoded to /foo%2520bar.

[adriansalamon]

What is the status of this? I have the same issue, happy to help if any help is needed ☺️

[fahchen]

What is the status of this? I have the same issue, happy to help if any help is needed ☺️

@adriansalamon This issue has been fixed in Req and a release is already available. In the meantime, here’s a workaround for this PR: you can pass a URI struct instead of a string.

for example:

# Do this
ReqS3.presign_url([url: URI.parse("s3://wojtekmach-test/hello world.txt")])

# Not this
ReqS3.presign_url([url: "s3://wojtekmach-test/hello world.txt"])

@wojtekmach Should we provide a convenient way for users to pass a string instead of a URI struct? Do you have any suggestions? If it’s not necessary, I’ll go ahead and close this PR.