Skip to content
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.

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

Why does HTTP1 module lack support for certain request methods? #33699

Closed
masx200 opened this issue Jun 2, 2020 · 6 comments
Closed

Why does HTTP1 module lack support for certain request methods? #33699

masx200 opened this issue Jun 2, 2020 · 6 comments
Labels
http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem. question Issues that look for answers.

Comments

@masx200
Copy link
Contributor

masx200 commented Jun 2, 2020

  • Version:
    node
    v14.2.0
  • Platform:

Linux localhost 4.9.148 #1 SMP PREEMPT Tue Mar 10 02:27:59 CST 2020 aarch64 Android

  • Subsystem:

What steps will reproduce the bug?

Check the request methods supported by the http1 module.

console.log(http.METHODS);
[
  "ACL",
  "BIND",
  "CHECKOUT",
  "CONNECT",
  "COPY",
  "DELETE",
  "GET",
  "HEAD",
  "LINK",
  "LOCK",
  "M-SEARCH",
  "MERGE",
  "MKACTIVITY",
  "MKCALENDAR",
  "MKCOL",
  "MOVE",
  "NOTIFY",
  "OPTIONS",
  "PATCH",
  "POST",
  "PROPFIND",
  "PROPPATCH",
  "PURGE",
  "PUT",
  "REBIND",
  "REPORT",
  "SEARCH",
  "SOURCE",
  "SUBSCRIBE",
  "TRACE",
  "UNBIND",
  "UNLINK",
  "UNLOCK",
  "UNSUBSCRIBE"
]

Check the request methods supported by the http2 module.

console.log(
  Object.entries(http2.constants).filter(([key, value]) => {
    return key.includes("HTTP2_METHOD");
  })
);
[
  ["HTTP2_METHOD_ACL", "ACL"],
  ["HTTP2_METHOD_BASELINE_CONTROL", "BASELINE-CONTROL"],
  ["HTTP2_METHOD_BIND", "BIND"],
  ["HTTP2_METHOD_CHECKIN", "CHECKIN"],
  ["HTTP2_METHOD_CHECKOUT", "CHECKOUT"],
  ["HTTP2_METHOD_CONNECT", "CONNECT"],
  ["HTTP2_METHOD_COPY", "COPY"],
  ["HTTP2_METHOD_DELETE", "DELETE"],
  ["HTTP2_METHOD_GET", "GET"],
  ["HTTP2_METHOD_HEAD", "HEAD"],
  ["HTTP2_METHOD_LABEL", "LABEL"],
  ["HTTP2_METHOD_LINK", "LINK"],
  ["HTTP2_METHOD_LOCK", "LOCK"],
  ["HTTP2_METHOD_MERGE", "MERGE"],
  ["HTTP2_METHOD_MKACTIVITY", "MKACTIVITY"],
  ["HTTP2_METHOD_MKCALENDAR", "MKCALENDAR"],
  ["HTTP2_METHOD_MKCOL", "MKCOL"],
  ["HTTP2_METHOD_MKREDIRECTREF", "MKREDIRECTREF"],
  ["HTTP2_METHOD_MKWORKSPACE", "MKWORKSPACE"],
  ["HTTP2_METHOD_MOVE", "MOVE"],
  ["HTTP2_METHOD_OPTIONS", "OPTIONS"],
  ["HTTP2_METHOD_ORDERPATCH", "ORDERPATCH"],
  ["HTTP2_METHOD_PATCH", "PATCH"],
  ["HTTP2_METHOD_POST", "POST"],
  ["HTTP2_METHOD_PRI", "PRI"],
  ["HTTP2_METHOD_PROPFIND", "PROPFIND"],
  ["HTTP2_METHOD_PROPPATCH", "PROPPATCH"],
  ["HTTP2_METHOD_PUT", "PUT"],
  ["HTTP2_METHOD_REBIND", "REBIND"],
  ["HTTP2_METHOD_REPORT", "REPORT"],
  ["HTTP2_METHOD_SEARCH", "SEARCH"],
  ["HTTP2_METHOD_TRACE", "TRACE"],
  ["HTTP2_METHOD_UNBIND", "UNBIND"],
  ["HTTP2_METHOD_UNCHECKOUT", "UNCHECKOUT"],
  ["HTTP2_METHOD_UNLINK", "UNLINK"],
  ["HTTP2_METHOD_UNLOCK", "UNLOCK"],
  ["HTTP2_METHOD_UPDATE", "UPDATE"],
  ["HTTP2_METHOD_UPDATEREDIRECTREF", "UPDATEREDIRECTREF"],
  ["HTTP2_METHOD_VERSION_CONTROL", "VERSION-CONTROL"]
]

