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

attempting to send a HTTP request with method "patch" is not normalized to upper case #1805

Closed
mattnworb opened this issue Dec 9, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@mattnworb
Copy link

mattnworb commented Dec 9, 2022

Bug Description

When using fetch() to make a PATCH request, sending the method name in all lowercase (which is arguably a programmer error) results in the actual HTTP request on the wire containing the method name as patch:

fetch(url, {method: 'patch', ...});
  121 423.127099          ::1 → ::1          TCP 291 63273 → 8000 [PSH, ACK] Seq=1 Ack=1 Win=407744 Len=215 TSval=1214918928 TSecr=3539375328

0000  1e 00 00 00 60 03 0c 00 00 f7 06 40 00 00 00 00   ....`......@....
0010  00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00   ................
0020  00 00 00 00 00 00 00 00 00 00 00 01 f7 29 1f 40   .............).@
0030  21 f8 37 c5 34 73 c8 7a 80 18 18 e3 00 ff 00 00   !.7.4s.z........
0040  01 01 08 0a 48 6a 31 10 d2 f6 94 e0 70 61 74 63   ....Hj1.....patc  <== note the lowercase
0050  68 20 2f 61 70 69 2f 76 33 2f 66 6f 6f 62 61 72   h /api/v3/foobar  <==
0060  20 48 54 54 50 2f 31 2e 31 0d 0a 68 6f 73 74 3a    HTTP/1.1..host:
0070  20 6c 6f 63 61 6c 68 6f 73 74 3a 38 30 30 30 0d    localhost:8000.
0080  0a 63 6f 6e 6e 65 63 74 69 6f 6e 3a 20 6b 65 65   .connection: kee
0090  70 2d 61 6c 69 76 65 0d 0a 61 75 74 68 6f 72 69   p-alive..authori
00a0  7a 61 74 69 6f 6e 3a 20 74 6f 6b 65 6e 20 66 6f   zation: token fo
00b0  6f 62 61 72 0d 0a 61 63 63 65 70 74 3a 20 2a 2f   obar..accept: */
00c0  2a 0d 0a 61 63 63 65 70 74 2d 6c 61 6e 67 75 61   *..accept-langua
00d0  67 65 3a 20 2a 0d 0a 73 65 63 2d 66 65 74 63 68   ge: *..sec-fetch
00e0  2d 6d 6f 64 65 3a 20 63 6f 72 73 0d 0a 75 73 65   -mode: cors..use
00f0  72 2d 61 67 65 6e 74 3a 20 75 6e 64 69 63 69 0d   r-agent: undici.
0100  0a 61 63 63 65 70 74 2d 65 6e 63 6f 64 69 6e 67   .accept-encoding
0110  3a 20 67 7a 69 70 2c 20 64 65 66 6c 61 74 65 0d   : gzip, deflate.
0120  0a 0d 0a                                          ...

whereas if you call fetch with method: 'get', the normalizeMethod() function in lib/fetch.js will fix your mistake and translate it to upper-case:

fetch(url, {method: 'get', ...});
  141 423.137031          ::1 → ::1          HTTP 290 GET /api/v3/foobar/ HTTP/1.1

0000  1e 00 00 00 60 03 04 00 00 f6 06 40 00 00 00 00   ....`......@....
0010  00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00   ................
0020  00 00 00 00 00 00 00 00 00 00 00 01 f7 2b 1f 40   .............+.@
0030  b4 f6 e7 59 b8 4d 81 fe 80 18 18 e3 00 fe 00 00   ...Y.M..........
0040  01 01 08 0a ab 00 8d 71 0b b2 aa 5a 47 45 54 20   .......q...ZGET   <== uppercase!
0050  2f 61 70 69 2f 76 33 2f 66 6f 6f 62 61 72 2f 20   /api/v3/foobar/
0060  48 54 54 50 2f 31 2e 31 0d 0a 68 6f 73 74 3a 20   HTTP/1.1..host:
0070  6c 6f 63 61 6c 68 6f 73 74 3a 38 30 30 30 0d 0a   localhost:8000..
0080  63 6f 6e 6e 65 63 74 69 6f 6e 3a 20 6b 65 65 70   connection: keep
0090  2d 61 6c 69 76 65 0d 0a 61 75 74 68 6f 72 69 7a   -alive..authoriz
00a0  61 74 69 6f 6e 3a 20 74 6f 6b 65 6e 20 66 6f 6f   ation: token foo
00b0  62 61 72 0d 0a 61 63 63 65 70 74 3a 20 2a 2f 2a   bar..accept: */*
00c0  0d 0a 61 63 63 65 70 74 2d 6c 61 6e 67 75 61 67   ..accept-languag
00d0  65 3a 20 2a 0d 0a 73 65 63 2d 66 65 74 63 68 2d   e: *..sec-fetch-
00e0  6d 6f 64 65 3a 20 63 6f 72 73 0d 0a 75 73 65 72   mode: cors..user
00f0  2d 61 67 65 6e 74 3a 20 75 6e 64 69 63 69 0d 0a   -agent: undici..
0100  61 63 63 65 70 74 2d 65 6e 63 6f 64 69 6e 67 3a   accept-encoding:
0110  20 67 7a 69 70 2c 20 64 65 66 6c 61 74 65 0d 0a    gzip, deflate..
0120  0d 0a                                             ..

Expected Behavior

The normalizeMethod(method) undici's util.js

undici/lib/fetch/util.js

Lines 668 to 673 in f6323f6

// https://fetch.spec.whatwg.org/#concept-method-normalize
function normalizeMethod (method) {
return /^(DELETE|GET|HEAD|OPTIONS|POST|PUT)$/i.test(method)
? method.toUpperCase()
: method
}
should include the PATCH verb in its test so that a value like 'patch' is translated to upper-case.

Environment

Node v18.12.1

@mattnworb mattnworb added the bug Something isn't working label Dec 9, 2022
@KhafraDev
Copy link
Member

The spec says that to normalize a method:

"To normalize a method, if it is a byte-case-insensitive match for DELETE, GET, HEAD, OPTIONS, POST, or PUT, byte-uppercase it."

Then there's a discussion here, which gives explicit reasoning for not normalizing patch:

"HTTP verbs are case-sensitive. That we normalize a couple of them is already, strictly speaking, against the rules, but we have to do so for compatibility. If you want to use PATCH, write PATCH. If you want patch, write patch. If you want Patch, write Patch. Etc. None of this has anything to do with CORS, they should all work equally well."

@mattnworb
Copy link
Author

mattnworb commented Dec 9, 2022

thanks for the reference. I agree that 'patch' is a programmer error, but didn't realize the method normalization here was also following the spec. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants