Allowing extended HTTP methods

Allowing extended HTTP methods

[Ryan Raymond]

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

I'm not sure how pull-requests are handled in this project, but Maxime
suggested I send the PR to this mailing list.

The following changes since commit 48548df91e9eb5d4a46391da0ad0a8cdd3387857:

  Fix effects analysis: field writes clobber object reads (2024-03-20
11:50:53 +0100)

are available in the Git repository at:

  https://git.sr.ht/~rjraymond/guile

for you to fetch changes up to b19ff6b1618de5b5abe13188ce0e359c07215d6b:

  Allow extended HTTP methods as mentioned in RFC 9110 16.1 (2024-03-28
00:34:21 -0400)

----------------------------------------------------------------
Ryan Raymond (1):
      Allow extended HTTP methods as mentioned in RFC 9110 16.1

 module/web/http.scm | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

Ryan

[-- Attachment #2: Type: text/html, Size: 990 bytes --]

RE: Allowing extended HTTP methods

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

As mentioned previous e-mails, either:

• the docstring forgets to mention that it is the caller’s responsibility it is to check that ‘str’ is a valid method name (and you need to check/adjust the callers to do the check, of which I have seen n)
• or 'parse-http-method’ is missing a check that (string-length str)>0 and the characters of str belong to tchar (defined in the RFC).

Please do not ignore reviews when making PRs – disagreeing with some review(s) and then mentioning in the commit message/cover letter/comments/... as appropriate why you deviate from them is one thing, just acting as if they didn’t happen is another.

In the previous discussion I have heard an argument for not doing the check, which basically amounts to flexibility for sake of flexibility in spite of potential security problems and in spite of the third option (i.e. with both security and flexibility) of not allowing it by default and instead adding an argument somewhere to add a custom checker/list of extra allowed methods/... that defaults to what RFC specifies, but I haven’t heard an argument for not properly documenting that what parse-http-method parses aren’t HTTP methods so the callers may need to do extra validation. 

Also, the commit message needs to be changed to changelog format.

Also worth checking if the Texinfo documentation mentions parse-http-method, and if so, adjust it.

Best regards,
Maxime Devos.

[-- Attachment #2: Type: text/html, Size: 6016 bytes --]