Plug.Conn.request_url with request_path not starting with "/"

[tomekowal]

I am wondering if it would make sense to insert "/" at the beginning of non-empty request_path.

My reasoning is this:

Plug.Conn.request_url Plug.Adapters.Test.Conn.conn(%Plug.Conn{}, :get, "hello", nil)
"http://www.example.comhello"

The path gets appended to the default host. However, I think that is inconsistent. E.g. path_info in this scenario is ["hello"] which is the same as when calling with the slash at the beginning. So the request_url should be "http://www.example.com/hello"

To add more context why I think it is useful: I was using LiveView today and when calling live(conn, "hello?jwt=asdf") it produced very strange behaviour. First mount passed the "jwt" => "asdf" but the second one did not. This is because live_view internally uses request_url to check if the live view is mounted at the router. This kind of error took me a lot of time to debug so I wondered if I can make live_view detect it and spare other people debugging time.

While debugging through phoenix_live_view -> phoenix -> plug I feel that this is the first place where it makes sense to guard against forgotten "/".

Let me know if you like the idea of adding the slash between request_path and host if it is not already there. If you also feel it is a good idea, I am happy to make a PR!

Alternatively, we could crash on request_paths that don't have "/" at the beginning to force everyone to use it and prevent such problems while maintaining very clean code where everyone uses the same convention with "/" at the beginning.

[josevalim]

I would go with raising for now, because I think we validate a leading / almost everywhere? Can you please send a PR?

[tomekowal]

Yes, I will!

[whatyouhide]

Closing in favor of the PR: https://github.com/elixir-plug/plug/pull/1059