Skip to content

URI.to_string causes ArgumentError when :port is :undefined #13462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pjdiiori opened this issue Apr 1, 2024 · 0 comments · Fixed by #13464
Closed

URI.to_string causes ArgumentError when :port is :undefined #13462

pjdiiori opened this issue Apr 1, 2024 · 0 comments · Fixed by #13464

Comments

@pjdiiori
Copy link

pjdiiori commented Apr 1, 2024

Elixir and Erlang/OTP versions

Erlang/OTP 26 [erts-14.2.3] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1]

Elixir 1.16.2 (compiled with Erlang/OTP 26)

Operating system

MacOS Sonoma 14.3.1

Current behavior

When parsing/validating a string with URI.new/1, if the string contains "//https://" you receive a URI struct with port: :undefined. Then, if you feed that struct into URI.to_string/1, you get an Argument error.

iex(1)> {:ok, uri} = URI.new("//https://www.example.com")
{:ok,
 %URI{
   scheme: nil,
   userinfo: nil,
   host: "https",
   port: :undefined,
   path: "//www.example.com",
   query: nil,
   fragment: nil
 }}
iex(2)> URI.to_string(uri)
** (ArgumentError) errors were found at the given arguments:

  * 1st argument: not an integer

    :erlang.integer_to_binary(:undefined)
    (elixir 1.15.7) lib/uri.ex:1057: String.Chars.URI.extract_authority/1
    (elixir 1.15.7) lib/uri.ex:1034: String.Chars.URI.to_string/1
    iex:2: (file)
iex(2)>

Expected behavior

I would expect that

  1. URI.new/1 should not return :undefined or any other atom for the :port field since its type is defined as nil | :inet.port_number() https://github.com/elixir-lang/elixir/blob/v1.16.2/lib/elixir/lib/uri.ex#L31. I'd think it would return either an error tuple, or a URI struct with port: nil.
  2. Or, URI.to_string/1 should handle a URI struct with port: :undefined by returning a string with the URI.
tfiedlerdejanze added a commit to tfiedlerdejanze/elixir that referenced this issue Apr 1, 2024
When a url string is schema less and the host is followed by a colon
without setting the actual port, `:uri_string.parse/1` returns the
port to be `:undefined`. As long as the url string is parseable,
we should translate `:undefiend` to `nil`, in order to ensure we return a valid URI struct.
tfiedlerdejanze added a commit to tfiedlerdejanze/elixir that referenced this issue Apr 1, 2024
Resolves elixir-lang#13462

When a url string is schema less and the host is followed by a colon
without setting the actual port, `:uri_string.parse/1` returns the
port to be `:undefined`. As long as the url string is parseable,
we should translate `:undefiend` to `nil`, in order to ensure we return a valid URI struct.
josevalim pushed a commit that referenced this issue Apr 2, 2024
Resolves #13462

When a url string is schema less and the host is followed by a colon
without setting the actual port, `:uri_string.parse/1` returns the
port to be `:undefined`. As long as the url string is parseable,
we should translate `:undefined` to `nil`, in order to ensure we return
a valid URI struct.
josevalim pushed a commit that referenced this issue Apr 4, 2024
Resolves #13462

When a url string is schema less and the host is followed by a colon
without setting the actual port, `:uri_string.parse/1` returns the
port to be `:undefined`. As long as the url string is parseable,
we should translate `:undefined` to `nil`, in order to ensure we return
a valid URI struct.
josevalim pushed a commit that referenced this issue May 6, 2024
Resolves #13462

When a url string is schema less and the host is followed by a colon
without setting the actual port, `:uri_string.parse/1` returns the
port to be `:undefined`. As long as the url string is parseable,
we should translate `:undefined` to `nil`, in order to ensure we return
a valid URI struct.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant