poem-web / poem

A full-featured and easy-to-use web framework with the Rust programming language.
Apache License 2.0
3.63k stars 295 forks source link

[poem-openapi] Path parameters in `prefix_path` do not generate the correct openapi specification #826

Open Zstorm999 opened 5 months ago

Zstorm999 commented 5 months ago

Hello !

I have an api specification of the form :

#[OpenApi(prefix_path="/hello/:name")]
impl Api {
    #[oai(path = "/:surname", method = "get")]
    async fn get(...) -> ... { ... }
}

Note that I have path parameters in both prefix_path and path.

Expected Behavior

I expect the parameter name to be included in the api spec, so the generated spec should be something like

"paths": {
    "/hello/{name}/{surname}": {
      "get": {
        "parameters": [
          {
            "name": "name",
            ...
          },
          {
            "name": "surname",
            ...
          }
        ],
}}}

Actual Behavior

Instead, what is being generated is

"paths": {
    "/hello/:name/{surname}": {
      "get": {
        "parameters": [
          {
            "name": "name",
            ...
          },
          {
            "name": "surname",
            ...
          }
        ],
}}}

This causes problems with api viewers (tested with redoc and swagger-ui), as they do not generate the correct request.

Expected:

curl -X GET http://localhost/api/hello/myname/mysurname

Actual

curl -X GET http://localhost/api/hello/:name/mysurname

Additional testing

For the record, I tested the following api construction in Rust:

#[OpenApi(prefix_path="/hello/{name}")]
impl Api {
    #[oai(path = "/:surname", method = "get")]
    async fn get(...) -> ... { ... }
}

This generates the correct specification, but poem is then unable to correctly use the route and I get a 404 error when trying to access it.

Specifications

Full code example ```rust use poem::{listener::TcpListener, Route, Server}; use poem_openapi::{param::{Path, Query}, payload::PlainText, OpenApi, OpenApiService}; struct Api; #[OpenApi(prefix_path="/hello/:name")] impl Api { #[oai(path = "/:surname", method = "get")] async fn index(&self, name: Path, surname: Path) -> PlainText { let name = name.0; let surname = surname.0; PlainText(format!("hello, {name} {surname}!")) } } #[tokio::main] async fn main() -> Result<(), std::io::Error> { if std::env::var_os("RUST_LOG").is_none() { std::env::set_var("RUST_LOG", "poem=debug"); } tracing_subscriber::fmt::init(); let api_service = OpenApiService::new(Api, "Hello World", "1.0").server("http://localhost:3000/api"); let ui = api_service.swagger_ui(); let spec = api_service.spec(); println!("{}", spec); Server::new(TcpListener::bind("0.0.0.0:3000")) .run(Route::new().nest("/api", api_service).nest("/", ui)) .await } ```
Zstorm999 commented 5 months ago

So I found the problem at this line: https://github.com/poem-web/poem/blob/83413facbe4e0c4e8d4e2c72dbc8ca23361e6dd9/poem-openapi-derive/src/api.rs#L197

When the full path is generated below, no conversion is applied to the prefix_path, only to the oai_path. This means the path is half-standardized in the final output.

Zstorm999 commented 1 month ago

So I searched a bit for this problem. There is indeed no generated oai path for the prefix_path attribute. However, due to this line:

https://github.com/poem-web/poem/blob/f81424dedeec44cbdf4dbe2e828b0d3bbf579486/poem-openapi-derive/src/api.rs#L26-L27

The prefix_path can actually be a variable, and not just a string literal. This makes it impossible to validate that path unless a breaking change is introduced requirering it to be a string literal just like the regular path.

There is already a test for this case:

https://github.com/poem-web/poem/blob/f81424dedeec44cbdf4dbe2e828b0d3bbf579486/poem-openapi/tests/api.rs#L133-L144