Describe the bug
A clear and concise description of what the bug is.
To Reproduce
Steps to reproduce the behavior:
from fastapi import FastAPI
from starlette.requests import Request
@app.get('/myblog/posts/{id}/{slug:path}')
async def view_post(request: Request, id:int, slug: str):
return dict(request.path_params)
http://127.0.0.1:8000/myblog/posts/42/my_article_is_very_insightful/this_could_be_anything/whatever{"detail":[{"loc":["query","slug"],"msg":"field required","type":"value_error.missing"}]}
A few things to notice:
view_post(request: Request, id:int): (no slug:str parameter) causes the error to go away{"id":"42","slug":"my_article_is_very_insightful/this_could_be_anything/whatever"}
request.query_params object above (which I imagine comes from Starlette because id appears to be a string) still contains the value for slug, meaning it was recognized at some point but FastAPI didn't catch it.'/myblog/posts/{id}/{slug}' (no path converter) fixes the issue as long as the slug does not contain any slashesExpected behavior
In the example provided above, the route should work the exact same way whether or not slug:str is in the function signature.
Environment:
While most Starlette convertors are unlikely to be of much use to FastAPI applications (because of the type validation already being performed by FastAPI) ((as well as being undocumented)), the lack of a path wildcard in FastAPI means there still remains this last use case for them. I wouldn't personally mind if this issuewas closed in favor of a different implementation of path wildcards, although the contents of request.path_params raise some interesting questions regarding the way in which FastAPI currently performs route parameter extraction and why there appears to be a mismatch between the request object and that FastAPI sees.
Does that help ?
https://github.com/euri10/fastapi/blob/slugbug/tests/test_slug.py
I think the main conflict we will find here is that there's no way to declare that in OpenAPI: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#pathTemplating
I think that FastAPI's implementation of routing is there from a pre-convertors version in Starlette.
We might think about a workaround, or having custom handling for convertors, although I'm not sure how would we represent that in OpenAPI. For example, there's no way to pass an actual path with Swagger UI. It converts a/b/c to a%2Fb%2Fc.
A current workaround is to not use FastAPI's validation and use Starlette's normal @app.route('/myblog/posts/{id}/{slug:path}'). But in that case you would have to handle validation, etc, by hand.
Ah, right, FastAPI is primarily about OpenAPI, I probably shouldn't try doing something like this in the first place (since it's more of a HTML page routing thing and that's what I was intending to use this for) and instead use Starlette directly.
Actually, that should probably be mentionned in the templating part of the doc, since there are fairly high chances that the people looking at this part are intending to use it for more traditional HTML responses.
EDIT: To be clear, I mean that the doc should mention that FastAPI should probably be used strictly for API purposes, and that if you want an API in conjunction with a web page, a Starlette root application with FastAPI mounted as an Endpoint would probably be preferable. FastAPI probably shouldn't try fixing this specofic "bug", because it would go against its goal of being a pythonic way of generating OpenAPI servers and definitions.
@sm-Fifteen it was just solved by @euri10 in #234. It is released in FastAPI 0.22.0.
It works with a/b/c and a%2Fb%2Fc, so, it ends up not being a problem with OpenAPI (even if it's a bit of kind of cheating).
To clarify a bit, FastAPI is made around APIs, and optimized for them, but still, the intention is to support most of the use cases. And the reason of being based in Starlette is to be able to inherit the awesome Starlette features.
So, the idea is that you should be able to do with FastAPI everything you can do with Starlette. And if you build APIs, you get extra benefits.
Soon custom converters available : https://github.com/encode/starlette/pull/458
Looking into the OpenAPI spec further, it would appear that the correct way to do something like this according to both OpenAPI 3.0 and RFC 6570 should actually be something closer to /myblog/posts/{id}/{+slug}, which would be translated in the path parameters as allowReserved: true (that spec is full of surprises!).
Should we be using this instead of Flask-style path variable convertors?
That's a very interesting finding @sm-Fifteen .
So, OpenAPI defines a way to allowReserved: true: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#fixed-fields-10 but it only applies to parameters in query. Not in path.
The example in Swagger you referenced says what are the convertor equivalences, but not where they are allowed.
Still, even though Swagger started all this, the spec lives now at: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md. So, the examples and docs at Swagger are nice and help, but in case of doubts (as this one), the "single source of truth" is the spec.
I see the example you found is cited in this issue: https://github.com/OAI/OpenAPI-Specification/issues/1288 and they discuss a bit more about styles, although not specifically about path parameters with reserved characters.
For reserved characters in path parameters (exactly what we are discussing here), there's this issue, and they discuss specifically the use of /myblog/posts/{id}/{+slug}: https://github.com/OAI/OpenAPI-Specification/issues/1840:
It is specifically disallowed on path parameters [...]
Allowing/handling /myblog/posts/{id}/{+slug} would not be spec-compliant, apart from the extra effort to parse it, etc... :sweat_smile:
I think for now we should stick to the current solution, with the :path Starlette convertor, only for this specific case.
For the rest of the cases, we can use the normal parameter declarations with standard Python types.
Oh, yeah, I get that it's not how you'd be writing your routes in an OpenAPI definition. My point is that the various route and query parameter options are based off of RFC 6570, meaning that a lot of complex route formats that OpenAPI can accept can be expressed in one URL template (and not specifically just allowReserved: true/{+path}, there could be interesting uses for style: matrix/{;list} or style: label/{.list}, not to mention the possibility of adding the query parameters to the route directly to avoid the need for disambiguation between body and query parameters).
{mypath:path} isn't exactly a lot more OpenAPI-compliant than {+mypath}, I just figured that most starlette path convertors are ({id:int}, {distance:float}) already kind of at odds with the way FastAPI handles type conversion with path just being the odd one out, and since most (if not all) OpenAPI parameter formats have an RFC 6570 equivalent while FastAPI tries to put as much information as possible in its function signatures and route annotations that this could be a good way to move parameter properties there.
With that said, though, this might be getting off-topic from the original issue.
Hehe yep.
Supporting the other templating formats might be something that we can think about in the future. For now I think we can use the current ideas.
And yes, {mypath:path} is not OpenAPI-compliant either... But given that it's something not "officially" supported by OpenAPI, keeping the Starlette implementation should work for these use cases, at least for now.
As this is (more or less) solved now, should we close this issue then?
Certainly, I'll probably just open a separate feature request for RFC 6570 templates at some point later.
Cool, thanks!