Re: [pve-devel] applied: [PATCH proxmox 1/2] notify: webhook: gotify: set Content-Length header

[Lukas Wagner <l.wagner@proxmox.com>]

On  2025-03-25 19:41, Thomas Lamprecht wrote:
> Am 21.03.25 um 10:56 schrieb Lukas Wagner:
>> To quote from RFC 9110 [1]:
>>
>>   A user agent SHOULD send Content-Length in a request when
>>   the method defines a meaning for enclosed content and it
>>   is not sending Transfer-Encoding. For example, a user agent
>>   normally sends Content-Length in a POST request even when
>>   the value is 0 (indicating empty content).
>>   A user agent SHOULD NOT send a Content-Length header field
>>   when the request message does not contain content and the
>>   method semantics do not anticipate such data.
>>
>> It seemed like our HTTP client lib did not set the header
>> automatically, which is why we should do it manually.
>>
>> While most services seemed to have worked fine without setting
>> the header, some Microsoft services seem to require it
>> to accept the webhook request [2].
>>
>> [1] https://datatracker.ietf.org/doc/html/rfc9110#name-content-length
>> [2] https://forum.proxmox.com/threads/158827
>>
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>>  proxmox-notify/src/endpoints/gotify.rs  |  4 ++++
>>  proxmox-notify/src/endpoints/webhook.rs | 19 ++++++++++++++++++-
>>  2 files changed, 22 insertions(+), 1 deletion(-)
>>
>>
> 
> applied both patches, thanks!
> 
> FWIW, as it was already encoded in the commit message for posterity I'd
> have been fine with the comment being a bit shorter, e.g., the link to
> the RFC and the last line, but it did not bother me to care amending the
> patch and it's not a clear-cut, or at least subjective, so just a nit.

The brief quote from the RFC gives good context on *why* the change should be done
in a self-contained way without having to go to the RFC text and search for the correct
paragraph. IMO it definitely makes sense to have it in the commit message.

-- 
- Lukas



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel