maciejtreder / serverless-apigw-binary

Serverless plugin for binary files support in AWS Gateway
MIT License
160 stars 33 forks source link

Updates API Gateway but PNG images are still base64 encoded. #33

Open richburdon opened 7 years ago

richburdon commented 7 years ago

The plugin successfully updates the API Gateway's binary support types, but when I try to serve PNG files from aws-serverless-express I'm still getting base64 encoded data returned from:

  app.use('/static', express.static(ENV.STATIC_DIR));

If I explicitly add an accept header with image/png then the correct image data is returned, but I can't do that from an <img/> tag.

Any suggestions? (seems like a popular issue):

https://forum.serverless.com/t/returning-binary-data-jpg-from-lambda-via-api-gateway/796 https://forum.serverless.com/t/force-return-of-binary-response-ignoring-accept-header/1299 https://github.com/maciejtreder/serverless-apigw-binary/issues/33 https://github.com/awslabs/aws-serverless-express/issues/39 https://github.com/awslabs/aws-serverless-express/issues/99 https://github.com/awslabs/aws-serverless-express/issues/104

jleclanche commented 6 years ago

Yeah I'd love to know as well. This sounds like an issue with the API Gateway though, right? I think it doesn't understand that given a generic Accept: */* header, it should decode the base64.

What I really don't get is that, if I return isBase64Encoded: true in the callback, the API gateway should just take it and decode it. It's never okay not to...

Doesn't work:

Works:

jleclanche commented 6 years ago

Note that the following works around it if you always return binary:

custom:
  apigwBinary:
    types:
      - "*/*"
simlu commented 6 years ago

You got to be kidding. I spend so much time on this. URG...

To provide some value: It will work if you change the endpoint to end with e.g. ".png".

rmm5t commented 6 years ago

What I really don't get is that, if I return isBase64Encoded: true in the callback, the API gateway should just take it and decode it. It's never okay not to...

Agreed.

Note that the following works around it if you always return binary:

custom:
  apigwBinary:
    types:
      - "*/*"

I experienced the same problem here. @jleclanche's suggestion was the only fix that was reliable and reasonable.

If it's effectively necessary to specify */* as the accepted type to get this plugin to work for most file types, why not have the plugin just default the types to ['*/*'] (thereby avoiding the need to add a custom block to configure it)?

Let me know if a PR would be welcome to make */* the default type. I'm happy to submit one.

jesusgollonet commented 6 years ago

Let me know if a PR would be welcome to make */* the default type. I'm happy to submit one.

@rmm5t I would advise against doing that. I just implemented this plugin in an already working service with other unrelated endpoints. Everything stopped working and I couldn't figure out why. Turns out that by using types:'*/*' i was now base64 encoding all my other payloads. I reverted back and things went back to normal.

I guess that's where @jleclanche's " if you always return binary" comes from

simlu commented 6 years ago

@jesusgollonet Can't you change your endpoint name to end with the image extension you're serving?

jesusgollonet commented 6 years ago

yeah, that's what i'm doing now. my comment was that making '*/*' the default would lead to confusion when integrating this into projects

jesusgollonet commented 6 years ago

fwiw, in case someone runs into the same... (hope it's ok to post here) As far as I can tell, using */* is the only workable solution if you want to serve images to be embedded in img tags, even using a path that ends in .png

The closest thing I got to avoid having to split my image service into a different one was adding the image extension and using image/* as a mime type. That works very well.. in chrome. Firefox and safari are actually sending */* for img requests.

So until API Gateway starts honoring isBase64Encoded : true regardless of the Accept header, integrating binary responses into a bigger service should be avoided or at least taken with good care (I've had to split the image service into a new one)

Townsheriff commented 4 years ago

In my case / did not work. Browser was calling with image/webp,image/apng,image/*,*/*;q=0.8 accept header. I suppose */* did not match the header value. Changing to image/* worked for me.

jonathanalberghini commented 4 years ago

This does not work. I cannot post json to any other functions with this plugin? Does anyone have a solutions for this.