search

elixir-plug / plug

Compose web applications with functions
https://hex.pm/packages/plug
Other
2.88k stars 586 forks source link

plug ssl: Enable redirect host to depend on conn.host #1030

Closed Harper04 closed 3 years ago

Harper04 commented 3 years ago

Hi,

i came across this use case for the third time now and decided to make a pull request. The host callback ( {MODULE, :func, []} is not able to get the host of the request. Therefore i cant redirect http://example.com to http://example.com while redirecting http://example.de to https://example.de.

I figured a way could be to just replace the first argument if i specify :host while preserving existing behavior.

Usage is

  force_ssl: [
    rewrite_on: [:x_forwarded_proto],
    host: {XXX.Endpoint, :ssl_redirect_host, [:host]},
    exclude: {XXX.Endpoint, :ssl_excluded_redirect_host?, []}
  ]
  def ssl_redirect_host("example.de"), do: "example.de"
  def ssl_redirect_host(host), do:  host
josevalim commented 3 years ago

The particular example that you provided could be achieved by simply setting not setting the option. Or do you have something slightly more complex?

In any case, this can be a breaking change if someone is passing the :host atom as an option today. The only way I can see support this is by adding a new redirect_unsafe_to option and deprecate the host one.

Harper04 commented 3 years ago

Oh did not know that! Phoenix sets the host by default according to its endpoint definition.

With that in mind the only use cases i see are 1) keep using the force_ssl option in phoenix possible. But maybe host: nil would suffice too cause phoenix is using put_new 2) having one redirect less from https://www.example.com to https://example.com

I think 2 does not justify a change like that. Thank you!