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.

Reply via email to