Skip to content

Commit

Permalink
Follow redirects for all HTTP methods
Browse files Browse the repository at this point in the history
Notes:
- edgurgel/httpoison#453 is manually worked around
- there is no redirect loop detection, it may make sense to add an
  option (with a default) to limit the max number of redirects
- POST requests which receive a 303 respose are followed internally by
  HTTPoison but with the GET method, thus if the server replies 303 to a
  POST request the tus upload will not work properly
  • Loading branch information
Gianluca Nitti committed Jan 26, 2022
1 parent e27114e commit 01f216e
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 11 deletions.
2 changes: 1 addition & 1 deletion lib/tus_client/head.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule TusClient.Head do
def request(url, headers \\ [], opts \\ []) do
url
|> HTTPoison.head(headers, Utils.httpoison_opts([], opts))
|> parse()
|> Utils.maybe_follow_redirect(&parse/1, &request(&1, headers, opts))
end

defp parse({:ok, %{status_code: status} = resp}) when status in [200, 204] do
Expand Down
2 changes: 1 addition & 1 deletion lib/tus_client/options.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule TusClient.Options do
def request(url, headers \\ [], opts \\ []) do
url
|> HTTPoison.options(headers, Utils.httpoison_opts([], opts))
|> parse()
|> Utils.maybe_follow_redirect(&parse/1, &request(&1, headers, opts))
end

defp parse({:ok, %{status_code: status} = resp}) when status in [200, 204] do
Expand Down
12 changes: 4 additions & 8 deletions lib/tus_client/patch.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,10 @@ defmodule TusClient.Patch do

url
|> HTTPoison.patch(data, hdrs, Utils.httpoison_opts([], opts))
|> case do
{:ok, %HTTPoison.MaybeRedirect{redirect_url: new_loc}}
when is_binary(new_loc) ->
do_request({:ok, data}, new_loc, offset, headers, opts)

response ->
parse(response)
end
|> Utils.maybe_follow_redirect(
&parse/1,
&do_request({:ok, data}, &1, offset, headers, opts)
)
end

defp do_request({:error, _} = err, _url, _offset, _headers, _opts), do: err
Expand Down
5 changes: 4 additions & 1 deletion lib/tus_client/post.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ defmodule TusClient.Post do

url
|> HTTPoison.post("", hdrs, Utils.httpoison_opts([], opts))
|> parse()
|> Utils.maybe_follow_redirect(
&parse/1,
&do_request({:ok, size}, &1, headers, opts)
)
end

defp do_request(res, _url, _headers, _opts) do
Expand Down
26 changes: 26 additions & 0 deletions lib/tus_client/utils.ex
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,32 @@ defmodule TusClient.Utils do
http_opts ++ [ssl: ssl_opts] ++ [follow_redirect: follow_redirect]
end

@doc false
def maybe_follow_redirect(
{:ok, %HTTPoison.MaybeRedirect{redirect_url: new_loc}},
_parse_fn,
rereq_fn
)
when is_binary(new_loc) do
rereq_fn.(new_loc)
end

# TODO remove this when https://github.com/edgurgel/httpoison/issues/453 is fixed
def maybe_follow_redirect(
{:ok, %HTTPoison.MaybeRedirect{headers: headers}} = resp,
parse_fn,
rereq_fn
) do
case get_header(headers, "location") do
new_loc when is_binary(new_loc) -> rereq_fn.(new_loc)
nil -> parse_fn.(resp)
end
end

def maybe_follow_redirect(response, parse_fn, _rereq_fn) do
parse_fn.(response)
end

defp get_header_impl(headers, header_name) do
headers
|> Enum.filter(fn
Expand Down
22 changes: 22 additions & 0 deletions test/tus_client/head_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,28 @@ defmodule TusClient.HeadTest do
assert {:error, :not_found} = Head.request(file_url(bypass.port))
end

test "request/1 302", %{bypass: bypass} do
Bypass.expect_once(bypass, "HEAD", "/files/foobar", fn conn ->
conn
|> put_resp_header(
"location",
"http://localhost:#{bypass.port}/somewhere/foobar"
)
|> resp(302, "")
end)

Bypass.expect_once(bypass, "HEAD", "/somewhere/foobar", fn conn ->
conn
|> put_resp_header("upload-offset", "0")
|> put_resp_header("upload-length", "1234")
|> put_resp_header("cache-control", "no-store")
|> resp(200, "")
end)

assert {:ok, %{upload_offset: 0, upload_length: 1234}} =
Head.request(file_url(bypass.port), [], follow_redirect: true)
end

defp file_url(port), do: endpoint_url(port) <> "/foobar"
defp endpoint_url(port), do: "http://localhost:#{port}/files"
end
21 changes: 21 additions & 0 deletions test/tus_client/options_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -83,5 +83,26 @@ defmodule TusClient.OptionsTest do
Options.request(endpoint_url(bypass.port))
end

test "request/1 307", %{bypass: bypass} do
Bypass.expect_once(bypass, "OPTIONS", "/files", fn conn ->
conn
|> put_resp_header("location", "http://localhost:#{bypass.port}/files2")
|> resp(307, "")
end)

Bypass.expect_once(bypass, "OPTIONS", "/files2", fn conn ->
conn
|> put_resp_header("tus-version", "1.0.0")
|> put_resp_header("tus-max-size", "1234")
|> put_resp_header("tus-extension", "creation,expiration")
|> resp(200, "")
end)

assert {:ok, %{max_size: 1234, extensions: ["creation", "expiration"]}} =
Options.request(endpoint_url(bypass.port), [],
follow_redirect: true
)
end

defp endpoint_url(port), do: "http://localhost:#{port}/files"
end
21 changes: 21 additions & 0 deletions test/tus_client/post_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,27 @@ defmodule TusClient.PostTest do
Post.request(endpoint_url(bypass.port), path, [{"foo", "bar"}])
end

test "request/1 301", %{bypass: bypass, tmp_file: path} do
Bypass.expect_once(bypass, "POST", "/files", fn conn ->
conn
|> put_resp_header("location", "http://localhost:#{bypass.port}/here")
|> resp(301, "<h1>go follow the location header</h1>")
end)

Bypass.expect_once(bypass, "POST", "/here", fn conn ->
conn
|> assert_upload_len()
|> assert_version()
|> put_resp_header("location", endpoint_url(bypass.port) <> "/foofile")
|> resp(201, "")
end)

assert {:ok, %{location: endpoint_url(bypass.port) <> "/foofile"}} ==
Post.request(endpoint_url(bypass.port), path, [],
follow_redirect: true
)
end

test "request/1 with metadata", %{bypass: bypass, tmp_file: path} do
Bypass.expect_once(bypass, "POST", "/files", fn conn ->
conn
Expand Down

0 comments on commit 01f216e

Please sign in to comment.