From f82e8347fdf3b596bf60ca271d2812146cc1883b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibaut=20Barr=C3=A8re?= Date: Mon, 19 Aug 2024 09:09:09 +0200 Subject: [PATCH] =?UTF-8?q?Correctif=20sur=20la=20disponibilit=C3=A9=20de?= =?UTF-8?q?=20la=20seule=20ressource=20h=C3=A9berg=C3=A9e=20sur=20Google?= =?UTF-8?q?=20Drive=20(#4127)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add a note about https://github.com/etalab/transport-site/pull/3464 * Implement hot-fix for Google Drive hosted content (#4122) * Fix credo warning * Mix format * Tests pour redirection http de GDrive --------- Co-authored-by: Frédéric Menou --- .../lib/transport/availability_checker.ex | 18 +++++++++++++++++- .../transport/availability_checker_test.exs | 8 ++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/apps/transport/lib/transport/availability_checker.ex b/apps/transport/lib/transport/availability_checker.ex index a4d9899883..3826c65fe7 100644 --- a/apps/transport/lib/transport/availability_checker.ex +++ b/apps/transport/lib/transport/availability_checker.ex @@ -41,6 +41,7 @@ defmodule Transport.AvailabilityChecker do # https://github.com/edgurgel/httpoison/issues/171#issuecomment-244029927 # https://github.com/etalab/transport-site/issues/3463 {:error, %HTTPoison.Error{reason: {:invalid_redirection, _}}} -> + # NOTE: this does not actually verifies that the target is available at the moment! true _ -> @@ -49,7 +50,22 @@ defmodule Transport.AvailabilityChecker do end def available?(format, url, false) when is_binary(url) do - case http_client().head(url, [], follow_redirect: true) do + options = [follow_redirect: true] + # Hot-fix for https://github.com/etalab/transport-site/issues/4122 + # Least intrusive fix, only pass `force_redirection` to `HTTPoison` if the param value is true, else + # do not specify it, to avoid side-effect. + # See: https://github.com/benoitc/hackney/blob/eca5fbb1ff2d84facefb2a633e00f6ca16e7ddfd/src/hackney_stream.erl#L173 + # Google Drive content (1 instance at time of writing) returns a 303, and by default `hackney` only allows + # POST method for this, but here HEAD/GET are supported and required. By using `force_redirection` in `hackney` + # options, this indicated `hackney` that the redirect should still occur. + options = + if URI.parse(url).host == "drive.google.com" do + options |> Keyword.merge(hackney: [force_redirect: true]) + else + options + end + + case http_client().head(url, [], options) do # See https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#successful_responses # Other 2xx status codes don't seem appropriate here {:ok, %Response{status_code: 200}} -> diff --git a/apps/transport/test/transport/availability_checker_test.exs b/apps/transport/test/transport/availability_checker_test.exs index 1493c8c4f9..52da1ddd10 100644 --- a/apps/transport/test/transport/availability_checker_test.exs +++ b/apps/transport/test/transport/availability_checker_test.exs @@ -67,6 +67,14 @@ defmodule Transport.AvailabilityCheckerTest do test_fallback_to_stream(403) end + test "redirection for Google Drive" do + expect(Transport.HTTPoison.Mock, :head, fn _url, [], [follow_redirect: true, hackney: [force_redirect: true]] -> + {:ok, %HTTPoison.Response{status_code: 200}} + end) + + assert AvailabilityChecker.available?("GTFS", "https://drive.google.com/test_url") + end + defp mock_head_with_status(status_code) do expect(Transport.HTTPoison.Mock, :head, fn _url, [], [follow_redirect: true] -> {:ok, %HTTPoison.Response{status_code: status_code}}