search

cherrypy / cheroot

Cheroot is the high-performance, pure-Python HTTP server used by CherryPy. Docs -->
https://cheroot.cherrypy.dev
BSD 3-Clause "New" or "Revised" License
185 stars 90 forks source link

Defer HTTP/1.1 parsing to Hyper (h11 project) #201

Open webknjaz opened 5 years ago

webknjaz commented 5 years ago

I suggest merging our HTTP implementation with Hyper/H11: https://github.com/python-hyper/h11 / https://h11.readthedocs.io/.

This would contribute to having a well-made parser shared with others.

I've also declared the intent @ https://github.com/python-hyper/h11/issues/84

❓ I'm submitting a ...

🐞 Is your feature request related to a problem? Please describe.

Currently, CherryPy team has to maintain its own HTTP parser implementation which doesn't scale.

🐣 Describe the solution you'd like

We should contribute to one well-designed upstream parser implementation instead of maintaining our own thing. Shared responsibility is the key to having a healthy open source project :)

📋 Describe alternatives you've considered

N/A

📋 Additional context

Hyper is a bring-your-own-I/O set of principles encouraging users to rely on protocol implementations which do not hit the network. It also potentially offers testability improvements.

It'll also prevent problems like https://github.com/cherrypy/cheroot/pull/74

jaraco commented 5 years ago

Agreed this sounds like a good idea. The trick will be - can it be implemented without causing too much disruption.

webknjaz commented 5 years ago

I hope yes, but it'll require extensive testing anyway and maybe it'll also require some changes on their side. But I hope that I'll be able to dedicate some time to this.

I think I've also asked @duper51 to look into this earlier...

ian-otto commented 5 years ago

I would tend to agree with this issue, we should probably move parsing logic out of this project, as it introduces complexity that doesn't need to be here.

Agreed this sounds like a good idea. The trick will be - can it be implemented without causing too much disruption.

If there is disruption, it will be on projects relying on quirks of the existing parser (which isn't the best in terms of overall RFC compliance). I'd say if it breaks something in a dependent project, the reduced complexity will make any fixes easier.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ian-otto commented 4 years ago

Reviving this, as I have some more free time to deal with it.

Quick update, I've started work on a replacement HTTPRequest class to be a relatively "drop-in" replacement for the existing one. That said, we will likely see changes in some of the attributes that are available post-request. This is due to removing some error handling related attributes since they are now handled in h11. More updates soon (mostly just wanted to remove the stale tag).

webknjaz commented 4 years ago

@ian-otto it'd be great if you could start a draft PR. It doesn't have to be completed but it's useful to see the progress even in the initial versions...

ian-otto commented 4 years ago

Created! See #262

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.