8000 remove ujson dependency (it is dead) · Issue #820 · fastapi/fastapi · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

remove ujson dependency (it is dead) #820

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.

Sign up for GitHub

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

Jump to bottom
Closed
hnykda opened this issue Dec 27, 2019 · 14 comments
Closed

remove ujson dependency (it is dead) #820

hnykda opened this issue Dec 27, 2019 · 14 comments
Labels
question Question or problem question-migrate reviewed

Comments

@hnykda
Copy link
hnykda commented Dec 27, 2019 •
edited
Loading

FastAPI is using ujson which has not been actively developed for about 3 years by now. Here is a relevant issue. It's starting to be problematic to actually compile ujson on newer platforms.

Possible alternative maybe orjson.
@hnykda hnykda added the bug Something isn't working label Dec 27, 2019
@bmschmidt bmschmidt mentioned this issue Dec 30, 2019
Closed
@dmontagu
Copy link
Collaborator
dmontagu commented Dec 30, 2019 •
edited
Loading

I would be in favor of recommending orjson instead of ujson.

I recall the creator of orjson reaching about this many months ago and there was some confusion due to slow responses, and also some issues on the pydantic side, but I think many/most of the barriers to usage have since been removed. If orjson seems to be taking over as the go-to high-performance json library, I think it would make sense to use that for development and recommend it.

To the extent that there is any difference in json encoding/decoding between json and orjson (or ujson and orjson), it would probably be best to document it.

I'd be willing to review a PR making this change (including updates to the docs). (I'd leave the ultimate decision about merging to @tiangolo though.)

@tiangolo
Copy link
Member

Yeah, I've been intending to do it for quite some time now... 😅

In fact, you can sort of do it right now (and since several versions ago).

Here's my plan:

I want to export everything from Starlette, to avoid confusions of the type "do I import this from FastAPI or from Starlette?", I've seen reports of that and even I've struggled with that. For example, FastAPI has its own HTTPException.

With that, I want to have a response ORJSONResponse exported from fastapi.responses, among Starlette's responses.

In fact, I added tests for creating custom responses (as part of a PR), testing specifically creating orjson responses: https://github.com/tiangolo/fastapi/blob/f803c77515662cd2382674d9fc8df1e6641b3ba7/tests/test_default_response_class.py#L9-L13
And you can replicate creating that response class and using it. It's just 3 lines (you would remove the custom media_type), but still, it should be provided by FastAPI (among all the other responses from Starlette).

app = FastAPI(default_response_class=ORJSONResponse)

Also, as a side note, UJSON is not really used by default anywhere. It's optional, and you would have to use the UJSONResponse class for it to be used, the same way you would use the (new/custom/future) ORJSONResponse.

@hnykda
Copy link
Author
hnykda commented Feb 13, 2020 •
edited
Loading

Well, that's elegant sir 🙇‍♂️ 👏 .

Also, as a side note, UJSON is not really used by default anywhere. It's optional, and you would have to use the UJSONResponse class for it to be used, the same way you would use the (new/custom/future) ORJSONResponse.

Then I don't understand why it's in requirements. It's written that it's an optional dependency for Pydantic, but it seems that Pydantic has already moved away from ujson here. Seems that their requirements nor setup.py don't have it anymore.

@dmontagu
Copy link
Collaborator
dmontagu commented Feb 13, 2020 •
edited
Loading

@hnykda the “all” set of requirements there includes everything necessary to make use of all documented features, including those that are optional. It is intended more for development/reference purposes than for use by consumers.

@hnykda
Copy link
Author
hnykda commented Feb 14, 2020 •
edited
Loading

Ah, got it! Thanks. I believe this can be closed then.

Personally, I would not reference any obsolete dependencies in my project even if they are optional (especially "future-looking" like Python 3.6+, latest libs, ...) . Reasons for that: helping the community to get rid dangerous/old/unmaintained libs and to avoid possibly repealing people who do not like to install stuff which are referencing such unmaintained projects.

@sm-Fifteen sm-Fifteen mentioned this issue Feb 24, 2020
Merged
@dagnelies
Copy link
dagnelies commented Jun 7, 2020 •
edited
Loading

Apparently, ujson found a new maintainer and is now alive and kicking again. Well, dunno about the kicking part, but releases are pretty fresh:

