tweak docs

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

src/error/mod.rs

@@ -29,15 +29,15 @@ pub type Result<T, E = Error> = std::result::Result<T, E>;
 #[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),
 }

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<Url> {
         &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<U, I>(&self, name: &str, elements: U) -> Result<url::Url, UrlGenerationError>
     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()
     }

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)?;