Why does HTTP1 module lack support for certain request methods?

The HTTP1 module does not support the following request methods.

[
  "BASELINE-CONTROL",
  "CHECKIN",
  "LABEL",
  "MKREDIRECTREF",
  "MKWORKSPACE",
  "ORDERPATCH",
  "PRI",
  "UNCHECKOUT",
  "UPDATE",
  "UPDATEREDIRECTREF",
  "VERSION-CONTROL"
]

Another problem is that the HTTP2 module also lacks certain request methods supported in HTTP1.

["M-SEARCH", "NOTIFY", "PURGE", "SOURCE", "SUBSCRIBE", "UNSUBSCRIBE"]

Write the following files.

D:\Desktop\test.js

require('http')
    .createServer((req, res) => {
        res.end(req.method);
    })
    .listen(8000);

Use node to run it.

node D:\Desktop\test.js

Then open http://localhost:8000/ in the google chrome browser.

Enter the following code in the browser console.

fetch("http://localhost:8000/", {

  "method": "VERSION-CONTROL",
 
}).then(console.log,);

fetch("http://localhost:8000/", {

 
  "method": "BASELINE-CONTROL",
 
}).then(console.log);

How often does it reproduce? Is there a required condition?

The probability of this problem occurring is 100%.

What is the expected behavior?

Response {type: "basic", url: "http://localhost:8000/", redirected: false, status: 200, ok: true, …}
body: (...)
bodyUsed: false
headers: Headers {}
ok: true
redirected: false
status: 200
statusText: "OK"
type: "basic"
url: "http://localhost:8000/"
__proto__: Response
Response {type: "basic", url: "http://localhost:8000/", redirected: false, status: 200, ok: true, …}
body: (...)
bodyUsed: false
headers: Headers {}
ok: true
redirected: false
status: 200
statusText: "OK"
type: "basic"
url: "http://localhost:8000/"
__proto__: Response

What do you see instead?

VERSION-CONTROL http://localhost:8000/ net::ERR_ABORTED 400 (Bad Request)

Response {type: "basic", url: "http://localhost:8000/", redirected: false, status: 400, ok: false, …}
body: (...)
bodyUsed: false
headers: Headers {}
ok: false
redirected: false
status: 400
statusText: "Bad Request"
type: "basic"
url: "http://localhost:8000/"
__proto__: Response

 BASELINE-CONTROL http://localhost:8000/ net::ERR_ABORTED 400 (Bad Request)


Response {type: "basic", url: "http://localhost:8000/", redirected: false, status: 400, ok: false, …}
body: (...)
bodyUsed: false
headers: Headers {}
ok: false
redirected: false
status: 400
statusText: "Bad Request"
type: "basic"
url: "http://localhost:8000/"
__proto__: Response

Additional information

@mscdex
Copy link
Contributor

mscdex commented Jun 2, 2020

In general, I believe the reason is that new HTTP methods can introduce new semantics, behavior, and/or parsing requirements in the protocol that the HTTP parser wouldn't be prepared for.

@devsnek
Copy link
Member

devsnek commented Jun 2, 2020

These are pretty much all different methods from WebDAV extensions, except for M-SEARCH (UPnP), and PRI (an http/2 meta-method used for http/1 interop)

@devsnek devsnek added http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem. question Issues that look for answers. labels Jun 2, 2020
@jasnell
Copy link
Member

jasnell commented Jun 2, 2020

@mscdex:

In general, I believe the reason is that new HTTP methods can introduce new semantics, behavior, and/or parsing requirements in the protocol that the HTTP parser wouldn't be prepared for.

It's much simpler and annoying than that... The original http-parser used a hard-coded list of supported methods for the sake of performance and expanding on that list was extremely difficult. It's been improved somewhat using the new parser written by @indutny but the legacy is still there. HTTP/2 (and eventually HTTP/3) don't suffer from that because how we handle header field values is significantly different.

@masx200
Copy link
Contributor Author

masx200 commented Jun 8, 2020

https://github.com/nodejs/llhttp

Now that the HTTP parser used by nodejs is llhttp, should it be easy to support the new request method?

@mnini
Copy link

mnini commented Jun 10, 2020

It's just actually one word... It could be made so it can be custom, and on server you can also make it to handle custom word. No problems with all the words being written then.

@jasnell
Copy link
Member

jasnell commented Jun 19, 2020

Will close this issue here given that nodejs/llhttp#61 has been opened. There's nothing more actionable here until that issue is resolved and llhttp is updated.

@jasnell jasnell closed this as completed Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

5 participants