Skip to content

Fix wsgi invalid request uri#4551

Merged
xrmx merged 23 commits into
open-telemetry:mainfrom
IonDragos2003:fix-wsgi-invalid-request-uri
May 18, 2026
Merged

Fix wsgi invalid request uri#4551
xrmx merged 23 commits into
open-telemetry:mainfrom
IonDragos2003:fix-wsgi-invalid-request-uri

Conversation

@IonDragos2003

@IonDragos2003 IonDragos2003 commented May 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Fixes #4447

WSGI Instrumentation currently parses RAW_URI / REQUEST_URI to derive url.path & url.query. Malformed request URIs can cause urllib.parse.urlparse to raise a ValueError, which breaks instrumentation and may result in a 500 response.

This change avoids parsing the raw request URI and instead derives url.path and url.query directly from the WSGI environ (PATH_INFO and QUERY_STRING).

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Added a regression test:
    • test_request_attributes_with_invalid_request_uri_uses_wsgi_environ
    • Verifies that malformed REQUEST_URI does not raise and that URL_PATH / URL_QUERY are derived from WSGI environ.
  • Updated existing test:
    • test_request_attributes_with_full_request_uri to reflect new behavior (no parsing of REQUEST_URI).

Ran tests:
uv run python -m pytest instrumentation/opentelemetry-instrumentation-wsgi/tests

All tests pass.

Does This PR Require a Core Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

emdneto
emdneto reviewed May 8, 2026
View reviewed changes

@emdneto emdneto left a comment •
edited
Loading

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.

example.com/ still valid no? I don't see the exception running this test against main

Comment thread instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py Outdated
@IonDragos2003
IonDragos2003 force-pushed the fix-wsgi-invalid-request-uri branch from 99e5faf to a1ca22a Compare May 8, 2026 12:42
@IonDragos2003

IonDragos2003 commented May 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Hi @emdneto, seems that these changes are now breaking some Falcon tests.

Should I do something like

if target:
    try:
        path, query = _parse_url_query(target)
    except ValueError:
        path = environ.get("PATH_INFO")
        query = environ.get("QUERY_STRING")
    _set_http_target(result, target, path, query, sem_conv_opt_in_mode)

@emdneto

emdneto commented May 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Hi @emdneto, seems that these changes are now breaking some Falcon tests.

Should I do something like

if target:
    try:
        path, query = _parse_url_query(target)
    except ValueError:
        path = environ.get("PATH_INFO")
        query = environ.get("QUERY_STRING")
    _set_http_target(result, target, path, query, sem_conv_opt_in_mode)

@IonDragos2003

I would fix the Falcon tests instead, since the target is no longer used to parse the path and query. Because of that, _has_fixed_http_target in the Falcon tests no longer has the same effect it used to.

Also, please fix precommit by running tox -e precommit and add a changelog entry! Thanks

@xrmx xrmx moved this to Reviewed PRs that need fixes in Python PR digest May 11, 2026
@IonDragos2003
IonDragos2003 requested a review from a team as a code owner May 11, 2026 12:35
emdneto
emdneto reviewed May 11, 2026
View reviewed changes
Comment thread instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py Outdated
@emdneto

emdneto commented May 12, 2026

Copy link
Copy Markdown
Member

Thanks for the PR!

Just a heads-up: we no longer update CHANGELOG.md directly. The changelog is now generated from changelog fragments using Towncrier.

Please add the appropriate changelog fragment for this change instead of editing CHANGELOG.md manually. You can find the instructions and expected format in CONTRIBUTING.md.

@emdneto emdneto mentioned this pull request May 12, 2026
Merged
@IonDragos2003
IonDragos2003 force-pushed the fix-wsgi-invalid-request-uri branch from 48f6b40 to 6f6c160 Compare May 13, 2026 08:57
tammy-baylis-swi
tammy-baylis-swi approved these changes May 13, 2026
View reviewed changes

@tammy-baylis-swi tammy-baylis-swi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm and adheres to PEP 333.

@github-project-automation github-project-automation Bot moved this from Reviewed PRs that need fixes to Approved PRs in Python PR digest May 13, 2026
emdneto
emdneto reviewed May 15, 2026
View reviewed changes
Comment thread instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py Outdated
@IonDragos2003
Update test
8738378 
Add PATH_INFO to the environment for CONNECT request.
xrmx
xrmx reviewed May 18, 2026
View reviewed changes
Comment thread .changelog/4551.fixed Outdated
Comment thread instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py Outdated
@xrmx
xrmx requested a review from emdneto May 18, 2026 08:35
@xrmx
xrmx enabled auto-merge (squash) May 18, 2026 12:12
@xrmx
xrmx merged commit 35f1575 into open-telemetry:main May 18, 2026
753 checks passed
@github-project-automation github-project-automation Bot moved this from Approved PRs to Done in Python PR digest May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xrmx xrmx xrmx approved these changes
@emdneto emdneto emdneto approved these changes
@tammy-baylis-swi tammy-baylis-swi tammy-baylis-swi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Python PR digest
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

WSGI instrumentation returns 500 when given invalid url

4 participants

@IonDragos2003 @emdneto @xrmx @tammy-baylis-swi