From eca2fcda4a89334813b532b0835bb222ac21ad81 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 5 Dec 2021 04:37:41 +0000 Subject: [PATCH] tweak docs --- CHANGES.md | 2 ++ src/error/mod.rs | 8 +++---- src/request.rs | 54 +++++++++++++++++++++--------------------------- src/rmap.rs | 26 ++++++++++++++--------- 4 files changed, 46 insertions(+), 44 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 78aa729df..1b108fee6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,12 +12,14 @@ * Rename `Accept::{mime_precedence => ranked}`. [#2480] * Rename `Accept::{mime_preference => preference}`. [#2480] * Un-deprecate `App::data_factory`. [#2484] +* `HttpRequest::url_for` no longer constructs URLs with query or fragment components. [#2430] ### Fixed * Accept wildcard `*` items in `AcceptLanguage`. [#2480] * Re-exports `dev::{BodySize, MessageBody, SizedStream}`. They are exposed through the `body` module. [#2468] * Typed headers containing lists that require one or more items now enforce this minimum. [#2482] +[#2430]: https://github.com/actix/actix-web/pull/2430 [#2468]: https://github.com/actix/actix-web/pull/2468 [#2480]: https://github.com/actix/actix-web/pull/2480 [#2482]: https://github.com/actix/actix-web/pull/2482 diff --git a/src/error/mod.rs b/src/error/mod.rs index ca84a427b..46d0dccc6 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -29,15 +29,15 @@ pub type Result = std::result::Result; #[derive(Debug, PartialEq, Display, Error, From)] #[non_exhaustive] pub enum UrlGenerationError { - /// Resource not found + /// Resource not found. #[display(fmt = "Resource not found")] ResourceNotFound, - /// Not all url paramters covered - #[display(fmt = "Not all url parameters covered")] + /// Not all URL parameters covered. + #[display(fmt = "Not all URL parameters covered")] NotEnoughElements, - /// URL parse error + /// URL parse error. #[display(fmt = "{}", _0)] ParseError(UrlParseError), } diff --git a/src/request.rs b/src/request.rs index 2a0178ccd..58222da47 100644 --- a/src/request.rs +++ b/src/request.rs @@ -100,7 +100,7 @@ impl HttpRequest { &self.head().headers } - /// The target path of this Request. + /// The target path of this request. #[inline] pub fn path(&self) -> &str { self.head().uri.path() @@ -108,26 +108,22 @@ impl HttpRequest { /// The query string in the URL. /// - /// E.g., id=10 + /// Example: `id=10` #[inline] pub fn query_string(&self) -> &str { self.uri().query().unwrap_or_default() } - /// Get a reference to url parameters container + /// Returns a reference to the URL parameters container. /// - /// A url parameter is specified in the form `{identifier}`, - /// where the identifier can be used later in a request handler to - /// access the matched value for that parameter. - /// - /// - /// # Percent Enconding and Url Paramters - /// - /// Because each url paramter is able to capture multiple path segments, - /// both `["%2F", "%25"]` found in the request uri are not decoded into `["/", "%"]` - /// in order to preserve path segment boundaries. - /// If a url paramter is expected to contain these characters, then it is on the user to decode them. + /// A url parameter is specified in the form `{identifier}`, where the identifier can be used + /// later in a request handler to access the matched value for that parameter. /// + /// # Percent Encoding and URL Parameters + /// Because each URL parameter is able to capture multiple path segments, both `["%2F", "%25"]` + /// found in the request URI are not decoded into `["/", "%"]` in order to preserve path + /// segment boundaries. If a url parameter is expected to contain these characters, then it is + /// on the user to decode them. #[inline] pub fn match_info(&self) -> &Path { &self.inner.path @@ -169,31 +165,29 @@ impl HttpRequest { self.head().extensions_mut() } - /// Generate url for named resource + /// Generates URL for a named resource. /// - /// This substitutes in sequence all url parameters that appear in the resource itself - /// and in parent [scopes](crate::web::scope), if any. + /// This substitutes in sequence all URL parameters that appear in the resource itself and in + /// parent [scopes](crate::web::scope), if any. /// /// It is worth noting that the characters `['/', '%']` are not escaped and therefore a single - /// url parameter may expand into multiple path segments and `elements` - /// can be percent-encoded beforehand without worrying about double encoding. - /// Any other character that is not valid in url ath context is escaped using percent-encoding. + /// URL parameter may expand into multiple path segments and `elements` can be percent-encoded + /// beforehand without worrying about double encoding. Any other character that is not valid in + /// a URL path context is escaped using percent-encoding. /// + /// # Examples /// ``` /// # use actix_web::{web, App, HttpRequest, HttpResponse}; - /// # /// fn index(req: HttpRequest) -> HttpResponse { - /// let url = req.url_for("foo", &["1", "2", "3"]); // <- generate url for "foo" resource + /// let url = req.url_for("foo", &["1", "2", "3"]); // <- generate URL for "foo" resource /// HttpResponse::Ok().into() /// } /// - /// fn main() { - /// let app = App::new() - /// .service(web::resource("/test/{one}/{two}/{three}") - /// .name("foo") // <- set resource name, then it could be used in `url_for` - /// .route(web::get().to(|| HttpResponse::Ok())) - /// ); - /// } + /// let app = App::new() + /// .service(web::resource("/test/{one}/{two}/{three}") + /// .name("foo") // <- set resource name so it can be used in `url_for` + /// .route(web::get().to(|| HttpResponse::Ok())) + /// ); /// ``` pub fn url_for(&self, name: &str, elements: U) -> Result where @@ -212,8 +206,8 @@ impl HttpRequest { self.url_for(name, &NO_PARAMS) } - #[inline] /// Get a reference to a `ResourceMap` of current application. + #[inline] pub fn resource_map(&self) -> &ResourceMap { self.app_state().rmap() } diff --git a/src/rmap.rs b/src/rmap.rs index a7f1884bb..432eaf83c 100644 --- a/src/rmap.rs +++ b/src/rmap.rs @@ -1,13 +1,14 @@ -use std::borrow::Cow; -use std::cell::RefCell; -use std::rc::{Rc, Weak}; +use std::{ + borrow::Cow, + cell::RefCell, + rc::{Rc, Weak}, +}; use actix_router::ResourceDef; use ahash::AHashMap; use url::Url; -use crate::error::UrlGenerationError; -use crate::request::HttpRequest; +use crate::{error::UrlGenerationError, request::HttpRequest}; #[derive(Clone, Debug)] pub struct ResourceMap { @@ -104,17 +105,22 @@ impl ResourceMap { .ok_or(UrlGenerationError::NotEnoughElements)?; let (base, path): (Cow<'_, _>, _) = if path.starts_with('/') { + // build full URL from connection info parts and resource path let conn = req.connection_info(); - let base = format!("{}://{}", conn.scheme(), conn.host(),).into(); - (base, path.as_str()) + let base = format!("{}://{}", conn.scheme(), conn.host()); + (Cow::Owned(base), path.as_str()) } else { - // external resource + // external resource; third slash would be the root slash in the path let third_slash_index = path .char_indices() .filter_map(|(i, c)| (c == '/').then(|| i)) .nth(2) - .unwrap_or(path.len()); - (path[..third_slash_index].into(), &path[third_slash_index..]) + .unwrap_or_else(|| path.len()); + + ( + Cow::Borrowed(&path[..third_slash_index]), + &path[third_slash_index..], + ) }; let mut url = Url::parse(&base)?;