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

Shouldn't hyper::header::Authorization be in headers instead of items? #1185

Closed
InstinctBas opened this issue Dec 12, 2019 · 8 comments
Closed
Labels
deficiency Something doesn't work as well as it could upstream An unresolvable issue: an upstream dependency bug

Comments

@InstinctBas
Copy link

I'm getting a compilation error on this code:

let client = Client::new(rocket()).expect("valid rocket instance");
let mut response = client
	.get("/someurl")
	.header(header::Host {
		hostname: hostname.to_owned(),
		port: None,
	})
	.header(header::Authorization(
		header::Bearer { token: token.to_owned() }
	))
	.dispatch();

Which is:

the trait `std::convert::From<rocket::http::hyper::header::Authorization<rocket::http::hyper::header::Bearer>>` is not implemented for `rocket::http::Header<'static>`

If I understand the code here:
https://github.com/SergioBenitez/Rocket/blob/master/core/http/src/hyper.rs#L49
correctly, shouldn't Authorization be in import_hyper_headers! instead of import_hyper_items! ?

@jebrosen
Copy link
Collaborator

Unfortunately that wouldn't work. import_hyper_headers! generates a call to to_string(), and Authorization is one of only a few headers that does not implement Display.

Rocket could implement it manually for Authorization or change the approach entirely, but any fix is unlikely to be in a release before 0.5, which currently loses all typed header support. #1067 is the tracking issue to decide what to do with them going forward.

@jebrosen jebrosen added deficiency Something doesn't work as well as it could request Request for new functionality upstream An unresolvable issue: an upstream dependency bug and removed request Request for new functionality labels Dec 12, 2019
@InstinctBas
Copy link
Author

Bugger... it's ashame we don't have fall back where you can add a header by supplying a simple name/value pair. Or is there an option for that which I've missed?

I've worked around it for now by sending the data as a cookie instead of in the header. Just trying to automate testing my API with CI :)

@jebrosen
Copy link
Collaborator

Or is there an option for that which I've missed?

You can use Header::new() with a name and value. Something like Header::new("Authorization", format!("Bearer {}", token)) should work.

@InstinctBas
Copy link
Author

Sweet! that'll do :)

@InstinctBas
Copy link
Author

@jebrosen hmm, it's complaining rocket::http::hyper::header::Header is a private member. Is there a different path to it?

@jebrosen
Copy link
Collaborator

I think that's not just a different name, it's the wrong Header. rocket::http::Header is the one you're after.

@InstinctBas
Copy link
Author

Hmmm wonder where I got the other path from, anyway that worked :)

@SergioBenitez
Copy link
Member

This is gone in master. #1067 tracks bringing something similar back. Ideally we'd resolve these kinds of issues then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deficiency Something doesn't work as well as it could upstream An unresolvable issue: an upstream dependency bug
Projects
None yet
Development

No branches or pull requests

3 participants