Skip to content

Fixed #36940 -- Improved ASGI script prefix path_info handling.#20749

Merged
jacobtylerwalls merged 1 commit intodjango:mainfrom
KhadyotTakale:fix/asgi-script-prefix
Mar 6, 2026
Merged

Fixed #36940 -- Improved ASGI script prefix path_info handling.#20749
jacobtylerwalls merged 1 commit intodjango:mainfrom
KhadyotTakale:fix/asgi-script-prefix

Conversation

@KhadyotTakale
Copy link
Copy Markdown
Contributor

Trac ticket number

ticket-36940

Branch description

The current ASGIRequest.__init__ uses str.removeprefix() to strip the script name from the request path to compute path_info. This is fragile because removeprefix is a pure string operation — it doesn't verify that the prefix is a proper path segment boundary.

For example, if script_name is /myapp and the path is /myapplication/page, removeprefix would incorrectly produce lication/page.

This patch replaces removeprefix with a check that ensures the script name is followed by / or is the exact path, before stripping it. This addresses the inline TODO comment.

AI Assistance Disclosure (REQUIRED)

  • If AI tools were used, I have disclosed which ones, and fully reviewed and verified their output.

AI tools (Claude) were used to understand the issue and guide the approach. The code was reviewed and verified manually.

Checklist

  • This PR follows the contribution guidelines.
  • This PR does not disclose a security vulnerability (see vulnerability reporting).
  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

@p-r-a-v-i-n
Copy link
Copy Markdown
Contributor

Hi @KhadyotTakale ,
please make sure to go through contributing docs.
Before submiting the contribution please check ticket status . docs

Copy link
Copy Markdown
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. A regression test is required. Please place it near the related tests in AsyncHandlerRequestTests.

Comment thread django/core/handlers/asgi.py Outdated
@KhadyotTakale KhadyotTakale force-pushed the fix/asgi-script-prefix branch from 8534ec7 to b51ce2a Compare March 6, 2026 17:44
Paths that happened to begin with the script name were inappropriately
stripped, instead of checking that script name preceded a slash.
@jacobtylerwalls jacobtylerwalls force-pushed the fix/asgi-script-prefix branch from b51ce2a to d0245dd Compare March 6, 2026 22:11
Copy link
Copy Markdown
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. Welcome aboard! ⛵

I pushed teeny cosmetic updates to blend in with project style.

Comment thread django/core/handlers/asgi.py
@jacobtylerwalls jacobtylerwalls merged commit b33c31d into django:main Mar 6, 2026
19 of 41 checks passed
tonybaloney pushed a commit to tonybaloney/swe-complex-django that referenced this pull request Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants