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

Fix #1067 by importing typed headers from hyperx #1535

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions core/codegen/tests/ui-fail-nightly/responder-types.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@ error[E0277]: the trait bound `u8: Responder<'_, '_>` is not satisfied
|
= note: required by `respond_to`

error[E0277]: the trait bound `Header<'_>: From<u8>` is not satisfied
error[E0277]: the trait bound `Header<'_>: std::convert::From<u8>` is not satisfied
--> $DIR/responder-types.rs:11:5
|
11 | other: u8,
| ^^^^^^^^^ the trait `From<u8>` is not implemented for `Header<'_>`
| ^^^^^^^^^ the trait `std::convert::From<u8>` is not implemented for `Header<'_>`
|
= help: the following implementations were found:
<Header<'static> as From<&Cookie<'_>>>
<Header<'static> as From<Cookie<'_>>>
<Header<'static> as std::convert::From<&rocket::http::Cookie<'_>>>
<Header<'static> as std::convert::From<AcceptCharset>>
<Header<'static> as std::convert::From<AcceptEncoding>>
<Header<'static> as std::convert::From<AcceptLanguage>>
and 55 others
= note: required because of the requirements on the impl of `Into<Header<'_>>` for `u8`

error[E0277]: the trait bound `u8: Responder<'_, '_>` is not satisfied
Expand All @@ -25,26 +28,32 @@ error[E0277]: the trait bound `u8: Responder<'_, '_>` is not satisfied
|
= note: required by `respond_to`

error[E0277]: the trait bound `Header<'_>: From<u8>` is not satisfied
error[E0277]: the trait bound `Header<'_>: std::convert::From<u8>` is not satisfied
--> $DIR/responder-types.rs:17:5
|
17 | other: u8,
| ^^^^^^^^^ the trait `From<u8>` is not implemented for `Header<'_>`
| ^^^^^^^^^ the trait `std::convert::From<u8>` is not implemented for `Header<'_>`
|
= help: the following implementations were found:
<Header<'static> as From<&Cookie<'_>>>
<Header<'static> as From<Cookie<'_>>>
<Header<'static> as std::convert::From<&rocket::http::Cookie<'_>>>
<Header<'static> as std::convert::From<AcceptCharset>>
<Header<'static> as std::convert::From<AcceptEncoding>>
<Header<'static> as std::convert::From<AcceptLanguage>>
and 55 others
= note: required because of the requirements on the impl of `Into<Header<'_>>` for `u8`

error[E0277]: the trait bound `Header<'_>: From<std::string::String>` is not satisfied
error[E0277]: the trait bound `Header<'_>: std::convert::From<std::string::String>` is not satisfied
--> $DIR/responder-types.rs:24:5
|
24 | then: String,
| ^^^^^^^^^^^^ the trait `From<std::string::String>` is not implemented for `Header<'_>`
| ^^^^^^^^^^^^ the trait `std::convert::From<std::string::String>` is not implemented for `Header<'_>`
|
= help: the following implementations were found:
<Header<'static> as From<&Cookie<'_>>>
<Header<'static> as From<Cookie<'_>>>
<Header<'static> as std::convert::From<&rocket::http::Cookie<'_>>>
<Header<'static> as std::convert::From<AcceptCharset>>
<Header<'static> as std::convert::From<AcceptEncoding>>
<Header<'static> as std::convert::From<AcceptLanguage>>
and 55 others
= note: required because of the requirements on the impl of `Into<Header<'_>>` for `std::string::String`

error[E0277]: the trait bound `usize: Responder<'_, '_>` is not satisfied
Expand All @@ -53,4 +62,4 @@ error[E0277]: the trait bound `usize: Responder<'_, '_>` is not satisfied
28 | fn foo() -> usize { 0 }
| ^^^^^ the trait `Responder<'_, '_>` is not implemented for `usize`
|
= note: required by `handler::<impl rocket::outcome::Outcome<rocket::Response<'o>, Status, rocket::Data>>::from`
= note: required by `handler::<impl Outcome<rocket::Response<'o>, Status, rocket::Data>>::from`
33 changes: 21 additions & 12 deletions core/codegen/tests/ui-fail-stable/responder-types.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@ error[E0277]: the trait bound `u8: Responder<'_, '_>` is not satisfied
|
= note: required by `respond_to`

error[E0277]: the trait bound `Header<'_>: From<u8>` is not satisfied
error[E0277]: the trait bound `Header<'_>: std::convert::From<u8>` is not satisfied
--> $DIR/responder-types.rs:11:5
|
11 | other: u8,
| ^^^^^ the trait `From<u8>` is not implemented for `Header<'_>`
| ^^^^^ the trait `std::convert::From<u8>` is not implemented for `Header<'_>`
|
= help: the following implementations were found:
<Header<'static> as From<&Cookie<'_>>>
<Header<'static> as From<Cookie<'_>>>
<Header<'static> as std::convert::From<&rocket::http::Cookie<'_>>>
<Header<'static> as std::convert::From<AcceptCharset>>
<Header<'static> as std::convert::From<AcceptEncoding>>
<Header<'static> as std::convert::From<AcceptLanguage>>
and 55 others
= note: required because of the requirements on the impl of `Into<Header<'_>>` for `u8`

