Fastapi: [QUESTION] How to catch misspelled/unknown parameters?

Created on 30 Mar 2020  路  12Comments  路  Source: tiangolo/fastapi

How can I catch misspelled or unknown query parameters? For instance:

from fastapi import FastAPI

app = FastAPI()

@app.get("/")
def root(myparam: int = 0):
    return {'myparam': myparam}
$ curl http://127.0.0.1:8000/?param=5
{"myparam":0}

It seems bad to silently accept the unknown param parameter. The caller wouldn't be aware that they misspelled the parameter and that it remained at its default value.

question

Most helpful comment

Thanks for the discussion here everyone.

FastAPI is based on OpenAPI, which is based on JSON Schema. That by default allows extra values and defines validation for the ones provided. So forbidding extra parameters wouldn't be acceptable as a default.

There are also many systems that add additional query parameters being used or not, e.g. Google Analytics, Facebook, etc. So I don't think it would work well for many use cases.


But on the other side, I see this could be useful in some cases (like yours), so I think we could have a path operation decorator parameter allow_extra_parameters=True that conditionally triggers your proposed change.

If you want, you could create a PR with that :nerd_face: :rocket:

All 12 comments

I think that's pretty typical for REST api's and isn't something I would expect FastAPI to prevent.

Example
https://petstore.swagger.io/v2/store/inventory?foo=bar

I suppose that's true but given fastapi's focus on parameter value validation I had expected it to also validate the parameter keys as well. I think the fact that the parameters are defined as part of the function signature led me to assume you would get an error just as if you called a regular python function with an invalid keyword argument.

Are there situations where you would _not_ want to assert that all parameter keys are valid?

You can try this one:

from fastapi import FastAPI, Query

app = FastAPI()

@app.get("/")
def root(myparam: int = Query(...)):
    return {'myparam': myparam}

I can't understand that why set a default value for must required field?

Here is a patch that I think implements what I am looking for:

diff --git a/fastapi/dependencies/utils.py b/fastapi/dependencies/utils.py
index 43ab4a0..577961f 100644
--- a/fastapi/dependencies/utils.py
+++ b/fastapi/dependencies/utils.py
@@ -35,7 +35,7 @@ from fastapi.utils import (
 )
 from pydantic import BaseConfig, BaseModel, create_model
 from pydantic.error_wrappers import ErrorWrapper
-from pydantic.errors import MissingError
+from pydantic.errors import MissingError, ExtraError
 from pydantic.utils import lenient_issubclass
 from starlette.background import BackgroundTasks
 from starlette.concurrency import run_in_threadpool
