Base64-encode content if content-encoding indicates that content is compressed, add enforceBase64 option

[melchor629]

Add support to use base64 encoding when the header Content-Encoding is set for compressed responses. See #117

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[Uzlopak]

I dont get it. If we base64 encode it it gets about 33% bigger. How does this make response.body smaller?

[melchor629]

I dont get it. If we base64 encode it it gets about 33% bigger. How does this make response.body smaller?

As discussed in the issue, we compress the response. In our case, generally, we get more savings even with the trade-off of base64 being 33% bigger. So if a response is about 5MB, when compressed imagine it is like 2MB, then 33% when converted in base64 is like 2,66MB, which is still acceptable for AWS Lambda.

So it is not to make it smaller, it is because the response is already smaller (because of compression), but the code does not handle compressed responses and does not tell the response is binary.

Hope it is clear now :)

[Uzlopak]

Still it strange that base64 gets accepted but normal binary stream not.

It is maybe still better to replace the implicit magic and sending the content as dataUrls

[adrai]

currently driving through half italy... (writing from the wifi connection of an autogrill)
if solved via option (and userland functionality) lgtm.

[adrai]

@melchor629 can you please also extend the types? https://github.com/fastify/aws-lambda-fastify/blob/master/index.d.ts#L9 and add a test? https://github.com/fastify/aws-lambda-fastify/blob/master/index.test-d.ts

[melchor629]

@melchor629 can you please also extend the types? https://github.com/fastify/aws-lambda-fastify/blob/master/index.d.ts#L9 and add a test? https://github.com/fastify/aws-lambda-fastify/blob/master/index.test-d.ts

Yeah sure, I completely forgot to update those (even though I had seen them). Thanks.

[Uzlopak]

So you can pass a isBinary function which then enforces base64 encoding? Is it normal that binary data is base64 encoded when coming from aws?

I would actually prefer a function called enforceBase64 or so, where the implementor gets the request and response object and then is able to do anything with the request and or response. I think of a custom function which checks for a X-Base64-Enforce header in the request and then return true resulting. So a client can actually can control the the payload like Accept-Encoding does give the Client the control of the Response.

[melchor629]

So you can pass a isBinary function which then enforces base64 encoding? Is it normal that binary data is base64 encoded when coming from aws?

I would actually prefer a function called enforceBase64 or so, where the implementor gets the request and response object and then is able to do anything with the request and or response. I think of a custom function which checks for a X-Base64-Enforce header in the request and then return true resulting. So a client can actually can control the the payload like Accept-Encoding does give the Client the control of the Response.

In the response to AWS API Gateway, if the body is binary, it must be encoded in base64 (see example or this blog post ). AWS made binary responses this way :/

If you prefer, I can rename the isBinary to enforceBase64 and pass the request as well.

Edit: note that this is only for the response, made a mistake in last commit using request instead of reply (already fixed that).

[Uzlopak]

But isnt your use case that you have non binary data like application/json which you purposefully encode to base64 to bypass the limitations?

[melchor629]

But isnt your use case that you have non binary data like application/json which you purposefully encode to base64 to bypass the limitations?

Not exactly, but yes. My case is that the JSON is compressed, so it is no longer text: it is now binary. Because of that, the response must be encoded in base64 (as per spec). The content is still application/json but compressed following the Content-Encoding header (in the response).

[Uzlopak]

Strange. After reading over the article i think we actually have to somehow check if we successfully compressed the data and then base64 it. Because what happens if we set content-encoding to identity?

[melchor629]

Strange. After reading over the article i think we actually have to somehow check if we successfully compressed the data and then base64 it. Because what happens if we set content-encoding to identity?

Currently, it is left to implementors of the function to check for those cases. @fastify/aws-lambda will not enforce the base64 conversion if the header is set.

My initial proposal was for checking the Content-Encoding header directly, but there was a request to change it for a more generic function (the one currently in this PR).

[Uzlopak]

To be honest i think your first approach was more appropriate, as compresssed binary data has to be base64 encoded in order to be passed throught aws gateway according to your linked blog post

So it should have been something like

const isCompressed = res.headers['content-encoding'] && res.headers['content-encoding'] !== 'identity'

And that combined with isBase64Encoded.

This would actually respect that we are maybe internally using fastify compress.

[Uzlopak]

Sry, typed with mobile. Accidently closed

[Uzlopak]

Well i think your first proposal was the better fit, because with a isCompressed we can actually wire fastify compress and this library under the hood. But with this PR you have to wire that manually write every time you use fastify compress with aws. And how does a dev know when and why he should use isBinary? Should it be only when content-encoding is not empty? Or like I suggest when content encoding is not empty and not 'identity' (which means also not compressed)

[melchor629]

Well i think your first proposal was the better fit, because with a isCompressed we can actually wire fastify compress and this library under the hood. But with this PR you have to wire that manually write every time you use fastify compress with aws. And how does a dev know when and why he should use isBinary? Should it be only when content-encoding is not empty? Or like I suggest when content encoding is not empty and not 'identity' (which means also not compressed)

Yep, that's better. Having both options will make it work out-of-the-box with fastify compress, but leaves the dev to further customise the base64 encoding with the custom function.

[zekth]

Some optimization remarks.

[adrai]

lgtm

[Uzlopak]

LGTM

But I would like to wait for feedback of @climba03003

[adrai]

@climba03003 ?

[climba03003]

LGTM

[adrai]

Thank you to all for the contributions....
This change is included in v3.1.0