From ad8d80907b6d481cc82d9125217a624a7dfc3308 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Wed, 28 Apr 2021 04:54:06 -0700 Subject: [PATCH] Remove unused reason phrase in 'Status'. Closes #534. Co-authored-by: YetAnotherMinion --- core/codegen/src/http_codegen.rs | 6 +- core/codegen/src/lib.rs | 2 +- core/codegen/tests/responder.rs | 2 +- core/http/src/status.rs | 276 +++++++++++++++------------ core/lib/src/response/response.rs | 40 ---- examples/error-handling/src/main.rs | 4 +- examples/error-handling/src/tests.rs | 4 +- 7 files changed, 162 insertions(+), 172 deletions(-) diff --git a/core/codegen/src/http_codegen.rs b/core/codegen/src/http_codegen.rs index 675accd5..a479a527 100644 --- a/core/codegen/src/http_codegen.rs +++ b/core/codegen/src/http_codegen.rs @@ -26,14 +26,14 @@ impl FromMeta for Status { return Err(meta.value_span().error("status must be in range [100, 599]")); } - Ok(Status(http::Status::raw(num as u16))) + Ok(Status(http::Status::new(num as u16))) } } impl ToTokens for Status { fn to_tokens(&self, tokens: &mut TokenStream) { - let (code, reason) = (self.0.code, self.0.reason); - tokens.extend(quote!(rocket::http::Status { code: #code, reason: #reason })); + let code = self.0.code; + tokens.extend(quote!(rocket::http::Status { code: #code })); } } diff --git a/core/codegen/src/lib.rs b/core/codegen/src/lib.rs index a59cb647..efcef005 100644 --- a/core/codegen/src/lib.rs +++ b/core/codegen/src/lib.rs @@ -303,7 +303,7 @@ route_attribute!(options => Method::Options); /// /// #[catch(default)] /// fn default(status: Status, req: &Request) -> String { -/// format!("{} - {} ({})", status.code, status.reason, req.uri()) +/// format!("{} ({})", status, req.uri()) /// } /// ``` /// diff --git a/core/codegen/tests/responder.rs b/core/codegen/tests/responder.rs index e4aa84d2..98b0e241 100644 --- a/core/codegen/tests/responder.rs +++ b/core/codegen/tests/responder.rs @@ -53,7 +53,7 @@ async fn responder_foo() { .respond_to(req) .expect("response okay"); - assert_eq!(r.status(), Status::raw(105)); + assert_eq!(r.status().code, 105); assert_eq!(r.content_type(), Some(ContentType::JSON)); assert_eq!(r.body_mut().to_string().await.unwrap(), "goodbye"); } diff --git a/core/http/src/status.rs b/core/http/src/status.rs index 5da1e2ab..5104f04c 100644 --- a/core/http/src/status.rs +++ b/core/http/src/status.rs @@ -42,20 +42,43 @@ impl StatusClass { class_check_fn!(is_unknown, "`Unknown`.", Unknown); } -/// Structure representing an HTTP status: an integer code and a reason phrase. +/// Structure representing an HTTP status: an integer code. /// -/// # Usage +/// A `Status` should rarely be created directly. Instead, an associated +/// constant should be used; one is declared for every status defined in the +/// HTTP standard. If a custom status code _must_ be created, note that it is +/// not possible to set a custom reason phrase. /// -/// Status classes should rarely be created directly. Instead, an associated -/// constant should be used; one is declared for every status defined -/// in the HTTP standard. +/// ```rust +/// # extern crate rocket; +/// use rocket::http::Status; +/// +/// // Create a status from a known constant. +/// let ok = Status::Ok; +/// assert_eq!(ok.code, 200); +/// assert_eq!(ok.reason(), Some("OK")); +/// +/// let not_found = Status::NotFound; +/// assert_eq!(not_found.code, 404); +/// assert_eq!(not_found.reason(), Some("Not Found")); +/// +/// // Or from a status code: `reason()` returns the phrase when known. +/// let gone = Status::new(410); +/// assert_eq!(gone.code, 410); +/// assert_eq!(gone.reason(), Some("Gone")); +/// +/// // `reason()` returns `None` when unknown. +/// let custom = Status::new(599); +/// assert_eq!(custom.code, 599); +/// assert_eq!(custom.reason(), None); +/// ``` /// /// # Responding /// /// To set a custom `Status` on a response, use a [`response::status`] -/// responder. Alternatively, respond with `(Status, T)` where `T: Responder`, but -/// note that the response may require additional headers to be valid as -/// enforced by the types in [`response::status`]. +/// responder, which enforces correct status-based responses. Alternatively, +/// respond with `(Status, T)` where `T: Responder`, but beware that the +/// response may be invalid if it requires additional headers. /// /// ```rust /// # extern crate rocket; @@ -69,47 +92,10 @@ impl StatusClass { /// ``` /// /// [`response::status`]: ../response/status/index.html -/// -/// ## Example -/// -/// A status of `200 OK` can be instantiated via the `Ok` constant: -/// -/// ```rust -/// # extern crate rocket; -/// use rocket::http::Status; -/// -/// # #[allow(unused_variables)] -/// let ok = Status::Ok; -/// ``` -/// -/// A status of `404 Not Found` can be instantiated via the `NotFound` constant: -/// -/// ```rust -/// # extern crate rocket; -/// use rocket::http::Status; -/// -/// # #[allow(unused_variables)] -/// let not_found = Status::NotFound; -/// ``` -/// -/// The code and phrase can be retrieved directly: -/// -/// ```rust -/// # extern crate rocket; -/// use rocket::http::Status; -/// -/// let not_found = Status::NotFound; -/// -/// assert_eq!(not_found.code, 404); -/// assert_eq!(not_found.reason, "Not Found"); -/// assert_eq!(not_found.to_string(), "404 Not Found".to_string()); -/// ``` #[derive(Debug, Clone, Copy)] pub struct Status { /// The HTTP status code associated with this status. pub code: u16, - /// The HTTP reason phrase associated with this status. - pub reason: &'static str } impl Default for Status { @@ -123,12 +109,66 @@ macro_rules! ctrs { $( #[doc="[`Status`] with code "] #[doc=$code_str] - #[doc=" and reason "] - #[doc=$reason] - #[doc="."] + #[doc="."] #[allow(non_upper_case_globals)] - pub const $name: Status = Status { code: $code, reason: $reason }; - )+ + pub const $name: Status = Status { code: $code }; + )+ + + /// Creates a new `Status` with `code`. This should be used _only_ to + /// construct non-standard HTTP statuses. Use an associated constant for + /// standard statuses. + /// + /// # Example + /// + /// Create a custom `299` status: + /// + /// ```rust + /// # extern crate rocket; + /// use rocket::http::Status; + /// + /// let custom = Status::new(299); + /// assert_eq!(custom.code, 299); + /// ``` + pub const fn new(code: u16) -> Status { + Status { code } + } + + /// Returns the class of a given status. + /// + /// # Example + /// + /// ```rust + /// # extern crate rocket; + /// use rocket::http::{Status, StatusClass}; + /// + /// let processing = Status::Processing; + /// assert_eq!(processing.class(), StatusClass::Informational); + /// + /// let ok = Status::Ok; + /// assert_eq!(ok.class(), StatusClass::Success); + /// + /// let see_other = Status::SeeOther; + /// assert_eq!(see_other.class(), StatusClass::Redirection); + /// + /// let not_found = Status::NotFound; + /// assert_eq!(not_found.class(), StatusClass::ClientError); + /// + /// let internal_error = Status::InternalServerError; + /// assert_eq!(internal_error.class(), StatusClass::ServerError); + /// + /// let custom = Status::new(600); + /// assert_eq!(custom.class(), StatusClass::Unknown); + /// ``` + pub const fn class(self) -> StatusClass { + match self.code / 100 { + 1 => StatusClass::Informational, + 2 => StatusClass::Success, + 3 => StatusClass::Redirection, + 4 => StatusClass::ClientError, + 5 => StatusClass::ServerError, + _ => StatusClass::Unknown + } + } /// Returns a Status given a standard status code `code`. If `code` is /// not a known status code, `None` is returned. @@ -151,88 +191,78 @@ macro_rules! ctrs { /// # extern crate rocket; /// use rocket::http::Status; /// - /// let not_found = Status::from_code(600); - /// assert!(not_found.is_none()); + /// let unknown = Status::from_code(600); + /// assert!(unknown.is_none()); /// ``` - pub fn from_code(code: u16) -> Option { + pub const fn from_code(code: u16) -> Option { match code { $($code => Some(Status::$name),)+ _ => None } } + + /// Returns the canonical reason phrase if `self` corresponds to a + /// canonical, known status code. Otherwise, returns `None`. + /// + /// # Example + /// + /// Reason phrase from a known `code`: + /// + /// ```rust + /// # extern crate rocket; + /// use rocket::http::Status; + /// + /// assert_eq!(Status::Created.reason(), Some("Created")); + /// assert_eq!(Status::new(200).reason(), Some("OK")); + /// ``` + /// + /// Absent phrase from an unknown `code`: + /// + /// ```rust + /// # extern crate rocket; + /// use rocket::http::Status; + /// + /// assert_eq!(Status::new(499).reason(), None); + /// ``` + pub const fn reason(&self) -> Option<&'static str> { + match self.code { + $($code => Some($reason),)+ + _ => None + } + } + + /// Returns the canonical reason phrase if `self` corresponds to a + /// canonical, known status code, or an unspecified but relevant reason + /// phrase otherwise. + /// + /// # Example + /// + /// ```rust + /// # extern crate rocket; + /// use rocket::http::Status; + /// + /// assert_eq!(Status::NotFound.reason_lossy(), "Not Found"); + /// assert_eq!(Status::new(100).reason_lossy(), "Continue"); + /// assert!(!Status::new(699).reason_lossy().is_empty()); + /// ``` + pub const fn reason_lossy(&self) -> &'static str { + if let Some(lossless) = self.reason() { + return lossless; + } + + match self.class() { + StatusClass::Informational => "Informational", + StatusClass::Success => "Success", + StatusClass::Redirection => "Redirection", + StatusClass::ClientError => "Client Error", + StatusClass::ServerError => "Server Error", + StatusClass::Unknown => "Unknown" + } + } }; } impl Status { - /// Creates a new `Status` with `code` and `reason`. This should be used _only_ - /// to construct non-standard HTTP statuses. Use an associated constant for - /// standard statuses. - /// - /// # Example - /// - /// Create a custom `299 Somewhat Successful` status: - /// - /// ```rust - /// # extern crate rocket; - /// use rocket::http::Status; - /// - /// let custom = Status::new(299, "Somewhat Successful"); - /// assert_eq!(custom.to_string(), "299 Somewhat Successful".to_string()); - /// ``` - #[inline(always)] - pub const fn new(code: u16, reason: &'static str) -> Status { - Status { code, reason } - } - - /// Returns the class of a given status. - /// - /// # Example - /// - /// ```rust - /// # extern crate rocket; - /// use rocket::http::{Status, StatusClass}; - /// - /// let processing = Status::Processing; - /// assert_eq!(processing.class(), StatusClass::Informational); - /// - /// let ok = Status::Ok; - /// assert_eq!(ok.class(), StatusClass::Success); - /// - /// let see_other = Status::SeeOther; - /// assert_eq!(see_other.class(), StatusClass::Redirection); - /// - /// let not_found = Status::NotFound; - /// assert_eq!(not_found.class(), StatusClass::ClientError); - /// - /// let internal_error = Status::InternalServerError; - /// assert_eq!(internal_error.class(), StatusClass::ServerError); - /// - /// let custom = Status::new(600, "Bizarre"); - /// assert_eq!(custom.class(), StatusClass::Unknown); - /// ``` - pub fn class(self) -> StatusClass { - match self.code / 100 { - 1 => StatusClass::Informational, - 2 => StatusClass::Success, - 3 => StatusClass::Redirection, - 4 => StatusClass::ClientError, - 5 => StatusClass::ServerError, - _ => StatusClass::Unknown - } - } - - /// Returns a status from a given status code. If the status code is a - /// standard code, then the reason phrase is populated accordingly. - /// Otherwise the reason phrase is set to "". - #[inline] - #[doc(hidden)] - pub fn raw(code: u16) -> Status { - match Status::from_code(code) { - Some(status) => status, - None => Status::new(code, "") - } - } - ctrs! { 100, "100", Continue => "Continue", 101, "101", SwitchingProtocols => "Switching Protocols", @@ -300,7 +330,7 @@ impl Status { impl fmt::Display for Status { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{} {}", self.code, self.reason) + write!(f, "{} {}", self.code, self.reason_lossy()) } } diff --git a/core/lib/src/response/response.rs b/core/lib/src/response/response.rs index e580973c..fdba8864 100644 --- a/core/lib/src/response/response.rs +++ b/core/lib/src/response/response.rs @@ -106,27 +106,6 @@ impl<'r> Builder<'r> { self } - /// Sets the status of the `Response` being built to a custom status - /// constructed from the `code` and `reason` phrase. - /// - /// # Example - /// - /// ```rust - /// use rocket::Response; - /// use rocket::http::Status; - /// - /// let response = Response::build() - /// .raw_status(699, "Alien Encounter") - /// .finalize(); - /// - /// assert_eq!(response.status(), Status::new(699, "Alien Encounter")); - /// ``` - #[inline(always)] - pub fn raw_status(&mut self, code: u16, reason: &'static str) -> &mut Builder<'r> { - self.response.set_raw_status(code, reason); - self - } - /// Adds `header` to the `Response`, replacing any header with the same name /// that already exists in the response. If multiple headers with /// the same name exist, they are all removed, and only the new header and @@ -544,25 +523,6 @@ impl<'r> Response<'r> { self.headers().get_one("Content-Type").and_then(|v| v.parse().ok()) } - /// Sets the status of `self` to a custom `status` with status code `code` - /// and reason phrase `reason`. This method should be used sparingly; prefer - /// to use [set_status](#method.set_status) instead. - /// - /// # Example - /// - /// ```rust - /// use rocket::Response; - /// use rocket::http::Status; - /// - /// let mut response = Response::new(); - /// response.set_raw_status(699, "Tripped a Wire"); - /// assert_eq!(response.status(), Status::new(699, "Tripped a Wire")); - /// ``` - #[inline(always)] - pub fn set_raw_status(&mut self, code: u16, reason: &'static str) { - self.status = Some(Status::new(code, reason)); - } - /// Returns an iterator over the cookies in `self` as identified by the /// `Set-Cookie` header. Malformed cookies are skipped. /// diff --git a/examples/error-handling/src/main.rs b/examples/error-handling/src/main.rs index 85f4ec96..a82b73f3 100644 --- a/examples/error-handling/src/main.rs +++ b/examples/error-handling/src/main.rs @@ -13,7 +13,7 @@ fn hello(name: &str, age: i8) -> String { #[get("/")] fn forced_error(code: u16) -> Status { - Status::raw(code) + Status::new(code) } #[catch(404)] @@ -39,7 +39,7 @@ fn sergio_error() -> &'static str { #[catch(default)] fn default_catcher(status: Status, req: &Request<'_>) -> status::Custom { - let msg = format!("{} - {} ({})", status.code, status.reason, req.uri()); + let msg = format!("{} ({})", status, req.uri()); status::Custom(status, msg) } diff --git a/examples/error-handling/src/tests.rs b/examples/error-handling/src/tests.rs index 3c5195f4..db5ac18d 100644 --- a/examples/error-handling/src/tests.rs +++ b/examples/error-handling/src/tests.rs @@ -30,9 +30,9 @@ fn forced_error() { assert_eq!(response.into_string().unwrap(), expected.1); let request = client.get("/533"); - let expected = super::default_catcher(Status::raw(533), request.inner()); + let expected = super::default_catcher(Status::new(533), request.inner()); let response = request.dispatch(); - assert_eq!(response.status(), Status::raw(533)); + assert_eq!(response.status(), Status::new(533)); assert_eq!(response.into_string().unwrap(), expected.1); let request = client.get("/700");