mailjet / mailjet-apiv3-nodejs

[API v3] Official Mailjet API v3 NodeJS wrapper
https://dev.mailjet.com
MIT License
236 stars 69 forks source link

[WIP] GET requests wrongly set `Content-Length` header #248

Closed SebastianBair closed 1 year ago

SebastianBair commented 1 year ago

When using Squid as proxy for Mailjet API requests via this SDK (in our case to solve similar connection errors as described / discussed in https://github.com/mailjet/mailjet-apiv3-nodejs/issues/176) the proxy rejects all GET calls (e.g. list contact lists or contact properties). This is caused by the Content-Length incorrectly being set to 2 which is technically invalid HTTP (see HTTP Semantics Section 8.6 which states 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).

The reason for the Content-Length header always being set to 2 for GET requests is this line in the SDK code: https://github.com/mailjet/mailjet-apiv3-nodejs/blob/v6.0.2/lib/request/index.ts#L93 \ This method returns an empty object for the body data provided to Axios that has a JSON-encoded length of 2. \ I would assume Axios calculates the length and sets it in the header because an empty object is still truthy. It might be possible that Axios has fixed this / added special handling for GET requests in a newer version (between the currently used 0.27.2 and their latest version 1.4.0), haven't checked this though.

As a quick fix I changed the method linked above to return undefined which is falsy and correctly causes Axios not to set the Content-Length error (see my fork: https://github.com/JoinMyTrip/mailjet-apiv3-nodejs/commit/9e6b424d5e5e1bc3696b8c84a24b1fc21b0d1eb8). I'm opening an issue instead of submitting a Pull Request because my quick fix surely didn't correctly cover all cases in your repository (briefly saw that your tests check for an empty object for GET requests for example).

The error is not visible when directly connecting to the Mailjet API server without a proxy but it's still technically invalid HTTP caused by the SDK.

ai-wintermute commented 1 year ago

Hi @SebastianBair thank you for your collaboration! The new release with the fix was published: v6.0.4