@@ -529,6 +529,12 @@ async def solve_dependencies(
     values.update(header_values)
     values.update(cookie_values)
     errors += path_errors + query_errors + header_errors + cookie_errors
+
+    dependant_names = {param.name for param in dependant.query_params}
+    for param in request.query_params:
+        if param not in dependant_names:
+            errors.append(ErrorWrapper(ExtraError(), loc=("query", param)))
+
     if dependant.body_params:
         (
             body_values,
from fastapi import FastAPI

app = FastAPI()

@app.get("/")
def root(myparam: int = 0):
    return {'myparam': myparam}



md5-a1dcb42fe96d1fbeed53f0a377752ad9



$ curl http://127.0.0.1:8000/?param=5
{"detail":[{"loc":["query","param"],"msg":"extra fields not permitted","type":"value_error.extra"}]}

Here is a patch that I think implements what I am looking for:

diff --git a/fastapi/dependencies/utils.py b/fastapi/dependencies/utils.py
index 43ab4a0..577961f 100644
--- a/fastapi/dependencies/utils.py
+++ b/fastapi/dependencies/utils.py
@@ -35,7 +35,7 @@ from fastapi.utils import (
 )
 from pydantic import BaseConfig, BaseModel, create_model
 from pydantic.error_wrappers import ErrorWrapper
-from pydantic.errors import MissingError
+from pydantic.errors import MissingError, ExtraError
 from pydantic.utils import lenient_issubclass
 from starlette.background import BackgroundTasks
 from starlette.concurrency import run_in_threadpool
@@ -529,6 +529,12 @@ async def solve_dependencies(
     values.update(header_values)
     values.update(cookie_values)
     errors += path_errors + query_errors + header_errors + cookie_errors
+
+    dependant_names = {param.name for param in dependant.query_params}
+    for param in request.query_params:
+        if param not in dependant_names:
+            errors.append(ErrorWrapper(ExtraError(), loc=("query", param)))
+
     if dependant.body_params:
         (
             body_values,
from fastapi import FastAPI

app = FastAPI()

@app.get("/")
def root(myparam: int = 0):
    return {'myparam': myparam}
$ curl http://127.0.0.1:8000/?param=5
{"detail":[{"loc":["query","param"],"msg":"extra fields not permitted","type":"value_error.extra"}]}

If need do this one, I think need a switch.
Not all api need this check,they need to be more compatible

If need do this one, I think need a switch.
Not all api need this check,they need to be more compatible

I'm not sure I fully understand, are you saying that there are existing users of fastapi who are depending on fastapi not checking for extra parameters?

I believe what you are suggesting is that fastapi users need to be able to disable this check because maybe clients, which they don't control, are sending extra params?

I agree that there should be a way to disable this check but I would argue that fastapi should be strict about the api spec by default to catch bugs quicker. A potential way to turn off this check could be a keyword argument in the decorator:

@app.get("/", allow_extra_fields=True)
def root(myparam: int = 0):
    return {'myparam': myparam}

In many cases, extra parameters are allowed.
For example google search, like:

https://www.google.com/search?q=fastapi&xxxx=123&idontknow=ok

And for me, the extra parameters is ok. I think need more people to discuss this feature.
By default, we should not check extra parameters, we can use a karg to allow it:

@app.get("/", disable_extra_fields=True)
def root(myparam: int = 0):
    return {'myparam': myparam}

And for me, the extra parameters is ok.

I agree.

I see no reason to check extra parameters by default. The pydantic model's config has a key to throw an error if additional data is passed to the model constructor, which is ignored by default. This is default behavior for many REST APIs to ignore all additional parameters (both in the query arguments and in the request body), since they simply do not need to use them. IMO, this is a rare case when you need to check that request contains only the required parameters and does not contain any missing or extra ones, so it should be (if it should exist at all) an additional parameter in the route decorator.

I agree that there should be a way to disable this check but I would argue that fastapi should be strict about the api spec by default to catch bugs quicker.

In fact, I don't quite understand what errors can be caused by the fact that you accept exrta parameters in the query string. Especially if you checked your query parameters with FastAPI validation features. All I can think of is that you use raw request.query_params in your route code, but this is not a very common case, and it can be checked in another way (for example, using an additional pydantic model).

I agree that there should be a way to disable this check but I would argue that fastapi should be strict about the api spec by default to catch bugs quicker.

In fact, I don't quite understand what errors can be caused by the fact that you accept exrta parameters in the query string.

The word "extra" is maybe not the best word to use, "unknown" would be better. I'm trying to catch people misspelling the optional query parameters on accident (which actually happened and burned a lot of time). As another example:

@app.get("/members")
def members(exclude_vip: bool = False):
       ....
GET /members?excludevip=1

It might not be obvious from the response that your parameter was ignore (maybe there are thousands of members and only a handful of vip members so either way you get back a huge list)

The intention of my patch is to catch these kind of bugs by default, just like we catch value type errors by default. If someone is sending truly extra params then I would argue that is the non-default exceptional case.

I agree there should be an option to only accept defined parameters. I've written many APIs and what generally happens is that a researcher will misspell a parameter in their code and will get incorrect results because they believe the parameter is being interpreted and used by the API and not silently dropped.

This can cause disastrous consequences for researchers. Imagine if a parameter named "exclude_false_positives" was a boolean value with a default of false and a researcher passed "exclude_fals_positives=true" and the API did not throw an error and the researcher used the data believing false positive had been excluded.

I get that it's nice for an API to be as flexible as possible, but if it gets too flexible like accepting unknown parameters, it can lead to confusion for the end users.

Also, an end-user usually always has an intention behind using a parameter. If they include something your API doesn't support, it's clearer to throw and error and then they can e-mail / contact you about it. Maybe you forgot to support the parameter? Maybe the end-user doesn't understand your documentation and you need to improve it? The only valid reason I can think of off the top of my head for adding parameters that the user knows doesn't exist is to cache-bust.

Just make it an initialization switch with a default to allow unknown parameters so it's backwards compatible?

Thanks for the discussion here everyone.

FastAPI is based on OpenAPI, which is based on JSON Schema. That by default allows extra values and defines validation for the ones provided. So forbidding extra parameters wouldn't be acceptable as a default.

There are also many systems that add additional query parameters being used or not, e.g. Google Analytics, Facebook, etc. So I don't think it would work well for many use cases.


But on the other side, I see this could be useful in some cases (like yours), so I think we could have a path operation decorator parameter allow_extra_parameters=True that conditionally triggers your proposed change.

If you want, you could create a PR with that :nerd_face: :rocket:

Assuming the original issue was solved, it will be automatically closed now. But feel free to add more comments or create new issues.

Was this page helpful?
0 / 5 - 0 ratings