Skip to content

add workaround for Twisted when receiving absolute-form URI in HTTP request #23

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
Merged
thrau merged 9 commits into from
Oct 17, 2024
Merged

add workaround for Twisted when receiving absolute-form URI in HTTP request #23

thrau merged 9 commits into from
Oct 17, 2024

Conversation

@bentsku
Copy link
Contributor

@bentsku bentsku commented Oct 11, 2024 •
edited
Loading

Motivation

As reported by localstack/localstack#11652, we've had an issue in LocalStack because the request would contain the whole URI in the request.path.

This can be traced down to Twisted with a simple reproducer:

from twisted.web import server, resource
from twisted.internet import reactor

class Simple(resource.Resource):
    isLeaf = True
    def render_POST(self, request):
        print(f"{request.path=}")
        return "<html>Hello, world!</html>"

site = server.Site(Simple())
reactor.listenTCP(4566, site)
reactor.run()

Sending a request with an absolute-form URI (see RFC7230: https://tools.ietf.org/html/rfc7230#section-5.3.2) like GET http://localhost:4566/path HTTP/1.1 will print request.path=http://localhost:4566/path.

When using the WSGI compatible part of twisted, it will even replace the first char by /, leading to PATH_INFO in the environ to be /ttp://localhost:4566/path.

This is only a workaround until we can fix it in Twisted directly, and if they're open to it.
Seems an issue was opened in 2010 about it: twisted/twisted#4371

Changes

@bentsku bentsku requested a review from thrau October 11, 2024 20:13
@bentsku bentsku self-assigned this Oct 11, 2024
@bentsku bentsku mentioned this pull request Oct 11, 2024
Merged
@bentsku bentsku marked this pull request as draft October 11, 2024 20:49
@bentsku bentsku marked this pull request as ready for review October 11, 2024 21:06
thrau
thrau approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

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

very nice! excellent 🕵️ detective work @bentsku.

@thrau thrau merged commit 11d8ebe into main Oct 17, 2024
@thrau thrau deleted the fix-twisted-environ branch October 17, 2024 18:22
thrau pushed a commit that referenced this pull request Oct 17, 2024
@bentsku @thrau
add workaround for Twisted when receiving absolute-form URI in HTTP r…
48eeaa8 
…equest (#23)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thrau thrau thrau approved these changes

Assignees

@bentsku bentsku

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bentsku @thrau