Skip to content
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

Should mint be responsible for error handling empty address? #351

Closed
sato11 opened this issue May 2, 2022 · 2 comments
Closed

Should mint be responsible for error handling empty address? #351

sato11 opened this issue May 2, 2022 · 2 comments

Comments

@sato11
Copy link

sato11 commented May 2, 2022

Taking a look into sneako/finch#186, I've found the case where :gen_tcp.connect/4 throws :badarg which mint is unable to handle. Is it in mint's scope to take care of it? Personally I'd find it great if it was since I feels that zero byte strings are not inconsistent with Mint.Types.address() which contains String.t(). What do you think?

iex(31)> Mint.HTTP.connect(:http, "httpbin.org", 80)  
{:ok,
 %Mint.HTTP1{
   buffer: "",
   host: "httpbin.org",
   mode: :active,
   port: 80,
   private: %{},
   proxy_headers: [],
   request: nil,
   requests: {[], []},
   scheme_as_string: "http",
   socket: #Port<0.9>,
   state: :open,
   streaming_request: nil,
   transport: Mint.Core.Transport.TCP
 }}
iex(32)> Mint.HTTP.connect(:http, "httpbin.void", 80)
{:error, %Mint.TransportError{reason: :nxdomain}}
iex(33)> Mint.HTTP.connect(:http, "", 80)            
** (exit) :badarg
    (kernel 8.3.1) gen_tcp.erl:218: :gen_tcp.connect/4
    (mint 1.4.0) lib/mint/core/transport/tcp.ex:41: Mint.Core.Transport.TCP.connect/3
    (mint 1.4.0) lib/mint/http1.ex:115: Mint.HTTP1.connect/4
@wojtekmach
Copy link
Contributor

wojtekmach commented May 2, 2022

I believe the problem is not that Mint crashes for this value but that the reason is very opaque. A common solution is to add additional information in accordance with https://www.erlang.org/eeps/eep-0054. Given it is an exit we cannot do that though. So unfortunately I don’t think Mint can do much about it. (It could special case "" but eg "/(*" would be invalid too.) I think the real fix would have to happen in OTP.

@sato11
Copy link
Author

sato11 commented May 2, 2022

Hi, thanks for the feedback and the link. I had a feeling that this is maybe OTP issue, but I was not very sure about it and asked here beforehand. But thanks to your comment I guess it's making sense to me. I'll be reporting it in OTP after a little more research. Thanks a lot!

@sato11 sato11 closed this as completed May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants