Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always getting 442 Unprocessable entity #23

Closed
einfachTobi opened this issue Oct 25, 2021 · 1 comment
Closed

Always getting 442 Unprocessable entity #23

einfachTobi opened this issue Oct 25, 2021 · 1 comment
Labels
question Further information is requested

Comments

@einfachTobi
Copy link

einfachTobi commented Oct 25, 2021

I cannot get msgpack-asgi running with FastAPI. Any request with msgpack-bytes is returned with a "422 Unprocessable Entity" error. The following minimal example will show the problem:

from pathlib import Path
import uvicorn
from fastapi import FastAPI
from pydantic import BaseModel
from msgpack_asgi import MessagePackMiddleware


class Foo(BaseModel):
    bar: int


app = FastAPI()
app.add_middleware(MessagePackMiddleware)


@app.post("/")
def index(thing: Foo):
    return thing


if __name__ == "__main__":
    uvicorn.run(f"{Path(__file__).stem}:app", host="0.0.0.0", port=5001, log_level="debug", reload=True)

The data is sent with the following snippet:

import requests
import msgpack

url = "http://127.0.0.1:5001/"
headers = {"content-type": "application/x-msgpack"}
data_raw = {"bar": 23}
data_packed = msgpack.packb(data_raw)

response_json = requests.post(url, json=data_raw)
response_msgpack = requests.post(url, data=data_packed, headers=headers)

Resulting in:

INFO:     127.0.0.1:54682 - "POST / HTTP/1.1" 200 OK
INFO:     127.0.0.1:54684 - "POST / HTTP/1.1" 422 Unprocessable Entity

So the data is accepted as json but refused as msgpack-bytes. May there be an incompatibility with newser versions of FastAPI or pydantic? Or am I just using this completely wrong?

@florimondmanca florimondmanca added the question Further information is requested label Oct 25, 2021
@florimondmanca
Copy link
Owner

florimondmanca commented Oct 25, 2021

Hi,

This does seem to be a FastAPI compatibility issue. Something's happening or interacting with FastAPI that makes this 422 error pop out on the FastAPI side.

For reference, here's an equivalent Starlette example that does work as intended, and illustrates what I would expect FastAPI to do — i.e. treat the incoming request as a JSON one from its own POV:

from starlette.applications import Starlette
from starlette.routing import Route
from starlette.responses import JSONResponse
from starlette.middleware import Middleware


async def index(request):
    return JSONResponse(await request.json())


app = Starlette(
    routes=[Route("/", index, methods=["POST"])],
    middleware=[Middleware(MessagePackMiddleware)],
)

Your guess about a recent incompatibility seems accurate. I pinned down the FastAPI version that introduces this error to fastapi==0.65.2 (see release notes).

Security fix tiangolo/fastapi#2118 introduced this change:

Check Content-Type request header before assuming JSON.

While msgpack-asgi re-encodes the data, it does not re-write the content-type header. So FastAPI receives JSON data in the body, but sees content-type: application/x-msgpack, so it does not try to read it, and ends up returning the 422 error. See lines https://github.com/tiangolo/fastapi/pull/2118/files#diff-2674bb0bf60f9e78a1fbe4319787bf92da4bfcf0c1f08e310c9c68925c145c59R232-R257 for a test case that seems to describe the situation. If we print(response_msgpack.json()) we even get the same error message:

{'detail': [{'loc': ['body'], 'msg': 'value is not a valid dict', 'type': 'type_error.dict'}]}

Now, there are a few questions…

  • Is this analysis accurate / complete? Is the bug actually arising from the code in 🐛 Check Content-Type request header before assuming JSON tiangolo/fastapi#2118, or from somewhere else that interacts with that change? I'm still not sure where the 422 is fired from exactly.
  • Should we rewrite the request content-type to application/json?
  • OR should we declare this as "FastAPI does not support sending msgpack payloads"?

cc @tiangolo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants