inejge / env_proxy

Determination of proxy parameters from the environment
Apache License 2.0
6 stars 6 forks source link

to_url broken with url > 2.1.1 #8

Open stevendanna opened 4 years ago

stevendanna commented 4 years ago

We've recently observed the to_string method returning None with the following warning:

[2020-03-04T10:46:59Z WARN  env_proxy] could not set URL scheme back to https 

I think I've tracked this down to an incompatibility between env_proxy and the 2.1.1 version of the URL crate.

The following code constructs a new Url with the scheme xhttp or xhttps and then asssume we can call set_scheme to set it back to the original scheme:

https://github.com/inejge/env_proxy/blob/8bcfd3a80ac2f46288b513df7964888438b2e288/src/lib.rs#L175-L197

As of this commit in the url crate, you cannot set the scheme from a non-canonical scheme to a canonical scheme:

https://github.com/servo/rust-url/commit/7efdc53193adfdfd65c1d39bc7ad4762dd4c272b#diff-b4aea3e418ccdb71239b96952d9cddb6R2029-R2077

As a result, the env_proxy to_url function fails in all of the use cases I've tried. The crate's tests also fail when using url 2.1.1+:

> rg url Cargo.lock 
14: "url 2.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
72:name = "url"
91:"checksum url 2.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "829d4a8476c35c9bf0bbce5a3b23f4106f79728039b726d292bb93bc106787cb"
> cargo +nightly test
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running target/debug/deps/env_proxy-e799bf1b7b6e89e3

running 13 tests
test tests::default_proxy_url_scheme ... FAILED
test tests::http_proxy_fallback ... FAILED
test tests::ftp_proxy_fallback ... FAILED
test tests::ftp_proxy_specific ... FAILED
test tests::no_proxy_subdomain ... ok
test tests::no_proxy_subdomain_dot ... ok
test tests::https_proxy_fallback ... FAILED
test tests::no_proxy_global ... ok
test tests::no_proxy_multiple_list ... ok
test tests::no_proxy_simple_name ... ok
test tests::http_proxy_specific ... FAILED
test tests::https_proxy_specific ... FAILED
test tests::proxy_url_with_explicit_scheme_port ... FAILED

failures:

---- tests::default_proxy_url_scheme stdout ----
thread 'tests::default_proxy_url_scheme' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(("proxy.example.com", 8080))`', src/lib.rs:405:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

---- tests::http_proxy_fallback stdout ----
thread 'tests::http_proxy_fallback' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(("proxy.example.com", 8080))`', src/lib.rs:437:9

---- tests::ftp_proxy_fallback stdout ----
thread 'tests::ftp_proxy_fallback' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(("proxy.example.org", 8081))`', src/lib.rs:515:9

---- tests::ftp_proxy_specific stdout ----
thread 'tests::ftp_proxy_specific' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(("proxy.example.com", 8080))`', src/lib.rs:496:9

---- tests::https_proxy_fallback stdout ----
thread 'tests::https_proxy_fallback' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(("proxy.example.org", 8081))`', src/lib.rs:476:9

---- tests::http_proxy_specific stdout ----
thread 'tests::http_proxy_specific' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(("proxy.example.com", 8080))`', src/lib.rs:392:9

---- tests::https_proxy_specific stdout ----
thread 'tests::https_proxy_specific' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(("proxy.example.com", 8080))`', src/lib.rs:457:9

---- tests::proxy_url_with_explicit_scheme_port stdout ----
thread 'tests::proxy_url_with_explicit_scheme_port' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(("proxy.example.com", 80))`', src/lib.rs:418:9

failures:
    tests::default_proxy_url_scheme
    tests::ftp_proxy_fallback
    tests::ftp_proxy_specific
    tests::http_proxy_fallback
    tests::http_proxy_specific
    tests::https_proxy_fallback
    tests::https_proxy_specific
    tests::proxy_url_with_explicit_scheme_port

test result: FAILED. 5 passed; 8 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--lib'

But pass if you run them with URL 2.0:

> rg url Cargo.lock 
14: "url 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
72:name = "url"
91:"checksum url 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "77ddaf52e65c6b81c56b7e957c0b1970f7937f21c5c6774c4e56fcb4e20b48c6"
> cargo +nightly test
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running target/debug/deps/env_proxy-c6ac0eaf955226c7

running 13 tests
test tests::default_proxy_url_scheme ... ok
test tests::http_proxy_fallback ... ok
test tests::ftp_proxy_fallback ... ok
test tests::ftp_proxy_specific ... ok
test tests::http_proxy_specific ... ok
test tests::https_proxy_fallback ... ok
test tests::https_proxy_specific ... ok
test tests::no_proxy_global ... ok
test tests::no_proxy_multiple_list ... ok
test tests::no_proxy_simple_name ... ok
test tests::no_proxy_subdomain ... ok
test tests::no_proxy_subdomain_dot ... ok
test tests::proxy_url_with_explicit_scheme_port ... ok

test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

   Doc-tests env_proxy

running 2 tests
test src/lib.rs -  (line 23) ... ok
test src/lib.rs -  (line 31) ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
stevendanna commented 4 years ago

More discussion around the url change here: https://github.com/servo/rust-url/issues/577

inejge commented 4 years ago

Uh, that change is not something I'd expect from a patch release. I created a workaround, try 5591cc7ffa135abe818ec5eb812de530de3ba153.

stevendanna commented 4 years ago

Thanks! I'll give it a try.

stevendanna commented 4 years ago

Your fix works for me locally, thanks again.

I started looking into whether there was a clean way to add some features upstream for this use case, I'll try to remember to follow up if I make progress on that.