Skip to content

Replace requests with httpx client for non blocking execution#6942

Closed
prabinoid wants to merge 1 commit intodevelopfrom
refactor/ohsome-stats
Closed

Replace requests with httpx client for non blocking execution#6942
prabinoid wants to merge 1 commit intodevelopfrom
refactor/ohsome-stats

Conversation

@prabinoid
Copy link
Copy Markdown
Collaborator

What type of PR is this? (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation
  • 🧑‍💻 Refactor
  • ✅ Test
  • 🤖 Build or CI
  • ❓ Other (please specify)

Describe this PR

Using requests (a sync lib) in an async context might block the thread, degrading performance.
Replace requests with httpx client for non blocking execution.

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

# 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)

Check warning

Code scanning / SonarCloud

Server-side requests should not be vulnerable to forging attacks Medium

Change this code to not construct the URL from user-controlled data. See more on SonarQube Cloud
# 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)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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?)

Copy link
Copy Markdown
Collaborator Author

@prabinoid prabinoid Jul 10, 2025

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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

@prabinoid
Copy link
Copy Markdown
Collaborator Author

Closing as its captured in PR 6943 where the updated ohsome apis are implemented.

@prabinoid prabinoid closed this Jul 10, 2025
@prabinoid prabinoid deleted the refactor/ohsome-stats branch September 1, 2025 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants