-
-
Notifications
You must be signed in to change notification settings - Fork 301
Replace requests with httpx client for non blocking execution #6942
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| from datetime import date, timedelta | ||
|
|
||
| import requests | ||
| import httpx | ||
| from databases import Database | ||
| from fastapi import APIRouter, Depends, Request, Query | ||
| from fastapi.responses import JSONResponse | ||
|
|
@@ -14,6 +14,7 @@ | |
| from backend.services.users.authentication_service import login_required | ||
| from backend.services.users.user_service import UserService | ||
|
|
||
|
|
||
| router = APIRouter( | ||
| prefix="/users", | ||
| tags=["users"], | ||
|
|
@@ -175,39 +176,38 @@ | |
| user: AuthUserDTO = Depends(login_required), | ||
| ): | ||
| """ | ||
| Get HomePage Stats | ||
| --- | ||
| tags: | ||
| - system | ||
| produces: | ||
| - application/json | ||
| parameters: | ||
| - in: header | ||
| name: Authorization | ||
| description: Base64 encoded session token | ||
| required: true | ||
| type: string | ||
| default: Token sessionTokenHere== | ||
| - in: query | ||
| name: url | ||
| type: string | ||
| description: get user stats for osm contributions | ||
| responses: | ||
| 200: | ||
| description: User stats | ||
| 500: | ||
| description: Internal Server Error | ||
| Get HomePage Stats for OSM contributions | ||
| """ | ||
| if not url: | ||
| return JSONResponse( | ||
| content={"Error": "URL is None", "SubCode": "URL not provided"}, | ||
| status_code=400, | ||
| ) | ||
|
|
||
| headers = {"Authorization": f"Basic {settings.OHSOME_STATS_TOKEN}"} | ||
|
|
||
| try: | ||
| headers = {"Authorization": f"Basic {settings.OHSOME_STATS_TOKEN}"} | ||
| # Make the GET request with headers | ||
| response = requests.get(url, headers=headers) | ||
| async with httpx.AsyncClient() as client: | ||
| response = await client.get(url, headers=headers, timeout=10.0) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is great, but it flagged an error - rightly so too. The user could pass in the URL to a malicious script here easily. Can we pass in something like the user id here and construct the URL on the backend? (and on that note, do we pass a URL to any other endpoints in TM?)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you Sam. We allow redirect uri to be passed in login. Although, by default, it is being fetched from config but can be overridden but will be handled by oauth app and it should be ok on that part.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redirect URI is fine, as it's not actually called by the backend. The problem is fetching the URL in the backend code, with a lib like requests / httpx |
||
| response.raise_for_status() | ||
|
|
||
| return response.json() | ||
|
|
||
| except httpx.HTTPStatusError as e: | ||
| return JSONResponse( | ||
| content={ | ||
| "Error": f"Upstream error: {e.response.status_code}", | ||
| "Details": e.response.text, | ||
| }, | ||
| status_code=e.response.status_code, | ||
| ) | ||
|
|
||
| except httpx.RequestError as e: | ||
| return JSONResponse( | ||
| content={"Error": "Error fetching data", "Details": str(e)}, | ||
| status_code=502, | ||
| ) | ||
|
|
||
| except Exception as e: | ||
| return JSONResponse( | ||
| content={"Error": str(e), "SubCode": "Error fetching data"}, status_code=400 | ||
|
|
||
Check warning
Code scanning / SonarCloud
Server-side requests should not be vulnerable to forging attacks Medium