error[E0277]: the trait bound `u8: Responder<'_, '_>` is not satisfied
Expand All @@ -25,26 +28,32 @@ error[E0277]: the trait bound `u8: Responder<'_, '_>` is not satisfied
|
= note: required by `respond_to`

error[E0277]: the trait bound `Header<'_>: From<u8>` is not satisfied
error[E0277]: the trait bound `Header<'_>: std::convert::From<u8>` is not satisfied
--> $DIR/responder-types.rs:17:5
|
17 | other: u8,
| ^^^^^ the trait `From<u8>` is not implemented for `Header<'_>`
| ^^^^^ the trait `std::convert::From<u8>` is not implemented for `Header<'_>`
|
= help: the following implementations were found:
<Header<'static> as From<&Cookie<'_>>>
<Header<'static> as From<Cookie<'_>>>
<Header<'static> as std::convert::From<&rocket::http::Cookie<'_>>>
<Header<'static> as std::convert::From<AcceptCharset>>
<Header<'static> as std::convert::From<AcceptEncoding>>
<Header<'static> as std::convert::From<AcceptLanguage>>
and 55 others
= note: required because of the requirements on the impl of `Into<Header<'_>>` for `u8`

error[E0277]: the trait bound `Header<'_>: From<std::string::String>` is not satisfied
error[E0277]: the trait bound `Header<'_>: std::convert::From<std::string::String>` is not satisfied
--> $DIR/responder-types.rs:24:5
|
24 | then: String,
| ^^^^ the trait `From<std::string::String>` is not implemented for `Header<'_>`
| ^^^^ the trait `std::convert::From<std::string::String>` is not implemented for `Header<'_>`
|
= help: the following implementations were found:
<Header<'static> as From<&Cookie<'_>>>
<Header<'static> as From<Cookie<'_>>>
<Header<'static> as std::convert::From<&rocket::http::Cookie<'_>>>
<Header<'static> as std::convert::From<AcceptCharset>>
<Header<'static> as std::convert::From<AcceptEncoding>>
<Header<'static> as std::convert::From<AcceptLanguage>>
and 55 others
= note: required because of the requirements on the impl of `Into<Header<'_>>` for `std::string::String`

error[E0277]: the trait bound `usize: Responder<'_, '_>` is not satisfied
Expand Down
1 change: 1 addition & 0 deletions core/http/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ private-cookies = ["cookie/private", "cookie/key-expansion"]
smallvec = "1.0"
percent-encoding = "2"
hyper = { version = "0.14", default-features = false, features = ["http1", "http2", "runtime", "server", "stream"] }
hyperx = { version = "1.3.0" }
http = "0.2"
mime = "0.3.13"
time = "0.2.11"
Expand Down
99 changes: 98 additions & 1 deletion core/http/src/hyper.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Re-exported hyper HTTP library types.
//! Re-exported hyper HTTP library types and hyperx typed headers.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is actually a bit weird, now that I'm seeing it. Can we try moving the headers parts into e.g. core/http/src/headers.rs instead?

//!
//! All types that are re-exported from Hyper reside inside of this module.
//! These types will, with certainty, be removed with time, but they reside here
Expand All @@ -21,6 +21,9 @@

/// Reexported http header types.
pub mod header {
use super::super::header::Header;
pub use hyperx::header::Header as HyperxHeaderTrait;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed in order to call HeaderName, right? As far as I can tell we did not re-export this trait in 0.4, but it does seem useful since it can be imported in the example. The name is a bit awkward, though.


macro_rules! import_http_headers {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's simply remove these header names re-exported from http. Typed headers should make most of them unneeded, and the only remaining benefit of import_http_headers was that applications don't need to depend on http directly.

There are a few places in rocket that use these, which used typed headers in 0.4 and could use them again.

($($name:ident),*) => ($(
pub use http::header::$name as $name;
Expand All @@ -43,4 +46,98 @@ pub mod header {
STRICT_TRANSPORT_SECURITY, TE, TRANSFER_ENCODING, UPGRADE, USER_AGENT,
VARY
}

macro_rules! import_hyperx_items {
($($item:ident),*) => ($(pub use hyperx::header::$item as $item;)*)
}

macro_rules! import_hyperx_headers {
($($name:ident),*) => ($(
impl ::std::convert::From<self::$name> for Header<'static> {
fn from(header: self::$name) -> Header<'static> {
Header::new($name::header_name(), header.to_string())
}
}
)*)
}

macro_rules! import_generic_hyperx_headers {
($($name:ident<$bound:ident>),*) => ($(
impl <T1: 'static + $bound> ::std::convert::From<self::$name<T1>>
for Header<'static> {
fn from(header: self::$name<T1>) -> Header<'static> {
Header::new($name::<T1>::header_name(), header.to_string())
}
}
)*)
}

