Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

HTTP server should allow custom methods #3192

Closed
aspyct opened this issue Apr 30, 2012 · 10 comments
Closed

HTTP server should allow custom methods #3192

aspyct opened this issue Apr 30, 2012 · 10 comments
Labels
Milestone

Comments

@aspyct
Copy link

aspyct commented Apr 30, 2012

Hello,

While working with the "http" module, and specifically the http server, I noticed that the server closes the connection instantly if the client attempts to use a custom method (i.e. something else than GET, HEAD...).

I think we should allow custom methods for this server:

  1. it would allow to use node for specific requirements
  2. custom methods are allowed by the HTTP standard.
  3. I don't think a lot of work is required to allow this behavior.

On my side, I currently need to use custom methods. Is there any way I could allow a method of my own with your HTTP server without having to hack the code ?

Regards,

Antoine

@bnoordhuis
Copy link
Member

Related to nodejs/http-parser#92.

  1. I don't think a lot of work is required to allow this behavior.

You may be surprised. Support for arbitrary request methods would first need to be added to joyent/http-parser in a way that a) doesn't suck (it should have a nice API) and b) imposes no performance penalty. I won't be working on that anytime soon but I will consider pull requests.

@bnoordhuis
Copy link
Member

On my side, I currently need to use custom methods. Is there any way I could allow a method of my own with your HTTP server without having to hack the code ?

Not in non-brittle ways. There are a number of pure JS HTTP parsers out there, e.g. https://github.com/substack/node-parsley

@aspyct
Copy link
Author

aspyct commented Apr 30, 2012

I actually ended up adding the required method in the http_parser and node_http_parser sources, and it works fine, but for one specific method only.

I'll try to find some time to fork and provide something more generic. Thx for your help.

@soletan
Copy link

soletan commented Nov 25, 2012

+1

I agree to have a proper parser reading HTTP method in compliance with RFC2616 5.1.1 with:

extension-method = token

and the latter produced as (defined in RFC2616 2.2):

token          = 1*<any CHAR except CTLs or separators>
separators     = "(" | ")" | "<" | ">" | "@"
                      | "," | ";" | ":" | "\" | <">
                      | "/" | "[" | "]" | "?" | "="
                      | "{" | "}" | SP | HT

This is a very limited set of excluded values. And since HTTP's method is contained in very leading octets of a request message there might be simple method reading octet by octet to the first separator or CTL. Obviously, penalty on performance isn't considerable due to coding this byte-wise parser sufficiently working without any state processing in C++.

I haven't checked the http_parser.cc in detail, but isn't there such a tokenizer to be reused here for sake of simplicity?

@isaacs
Copy link

isaacs commented Dec 30, 2012

Revisit for 0.12.

@buschtoens
Copy link

Just to bring this up again, as 0.12 is on it's way (as far as I can see).
Would it be possible to completely get rid of the C++ bindings and do the HTTP module in JS only or would that be too much of a performance bottleneck? Maybe we could borrow some of node-parsley's code?

Skipping through the source I get the feeling, that the HTTP module deserves a rewrite anyway.

@isaacs
Copy link

isaacs commented Apr 5, 2013

Yes, the http module needs a lot of work. That's planned for 0.12.

Moving the parser to JS is an interesting idea, but not one that'll probably happen for 0.12.

@pfrazee
Copy link

pfrazee commented Jun 18, 2013

Hi, just wanted to 👍 this update

@jasnell
Copy link
Member

jasnell commented Aug 20, 2013

@jezell .. thank you for linking issue #6078 to this. I had missed this existing issue when I opened it. There are a few issues that I'm seeing in this code. The GEM/PUN -> GET/PUT is definitely one (i see there was already a pull request to cover that one); dropping the connection rather than returning a 405 or other error code is another; the complete lack of support for parsing perfectly valid methods is only part of it.

@isaacs
Copy link

isaacs commented Aug 27, 2013

If nodejs/http-parser#158 lands, then this is trivial to do. If not, then it's impossible.

Closing this issue. Please take any discussion over to nodejs/http-parser#158. If/when that lands, we'll do this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants