From 934c6f44764af0a589e0a31bf6cb89aaa4b7e681 Mon Sep 17 00:00:00 2001 From: Neil Berkman Date: Tue, 10 Dec 2019 13:13:51 -0800 Subject: [PATCH] Handle providers listed with subdomains host_matches? previously would fail to match providers with URLs such as http://www.twitter.com/. Generalized the regex to handle subdomains. --- lib/furlex/oembed.ex | 33 ++++++++++++++++--------------- test/fixtures/providers.json | 15 +++++++++++++- test/furlex/oembed_test.exs | 38 ++++++++++++++++++++++++------------ 3 files changed, 57 insertions(+), 29 deletions(-) diff --git a/lib/furlex/oembed.ex b/lib/furlex/oembed.ex index ab185be..dcf7a5e 100644 --- a/lib/furlex/oembed.ex +++ b/lib/furlex/oembed.ex @@ -16,22 +16,24 @@ defmodule Furlex.Oembed do Soft fetch will fetch cached providers. Hard fetch requests providers from oembed.com and purges the cache. """ - @spec fetch_providers(Atom.t) :: {:ok, List.t} | {:error, Atom.t} + @spec fetch_providers(Atom.t()) :: {:ok, List.t()} | {:error, Atom.t()} def fetch_providers(type \\ :soft) + def fetch_providers(:hard) do case get("/providers.json") do {:ok, %{body: providers}} -> - GenServer.cast __MODULE__, {:providers, providers} + GenServer.cast(__MODULE__, {:providers, providers}) {:ok, providers} - other -> - Logger.error "Could not fetch providers: #{inspect other}" + other -> + Logger.error("Could not fetch providers: #{inspect(other)}") {:error, :fetch_error} end end + def fetch_providers(_soft) do case GenServer.call(__MODULE__, :providers) do - nil -> fetch_providers(:hard) + nil -> fetch_providers(:hard) providers -> {:ok, providers} end end @@ -47,10 +49,10 @@ defmodule Furlex.Oembed do iex> Oembed.endpoint_from_url "https://vimeo.com/88856141", %{"format" => "xml"} {:ok, "https://vimeo.com/api/oembed.xml"} """ - @spec endpoint_from_url(String.t, Map.t) :: {:ok, String.t} | {:error, Atom.t} + @spec endpoint_from_url(String.t(), Map.t()) :: {:ok, String.t()} | {:error, Atom.t()} def endpoint_from_url(url, params \\ %{"format" => "json"}, opts \\ []) do case provider_from_url(url, opts) do - nil -> + nil -> {:error, :no_oembed_provider} provider -> @@ -60,39 +62,38 @@ defmodule Furlex.Oembed do # Maps a url to a provider, or returns nil if no such provider exists defp provider_from_url(url, opts) do - fetch_type = - if Keyword.get(opts, :skip_cache?, false), do: :hard, else: :soft + fetch_type = if Keyword.get(opts, :skip_cache?, false), do: :hard, else: :soft {:ok, providers} = fetch_providers(fetch_type) case URI.parse(url) do - %URI{host: nil} -> + %URI{host: nil} -> nil %URI{host: host} -> - Enum.find providers, &host_matches?(host, &1) + Enum.find(providers, &host_matches?(host, &1)) end end defp endpoint_from_provider(provider, params) do - [ endpoint | _] = provider["endpoints"] + [endpoint | _] = provider["endpoints"] - url = endpoint["url"] + url = endpoint["url"] regex = ~r/{(.*?)}/ - url = Regex.replace regex, url, fn _, key -> params[key] end + url = Regex.replace(regex, url, fn _, key -> params[key] end) {:ok, url} end defp host_matches?(host, %{"provider_url" => provider_url}) do - Regex.match? ~r/https?:\/\/#{host}/, provider_url + Regex.match?(~r/https?:\/\/([a-zA-Z0-9]+\.)?#{host}/, provider_url) end ## GenServer callbacks @doc false def start_link(opts \\ []) do - GenServer.start_link __MODULE__, nil, opts + GenServer.start_link(__MODULE__, nil, opts) end def init(state) do diff --git a/test/fixtures/providers.json b/test/fixtures/providers.json index 73ed851..5627b32 100644 --- a/test/fixtures/providers.json +++ b/test/fixtures/providers.json @@ -1,4 +1,17 @@ [ + { + "provider_name": "Twitter", + "provider_url": "http:\/\/www.twitter.com/", + "endpoints": [ + { + "schemes": [ + "https:\/\/twitter.com\/*\/status\/*", + "https:\/\/*.twitter.com\/*\/status\/*" + ], + "url": "https:\/\/publish.twitter.com\/oembed" + } + ] + }, { "provider_name": "VideoJug", "provider_url": "http:\/\/www.videojug.com", @@ -147,4 +160,4 @@ } ] } -] +] \ No newline at end of file diff --git a/test/furlex/oembed_test.exs b/test/furlex/oembed_test.exs index 70176bb..8964f81 100644 --- a/test/furlex/oembed_test.exs +++ b/test/furlex/oembed_test.exs @@ -5,35 +5,49 @@ defmodule Furlex.OembedTest do setup do bypass = Bypass.open() - url = "http://localhost:#{bypass.port}" - config = Application.get_env :furlex, Oembed, [] + url = "http://localhost:#{bypass.port}" + config = Application.get_env(:furlex, Oembed, []) - new_config = Keyword.put config, :oembed_host, url - Application.put_env :furlex, Oembed, new_config + new_config = Keyword.put(config, :oembed_host, url) + Application.put_env(:furlex, Oembed, new_config) - on_exit fn -> - Application.put_env :furlex, Oembed, config + on_exit(fn -> + Application.put_env(:furlex, Oembed, config) :ok - end + end) {:ok, bypass: bypass} end test "returns endpoint from url", %{bypass: bypass} do - Bypass.expect bypass, &handle/1 + Bypass.expect(bypass, &handle/1) assert {:error, :no_oembed_provider} == - Oembed.endpoint_from_url("foobar") + Oembed.endpoint_from_url("foobar") - url = "https://vimeo.com/88856141" + url = "https://vimeo.com/88856141" params = %{"format" => "json"} - {:ok, endpoint} = Oembed.endpoint_from_url(url, params, [skip_cache?: true]) + {:ok, endpoint} = Oembed.endpoint_from_url(url, params, skip_cache?: true) assert endpoint == "https://vimeo.com/api/oembed.json" end + test "returns endpoint from url with subdomain", %{bypass: bypass} do + Bypass.expect(bypass, &handle/1) + + assert {:error, :no_oembed_provider} == + Oembed.endpoint_from_url("foobar") + + url = "https://twitter.com/arshia__/status/1204481088422178817?s=20" + params = %{"format" => "json"} + + {:ok, endpoint} = Oembed.endpoint_from_url(url, params, skip_cache?: true) + + assert endpoint == "https://publish.twitter.com/oembed" + end + def handle(%{request_path: "/providers.json"} = conn) do assert conn.method == "GET" @@ -42,6 +56,6 @@ defmodule Furlex.OembedTest do |> Path.join() |> File.read!() - Plug.Conn.resp conn, 200, providers + Plug.Conn.resp(conn, 200, providers) end end