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(kernel): cannot pass decorated structs to kernel as "any" #997

Merged
merged 5 commits into from
Nov 18, 2019

Conversation

RomainMuller
Copy link
Contributor

@RomainMuller RomainMuller commented Nov 18, 2019

When a struct is serialized with a $jsii.struct decoration from a jsii host language and passed into a value typed any, the kernel ignored the decoration. This resulted in a malformed object passed to the javascript code.

This fix undecorates the object and continues to deserialize it as any.

Fixes aws/aws-cdk#5066


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mergify
Copy link
Contributor

mergify bot commented Nov 18, 2019

The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged.
[Conventional Commits]: https://www.conventionalcommits.org

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

When a struct is serialized with a $jsii.struct decoration from a jsii host language and passed into a value typed `any`, the kernel ignored the decoration. This resulted in a malformed object passed to the javascript code.

This fix undecorates the object and continues to deserialize it as `any`.

Fixes aws/aws-cdk#5066
@eladb eladb force-pushed the rmuller/struct-tagging branch from cee5b0b to 684a368 Compare November 18, 2019 11:09
@eladb eladb changed the title add compliance test for reproduction of aws/aws-cdk#5066 fix(kernel): cannot pass decorated structs to kernel as "any" Nov 18, 2019
@eladb eladb requested a review from rix0rrr November 18, 2019 11:10
@eladb eladb marked this pull request as ready for review November 18, 2019 11:12
@eladb eladb requested review from bmaizels and a team as code owners November 18, 2019 11:12
@RomainMuller RomainMuller removed the request for review from bmaizels November 18, 2019 11:16
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Contributor Author

@RomainMuller RomainMuller left a comment

Choose a reason for hiding this comment

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

Can't approve as I'm tagged as the author here... But ✅

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@eladb eladb merged commit 2bd3183 into master Nov 18, 2019
@eladb eladb deleted the rmuller/struct-tagging branch November 18, 2019 14:51
RomainMuller added a commit that referenced this pull request Nov 19, 2019
When moving out of callbacks, the result was not turned into a reference
before being passed out to the jsii kernel, causing TypeErrors when
attempting to turn structs (and probably other non-primitive types) to
JSON.

Discovered while attempting to reproduce #1003, which can no longer
trigger as it depended on returning a Struct through and `any` and being
hit with the bug fixed in #997... the fix also removed the conditions
that make leveraging the cause possible (or at the very least I could
not come up with a reproduction, and have verified that the original bug
report no longer reproduces once #997 is in).

That being said, still introduced the necessary changes to address the
cause by properly discovering interfaces and adding the relevant
overrides filtering, as this may improve the performance and reliability
of the jsii runtime for Java.

Fixes #1003
RomainMuller added a commit that referenced this pull request Nov 19, 2019
When moving out of callbacks, the result was not turned into a reference
before being passed out to the jsii kernel, causing TypeErrors when
attempting to turn structs (and probably other non-primitive types) to
JSON.

Discovered while attempting to reproduce #1003, which can no longer
trigger as it depended on returning a Struct through and `any` and being
hit with the bug fixed in #997... the fix also removed the conditions
that make leveraging the cause possible (or at the very least I could
not come up with a reproduction, and have verified that the original bug
report no longer reproduces once #997 is in).

That being said, still introduced the necessary changes to address the
cause by properly discovering interfaces and adding the relevant
overrides filtering, as this may improve the performance and reliability
of the jsii runtime for Java.

Fixes #1003
mergify bot pushed a commit that referenced this pull request Nov 28, 2019
* fix(python): correctly handle structs out of callbacks

When moving out of callbacks, the result was not turned into a reference
before being passed out to the jsii kernel, causing TypeErrors when
attempting to turn structs (and probably other non-primitive types) to
JSON.

Discovered while attempting to reproduce #1003, which can no longer
trigger as it depended on returning a Struct through and `any` and being
hit with the bug fixed in #997... the fix also removed the conditions
that make leveraging the cause possible (or at the very least I could
not come up with a reproduction, and have verified that the original bug
report no longer reproduces once #997 is in).

That being said, still introduced the necessary changes to address the
cause by properly discovering interfaces and adding the relevant
overrides filtering, as this may improve the performance and reliability
of the jsii runtime for Java.

Fixes #1003

* improve documentation & test coverage

* fixup

* inline-fy doc

* remove spurious 'final'

* Add missing fget

* add type annotation to make MyPy happy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

java (dotnet?) regression: unable to use structs with escape hatches
3 participants