APIGW: binary support for non-proxy integration #12215
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
2052964 to
768d6df
Compare
349b657 to
294e374
Compare
768d6df to
4d05456
Compare
4d05456 to
3afb9e3
Compare
LocalStack Community integration with Pro 2 files 2 suites 29m 22s ⏱️ Results for commit 3afb9e3. |
|
Currently, only patch changes are allowed on master. Your PR labels (aws:apigateway, semver: minor) indicate that it cannot be merged into the master at this time. |
cloutierMat
approved these changes
Feb 7, 2025
Comment on lines
+262
to
+277
| if is_binary_request: | ||
| if content_handling and content_handling == ContentHandlingStrategy.CONVERT_TO_TEXT: | ||
| body = base64.b64encode(body) | ||
| # if the content handling is not defined, or CONVERT_TO_BINARY, we do not touch the body and leave it as | ||
| # proper binary | ||
| else: | ||
| if not content_handling or content_handling == ContentHandlingStrategy.CONVERT_TO_TEXT: | ||
| body = body.decode(encoding="UTF-8", errors="replace") | ||
| else: | ||
| # it means we have CONVERT_TO_BINARY, so we need to try to decode the base64 string | ||
| try: | ||
| body = base64.b64decode(body) | ||
| except ValueError: | ||
| raise InternalServerError("Internal server error") | ||
|
|
||
| return body |
There was a problem hiding this comment.
nice digging! There are a few hoops to jump through here as well 🚀
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Follow up from #12199, is reviewable
but will come out of draft once the base is merged to avoid the rebase review madness.This PR implements the full Binary media support for REST APIs. The AWS documentation will do a much better job explaining what this means exactly, but in big line, it allows you to define some behavior in how your payload in being pass through to your integration: it can be pass as is, converted to a safe UTF-8 string, or base64 decoded or encoded.
There are precise tables on the behavior, but I tend to not trust the documentation so much, so I took it upon myself to write AWS validated tests, which was to be honest, quite a pain again 😅
The behavior in itself is not too complicated, it is mapping some table to if conditions (which might be simplified, if you see it during the review, please say so, I didn't optimize the conditions).
Some changes in behavior is how the
responseTemplatesVTL rendering actually needs the value to be a string, and so if it comes out of the "converter", it needs to be of the right type. We might want to add better log statements to help the user understand why it would fail for them.I've also sneaked some
TODOremoval, and a small change inupdate_integration_response, this API op really needs to be validated and fixed.Resources:
Changes
Content-Typeheadersmodify the method(ported back to base PR) to check if a request if of "binary" type (AWS terms), meaning it matches thebinaryMediaTypesUpdateIntegrationResponseto support our test case