https://pypi.org/project/ujson/

and repo seems active again. Just mentioning this. Dunno which one is the better lib.

@dagnelies
Copy link

Nevermind, apparently orjson is a fine choice too. Apparently one of the fastest.

@ahmadazizi
Copy link

Just to mention, installing fastapi on windows platform (python 3.9) fails, since orjson neither provide the compatible wheel nor could be compiled.

@jacopofar
Copy link

On Debian stable installing orjson currently fails because it targets manylinux2014 which is supported by pip since version 19.3, but Debian stable install pip 18.1 (if installed through Debian repos python3-pip).

In this case running python3 -m pip install --upgrade pip works, but the error can be tricky to detect

@ahtik
Copy link
ahtik commented Nov 26, 2020

@ahmadazizi, orjson wheels are now available also for Windows Python 3.9.

@Jarmos-san Jarmos-san mentioned this issue Feb 15, 2021
Closed
6 tasks
@ycd
Copy link
Contributor
ycd commented Feb 16, 2021

Since Starlette dropped support for UJSONResponse in encode/starlette#1047 (Requires Starlette 0.14.1), we can remove it too, ujson causes build fails on many distributions due C Dependencies (See #2811 ).

This require changes in #2335, since it re-adds UJSONResponse.

waynesun09 added a commit to RedHatQE/notify-service that referenced this issue Jan 31, 2022
@waynesun09
Merge pull request #36 from waynesun09/fastapi-update
eed860d 
Update fastapi to latest

This will revert PR:
#35

fastapi still depends on the older ujson version.

Issue with remove ujson is tracked in:

fastapi/fastapi#820
@omidmaldar
Copy link
omidmaldar commented Feb 20, 2022 •
edited
Loading

Regarding ujson, there is a security alert reported with moderate severity which is patched in 5.1.0. Fastapi 0.74.0 requires ujson (>=4.0.1,<5.0.0) which is not safe.

@melt7777
Copy link

Hello all, I did find a workaround which allows us to build images without the ujson. I am using the python-poetry package manager, instead of requirements.txt.

FastAPI is fantastic so far, thank you all for your hard work on this project.

Here, roughly the steps I used:

  1. poetry init
  2. poetry add fastapi -E all - it installed the ujson package and placed the ujson bits into the pyproject.toml and the newly created poetry.lock file.
  3. Manually edit the .toml and .lock files and remove all the ujson items.
  4. Build image as usual and it will not have the ujson or its vuln in the image.

Hope this helps someone until they scan make the permanent fix.

Thank you,

~ melt

jcmdln added a commit to jcmdln/snagem that referenced this issue Feb 24, 2022
@jcmdln
Don't use ujson in FastAPI
e7314bd 
We can use orjson in most cases:
fastapi/fastapi#820
waynesun09 added a commit to waynesun09/notify-service that referenced this issue Mar 10, 2022
@waynesun09
Remove ujson dependency
b6f85b3 
- Update default_response_class to ORJSONResponse.
- Remove the ujson dependency as it's deprecated.
- Remove install [all] with fastapi
- Explicitly add dependency packages for Pydantic and Starlette
- Update FastAPI to latest version 0.75.0
- Lock stomp.py to 7.0.0 as 8.0.0 broke the app

This fixes ujson CVE-2021-45958.

This is related to:
fastapi/fastapi#820

Signed-off-by: Wayne Sun <gsun@redhat.com>
@waynesun09 waynesun09 mentioned this issue Mar 10, 2022
Merged
@alexted
Copy link
alexted commented Nov 28, 2022

Are you sure the ujson is EOL? According to its repository, the last release was in mid-September, and the last commit was 3 weeks ago - the project seems to be quite alive.
Another point is that it is inferior to orjson in performance. In that case, it can be left as an option.

@tiangolo tiangolo added question Question or problem reviewed and removed bug Something isn't working labels Feb 24, 2023
@tiangolo tiangolo changed the title [FEATURE] remove ujson dependency (it is dead) remove ujson dependency (it is dead) Feb 24, 2023
@fastapi fastapi locked and limited conversation to collaborators Feb 28, 2023
@tiangolo tiangolo converted this issue into discussion #9155 Feb 28, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Assignees
No one assigned
Labels
question Question or problem question-migrate reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
0