Skip to content

Conversation

lazulit3
Copy link

@lazulit3 lazulit3 commented Dec 20, 2024

When testing my middleware in another project I noticed that rustify will sometimes produce extraneous // in the Uri produced by http::build_url().

This is due to some unhandled cases where base path, trailing slashes, and path with leading slashes may result in adding empty path segments to the joined path.

Testing & Failure Cases

This PR adds 9 different test cases. Prior to the fix, 3 were failing.

You can checkout f7ab901 and cargo test to reproduce.

Example

This is an example similar to how I stumbled on this:

    use rustify::Endpoint;
    use rustify_derive::Endpoint;

    #[derive(Endpoint)]
    #[endpoint(path = "/endpoint")]
    struct ExampleEndpoint {
        #[endpoint(query)]
        hello: String,
    }

    #[test]
    fn rustify_build_url_extra_slash_example() {
        let endpoint = ExampleEndpoint {
            hello: "world".into(),
        };
        let expected = "https://192.0.2.2/api/endpoint?hello=world";

        let request_uri = endpoint.request("https://192.0.2.2/api").unwrap();

        assert_eq!(request_uri.uri(), expected);
    }
assertion `left == right` failed
  left: https://192.0.2.2/api//endpoint?hello=world
 right: "https://192.0.2.2/api/endpoint?hello=world"

This issue doesn't really impact actual HTTP requesting / library usage, but may surprise developers trying to write tests. :)

Fix

build_url() is adjust to better avoid empty path segments by:

  • popping off empty segment on the base URL path (for cases where it had a trailing slash)
  • stripping the leading / from the endpoint/relative path (to avoid adding an empty path segment)

Adds test cases testing `build_url()` with different combinations of leading and trailing slashes between the `base` and `path`.

This is for identifying bugs where an empty path segment results in a double `//` like `https://192.0.2.2/base/path//foobar`
Fixes approach to joining `base` and `path` in `http::build_url()` so that
trailing or leading slashes (`/`) do not result in empty path segments (`//` in produced `Url`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant