CrowdHailer / raxx

Interface for HTTP webservers, frameworks and clients
https://hexdocs.pm/raxx
Apache License 2.0
400 stars 29 forks source link

Remove eex_html dep? #170

Closed lasseebert closed 5 years ago

lasseebert commented 5 years ago

After Raxx.View was extracted, it seems that EExHTML is only used here.

Would it be a good idea to remove the eex_html dependency, since that small piece of redirect html should be easy to escape safely?

CrowdHailer commented 5 years ago

Agreed.

It was on a todo list of mine somewhere as I was in the process of removing dependencies over the last few releases.

Just as a counter point EExHTML is a very small dependency. Will escaping even this small piece require a general purpose html escape function, i.e. very similar to the one in EExHTML

lasseebert commented 5 years ago

One could also argue that a redirect with a HTML body is not in the scope of Raxx after Raxx.View has been extracted (I guess to make Raxx more general purpose?).

Perhaps it would be an idea to change redirect/2 to not include a body by default, but to make it configurable to do so. Something like this:

@spec redirect(String.t(), status: status, body: String.t(), content_type: String.t()) :: Raxx.Response.t()
def redirect(url, opts \\ []) do
  status = Keyword.get(opts, :status, :see_other)

  response = status |> response() |> set_header("location", url)

  case Keyword.fetch(opts, :body) do
    {:ok, body} ->
      content_type = Keyword.fetch!(opts, :content_type)

      response
      |> set_header("content-type", content_type)
      |> set_body(body)

    :error ->
      response
  end
end
CrowdHailer commented 5 years ago

Good answer, because it's fair enough to point out that the HTML body is useless if creating an API service.

lasseebert commented 5 years ago

I can make a PR if you'd like that :)

CrowdHailer commented 5 years ago

That would be lovely :+1: