Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(aws-lambda,netlify): base64 encode binary responses #1274

Merged
merged 29 commits into from
Jul 21, 2023
Merged

Conversation

harlan-zw
Copy link
Contributor

@harlan-zw harlan-zw commented May 31, 2023

🔗 Linked issue

See https://nuxt-og-image-playground-netlify.netlify.app/api/resvg.png

Code
import { defineEventHandler, setHeader } from 'h3'
import { Resvg } from '@resvg/resvg-js'

export default defineEventHandler(async (e) => {
    const resvgJS = new Resvg('<svg xmlns="http://www.w3.org/2000/svg" version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:svgjs="http://svgjs.dev/svgjs" viewBox="0 0 700 700" width="700" height="700"><defs><linearGradient gradientTransform="rotate(150, 0.5, 0.5)" x1="50%" y1="0%" x2="50%" y2="100%" id="ffflux-gradient"><stop stop-color="hsl(315, 100%, 72%)" stop-opacity="1" offset="0%"></stop><stop stop-color="hsl(227, 100%, 50%)" stop-opacity="1" offset="100%"></stop></linearGradient><filter id="ffflux-filter" x="-20%" y="-20%" width="140%" height="140%" filterUnits="objectBoundingBox" primitiveUnits="userSpaceOnUse" color-interpolation-filters="sRGB">\n' +
        '  <feTurbulence type="fractalNoise" baseFrequency="0.005 0.003" numOctaves="2" seed="2" stitchTiles="stitch" x="0%" y="0%" width="100%" height="100%" result="turbulence"></feTurbulence>\n' +
        '  <feGaussianBlur stdDeviation="20 0" x="0%" y="0%" width="100%" height="100%" in="turbulence" edgeMode="duplicate" result="blur"></feGaussianBlur>\n' +
        '  <feBlend mode="color-dodge" x="0%" y="0%" width="100%" height="100%" in="SourceGraphic" in2="blur" result="blend"></feBlend>\n' +
        '  \n' +
        '</filter></defs><rect width="700" height="700" fill="url(#ffflux-gradient)" filter="url(#ffflux-filter)"></rect></svg>', {
        logLevel: 'trace',
        fitTo: {
            mode: 'width',
            value: 1200,
        },
    })
    setHeader(e, 'Content-Type', 'image/png')
    return resvgJS.render().asPng()
})

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Netlify uses AWS Lambda. AWS Lambda requires sending buffers as strings (they are re-encoded via a proxy afaik).

Nitro currently converts all buffers to default toString(). This breaks images, as they need to be sent with base64 encoding (see https://aws.amazon.com/blogs/compute/handling-binary-data-using-amazon-api-gateway-http-apis/), with a special flag isBase64Encoded.

The fix is to check if we're working with a buffer and the response is an image, then use base 64 encoding. This may affect other response types though.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@harlan-zw harlan-zw requested a review from pi0 May 31, 2023 15:28
harlan-zw added a commit to nuxt-modules/og-image that referenced this pull request May 31, 2023
@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Merging #1274 (a5547c9) into main (421d625) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1274   +/-   ##
=======================================
  Coverage   76.21%   76.22%           
=======================================
  Files          73       73           
  Lines        7433     7435    +2     
  Branches      728      728           
=======================================
+ Hits         5665     5667    +2     
  Misses       1767     1767           
  Partials        1        1           

see 1 file with indirect coverage changes

@@ -30,9 +31,20 @@ export async function lambda(
body: event.body, // TODO: handle event.isBase64Encoded
});

const headers = normalizeOutgoingHeaders(r.headers);
// image buffers must be base64 encoded
if (Buffer.isBuffer(r.body) && headers["content-type"].startsWith("image/")) {
Copy link
Contributor Author

@harlan-zw harlan-zw May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this header check is hacky, looking for a better alternative

maybe it could just be omitted?

Copy link
Member

@Hebilicious Hebilicious Jul 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we trust mime.getType to return the correct type, this should be fine for images, as all images types should start with image

However we should do this for all Binary media for lambda based presets.

As always with AWS, the list of supported media types isn't documented anywhere, altough, chatGPT gives me this:

image/png
image/jpeg
image/gif
application/octet-stream
application/pdf
application/x-zip-compressed
audio/mpeg
audio/mp4
video/mp4
application/zip
audio/webm
video/webm

Which sorts of make sense... But I think a cleaner way to handle all possible use cases without relying on the header would be to use a package such as https://www.npmjs.com/package/isbinaryfile, because I'm not sure Buffer works in all scenarios.

We might also need to rely on the header anyways, as what AWS does with Binary files which aren't media (whatever they mean) is currently a mistery.

@harlan-zw harlan-zw changed the title fix(netlify): support images responses fix(netlify): support image responses May 31, 2023
@harlan-zw harlan-zw changed the title fix(netlify): support image responses fix(aws-lambda,netlify): support image responses May 31, 2023
@pi0
Copy link
Member

pi0 commented Jul 19, 2023

Indeed it should be also generic to support any blob/binrary response regardless of mime type.

Copy link
Member

@Hebilicious Hebilicious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pi0 pi0 changed the title fix(aws-lambda,netlify): support image responses fix(aws-lambda,netlify): base64 encode binary responses Jul 20, 2023
src/runtime/utils.lambda.ts Outdated Show resolved Hide resolved
@pi0 pi0 merged commit 7b8475c into main Jul 21, 2023
6 checks passed
@pi0 pi0 deleted the fix/netlify-images branch July 21, 2023 10:59
@pi0 pi0 mentioned this pull request Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants