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

Unable to have an API route that returns binary data #56

Closed
john-tipper opened this issue Nov 23, 2022 · 5 comments
Closed

Unable to have an API route that returns binary data #56

john-tipper opened this issue Nov 23, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@john-tipper
Copy link
Contributor

john-tipper commented Nov 23, 2022

I want to have an API route that returns binary data - by way of example, let's say that there's an API route that generates a dynamic image /api/image/generate and where the content is returned by something looking like this:

export default async (req: NextApiRequest, res: NextApiResponse) => {

...
  // buffer contains the image of type defined by mime, is of type Buffer
  res.setHeader('Content-Type', mime);
  res.setHeader('Content-Length', buffer.length);
  res.status(200).send(buffer);

}

If I have run Nextjs locally, this works fine. I can enter the API route directly into the browser and the image will render. However, if I run this in nextjs-lambda, I get an error. Specifically, the content length of the actual data returned is very much larger than the actual image size. I have an image of size 224214 and the content length reported by the server lambda is 403034 and the content is corrupted.

The issue is that the API Gateway (HTTP API) that sits in front of the lambda is not recognising the fact that the data being returned is binary and is mangling it (https://aws.amazon.com/blogs/compute/handling-binary-data-using-amazon-api-gateway-http-apis/). This library explicitly sets the response from the lambda to be non-binary: https://github.com/sladg/nextjs-lambda/blob/master/lib/standalone/server-handler.ts#L39

I don't see what the harm would be to remove that config flag: when we look at the way the serverless-http response from the Nextjs lambda is formatted, if the binary option is not specified then the response will not be treated as binary unless the underlying lambda returns a binary header or the Content-Type is set to be a binary type (by matching a type defined in process.env.BINARY_CONTENT_TYPES). Specifically, by not setting that flag we would hit this if statement.

Could we please consider removing that line?

This would allow for setting BINARY_CONTENT_TYPES=image/png,image/jpg,image/jpeg or similar and then such content would be returned to API Gateway in a way that APIG can handle it appropriately.

john-tipper added a commit to john-tipper/nextjs-lambda that referenced this issue Nov 23, 2022
@sladg
Copy link
Owner

sladg commented Nov 23, 2022

Hey! Yeah this scenario makes sense and I understand the value of allowing for binary data to be returned.

Looking at the serverless-http configuration, it might actually make sense to ditch the configuration and env variables and just set it to true?

That would basically allow any binary data to be returned from API routes without need to configuration, what you think?

It seems like binary is set to true by default anyways. https://github.com/dougmoscrop/serverless-http/blob/master/docs/ADVANCED.md#binary-mode

@sladg sladg added the enhancement New feature or request label Nov 23, 2022
@john-tipper
Copy link
Contributor Author

john-tipper commented Nov 23, 2022 via email

@sladg
Copy link
Owner

sladg commented Nov 23, 2022

Feel free to edit your PR, I will be happy to approve. Alternatively, I will get to it over a weekend

@john-tipper
Copy link
Contributor Author

john-tipper commented Nov 23, 2022 via email

sladg pushed a commit that referenced this issue Nov 23, 2022
* #56 Allow an API route to return binary data by not hardcoding the response as non-binary.

* Set binary mode to be true
@sladg
Copy link
Owner

sladg commented Nov 23, 2022

Merged ✅

@sladg sladg closed this as completed Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants