search

ramsayleung / rspotify

Spotify Web API SDK implemented on Rust
MIT License
632 stars 121 forks source link

Fix ureq-native-tls feature to actually use native-tls #471

Closed jirutka closed 5 months ago

jirutka commented 5 months ago

Description

Feature ureq-native-tls currently doesn't work because ureq requires the TLS connector to be specified explicitly for native-tls:

native-tls enables an adapter so you can pass a native_tls::TlsConnector instance to AgentBuilder::tls_connector. Due to the risk of diamond dependencies accidentally switching on an unwanted TLS implementation, native-tls is never picked up as a default or used by the crate level convenience calls (ureq::get etc) – it must be configured on the agent. -- https://github.com/algesten/ureq#features

When ncspot is built with rspotify with ureq-native-tls, it's unable to create an HTTPS connection:

[ncspot::spotify_api] [DEBUG] http error: Transport(Transport { kind: UnknownScheme, message: Some("cannot make HTTPS request because no TLS backend is configured"), url: Some(Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("api.spotify.com")), port: None, path: "/v1/me/", query: None, fragment: None }), source: None })

Motivation and Context

rspotify currently doesn’t work when built with ureq-native-tls.

Dependencies

tls-native (optional)

Type of change

Please delete options that are not relevant.

How has this been tested?

CI

Is this change properly documented?

Yes

Related to #402

Fixes hrkfdn/ncspot#1159

ramsayleung commented 5 months ago

native-tls enables an adapter so you can pass a native_tls::TlsConnector instance to AgentBuilder::tls_connector. Due to the risk of diamond dependencies accidentally switching on an unwanted TLS implementation, native-tls is never picked up as a default or used by the crate level convenience calls (ureq::get etc) – it must be configured on the agent. -- https://github.com/algesten/ureq#features

If I understand correctly, if you want to use ureq-native-tls feature, you have to enable ureq-native-tls feature and also explictly add native-tls as dependency and set it up?

jirutka commented 5 months ago

you have to enable ureq-native-tls feature and also explictly add native-tls as dependency and set it up?

You have to explicitly pass a native_tls::TlsConnector instance to AgentBuilder::tls_connector. native_tls is provided by the native-tls crate, so yeah, you have to add native-tls as a dependency.

ramsayleung commented 5 months ago

Thanks for the contribution :)

jirutka commented 5 months ago

Can you please release a new version soon? :)

ramsayleung commented 5 months ago

Yeah, I've released the v0.13.1