import_hyperx_items! {
Accept, AcceptCharset, AcceptEncoding, AcceptLanguage, AcceptRanges,
AccessControlAllowCredentials, AccessControlAllowHeaders,
AccessControlAllowMethods, AccessControlAllowOrigin,
AccessControlExposeHeaders, AccessControlMaxAge,
AccessControlRequestHeaders, AccessControlRequestMethod, Allow,
Authorization, Basic, Bearer, ByteRangeSpec, CacheControl,
CacheDirective, Charset, Connection, ConnectionOption,
ContentDisposition, ContentEncoding, ContentLanguage, ContentLength,
ContentLocation, ContentRange, ContentRangeSpec, ContentType, Cookie,
Date, DispositionParam, DispositionType, Encoding, EntityTag, ETag,
Expect, Expires, From, Host, HttpDate, IfMatch, IfModifiedSince,
IfNoneMatch, IfRange, IfUnmodifiedSince, LastEventId, LastModified,
Link, LinkValue, Location, Origin, Pragma, Prefer, Preference,
PreferenceApplied, Protocol, ProtocolName, ProxyAuthorization, Quality,
QualityItem, Range, RangeUnit, Referer, ReferrerPolicy, RetryAfter,
Scheme, Server, SetCookie, StrictTransportSecurity,
Te, TransferEncoding, Upgrade, UserAgent, Vary, Warning, q, qitem
}

import_hyperx_headers! {
Accept, AcceptCharset, AcceptEncoding, AcceptLanguage, AcceptRanges,
AccessControlAllowCredentials, AccessControlAllowHeaders,
AccessControlAllowMethods, AccessControlAllowOrigin,
AccessControlExposeHeaders, AccessControlMaxAge,
AccessControlRequestHeaders, AccessControlRequestMethod, Allow,
CacheControl, Connection, ContentDisposition, ContentEncoding,
ContentLanguage, ContentLength, ContentLocation, ContentRange,
ContentType, Cookie, Date, ETag, Expires, Expect, From, Host, IfMatch,
IfModifiedSince, IfNoneMatch, IfUnmodifiedSince, IfRange, LastEventId,
LastModified, Link, Location, Origin, Pragma, Prefer, PreferenceApplied,
Range, Referer, ReferrerPolicy, RetryAfter, Server,
StrictTransportSecurity, Te, TransferEncoding, Upgrade, UserAgent, Vary,
Warning
}
import_generic_hyperx_headers! {
Authorization<Scheme>,
ProxyAuthorization<Scheme>
}
// Note: SetCookie is missing, since it must be formatted as separate header lines...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one isn't too bad a loss - SetCookie (and Cookie) is typically done via CookieJar.

}

#[cfg(test)]
mod tests {
use crate::header::HeaderMap;
use super::header::HyperxHeaderTrait; // Needed for Accept::header_name() below?!?!

#[test]
fn add_typed_header() {
use super::header::{Accept, QualityItem, q, qitem};
let mut map = HeaderMap::new();
map.add(Accept(vec![
QualityItem::new("audio/*".parse().unwrap(), q(200)),
qitem("audio/basic".parse().unwrap()),
]));
assert_eq!(map.get_one(Accept::header_name()), Some("audio/*; q=0.2, audio/basic"));
}

#[test]
fn add_typed_header_with_type_params() {
use super::header::{Authorization, Basic};
let mut map = HeaderMap::new();
map.add(Authorization(Basic {
username: "admin".to_owned(),
password: Some("12345678".to_owned())}));
assert_eq!(map.get_one("Authorization"), Some("Basic YWRtaW46MTIzNDU2Nzg="));
}

}
1 change: 1 addition & 0 deletions core/lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ version_check = "0.9.1"
[dev-dependencies]
bencher = "0.1"
figment = { version = "0.10", features = ["test"] }
language-tags = { version=">=0.2, <0.3" }

[[bench]]
name = "format-routing"
Expand Down
35 changes: 35 additions & 0 deletions core/lib/tests/respond-with-typed-headers-1067.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use rocket;

use rocket::{get, routes};
use rocket::response::Responder;
use rocket::http::hyper::header::{ContentLanguage, qitem};
use language_tags::langtag;

#[derive(Responder)]
struct DerivedResponder {
text: &'static str,
lang_header: ContentLanguage,
}

#[get("/")]
fn index() -> DerivedResponder {
DerivedResponder {
text: "Fyr raketten af!",
lang_header: ContentLanguage(vec![
qitem(langtag!(da))
])
}
}

#[test]
fn test_derive_reexports() {
use rocket::local::blocking::Client;

let rocket = rocket::ignite().mount("/", routes![index]);
let client = Client::tracked(rocket).unwrap();

let response = client.get("/").dispatch();
let content_languages = response.headers().get_one("content-language");
assert_eq!(content_languages, Some("da"));
assert_eq!(response.into_string().unwrap(), "Fyr raketten af!");
}