-
Notifications
You must be signed in to change notification settings - Fork 468
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
Fix build with OpenSSL 1.1.1 #170
Conversation
Update hyper to 0.12 Update hyper-tls to 0.3 update websocket to 0.21 Update http.rs to new hyper 0.12 API bump version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Two minor issues.
}) | ||
let auth = match url.password() { | ||
Some(pass) => format!("{}:{}", user, pass), | ||
None => format!("{}:", user) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you sure we need the trailing :
? Maybe let's encode only the username.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was conflicted on this myself, since the HTTP Basic Auth RFC does not specify exactly what to do with an empty password, just that the format must be 'Basic username:password', with user:pass part encoded in Base64. I am not well-versed on HTTP spec, though, so I may be wrong on this, but figured it would be safest to do 'user:'
https://tools.ietf.org/html/rfc7617#section-2
My conclusion was no password just counts as an 'empty' password so it would be encoded as 'Basic username:'
Just checked the removed Basic Typed implementation in Hyper 0.11, it seems the trailing ':' would be included regardless
https://github.com/hyperium/hyper/blob/76fdbcf23cd35cebb03bf4c0e3025b671578bd75/src/header/common/authorization.rs#L156
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, makes sense!
src/transports/http.rs
Outdated
} | ||
req.set_body(request); | ||
|
||
// Send basic auth header |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment should be above if
I suppose :)
* Fix build with OpenSSL 1.1.1 Update hyper to 0.12 Update hyper-tls to 0.3 update websocket to 0.21 Update http.rs to new hyper 0.12 API bump version * fix comment placement
* Add `traces` namespace basic code for traces namespace and types tests for traces and trace_filtering types builder for Trace Filter tests for `traces` namespace and traces types (traces + trace_filtering) move ValueOrArray back * Add `traces` namespace basic code for traces namespace and types tests for traces and trace_filtering types builder for Trace Filter tests for `traces` namespace and traces types (traces + trace_filtering) * fix return type of `trace_get` * Fix types for Addresses Some addresses were usize, some were H160. This changes all addresses in `traces` to be `Address` * make blockNumber and blockHash optional in TransactionReceipt (#164) * Derive Eq and Hash for Bytes (#168) This just forwards to the implementation of Vec. * Fix build with OpenSSL 1.1.1 (#170) * Fix build with OpenSSL 1.1.1 Update hyper to 0.12 Update hyper-tls to 0.3 update websocket to 0.21 Update http.rs to new hyper 0.12 API bump version * fix comment placement * Revise `Traces` namespace remove custom deserialize, switch to untagged serde representation
Update hyper to 0.12
Update hyper-tls to 0.3
update websocket to 0.21
Update http.rs to new hyper 0.12 API
bump version
Fixes #162
This fix means upgrading to hyper 0.12 to support OpenSSL 1.1.1. This works out OK, but the largest change to take note of would probably be the removal of typed-headers in 0.12.