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

Proxy silently ignored without http: #1282

Closed
gurgeous opened this issue May 18, 2021 · 4 comments · Fixed by #1283
Closed

Proxy silently ignored without http: #1282

gurgeous opened this issue May 18, 2021 · 4 comments · Fixed by #1283

Comments

@gurgeous
Copy link
Contributor

Basic Info

  • Faraday Version: 1.4.1
  • Ruby Version: 2.7

Issue description

Proxy silently ignored without http:. For example, if you set proxy to something:1234 it will be silently ignored. The correct proxy setting would be http://something:1234. This is an important issue because it's undocumented, difficult to debug, and can have serious consequences for crawling projects.

I propose either:

  1. In ProxyOptions.from, reject strings that lack a scheme.
    or
  2. In ProxyOptions.from, default to http:// for strings that lack a scheme.

Let me know if you agree with one of these options and I'l put together a PR. Thanks!

Steps to reproduce

> Faraday::ProxyOptions.from("something:1234").host
# => nil

> Faraday::ProxyOptions.from("http://something:1234").host
# => "something"
@iMacTia
Copy link
Member

iMacTia commented May 20, 2021

Thanks @gurgeous option 2 sounds good to me, but I have to admit I'm no expert on proxies... are we sure there's no use case where one might want to omit the protocol?

We pass the string as is to URI for parsing. something:1234 is a valid URI, although that will populate the scheme and opaque properties. It seems like a use case for mailto and similar links, which I guess would not apply to proxies.

@gurgeous
Copy link
Contributor Author

I am not an expert either... as far as I know the only use cases are http(s) and socks? Not sure if socks is even supported yet. I'll put together a simple PR and we can see if anyone complains.

@iMacTia
Copy link
Member

iMacTia commented May 21, 2021

Sounds good, thanks!
I don't think we support SOCKS atm, but I remember there was some work on that in the past

@olleolleolle
Copy link
Member

@iMacTia Reading that work, made me realize that "assume it's an HTTP proxy" is a pretty good assumption.

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

Successfully merging a pull request may close this issue.

3 participants