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

Modularize passStyleOf #3571

Merged
merged 3 commits into from
Aug 4, 2021
Merged

Modularize passStyleOf #3571

merged 3 commits into from
Aug 4, 2021

Conversation

erights
Copy link
Member

@erights erights commented Aug 1, 2021

Pure refactor: A major internal refactoring, to set things up for upcoming big changes. But this refactor by itself should have no observable effects.

Well, except for the following, which affects no current code:

Before this PR, in order to allow malformed errors to be passed through marshal, we had passStyleOf validate them as copyError, which could therefore be stores within passable structures. The allowed malformedness included the possibility of extra properties leading to surprising power. These would not be passed, so there was no distributed security problem. But the possibility of these extra properties on a Passable creates a local hazard. In this PR, malformed errors are passable in the same way, but are no longer considered Passable.

Well formed errors are still Passable. However errors are no longer Comparable. They cannot be compared using sameStructure.

I have separately verified that this PR in compatible with #3525

@erights erights self-assigned this Aug 1, 2021
@erights erights marked this pull request as ready for review August 1, 2021 05:03
@erights erights changed the title WIP: Modularize passStyleOf Modularize passStyleOf Aug 1, 2021
@erights erights requested a review from michaelfig August 1, 2021 05:04
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

Looks clearly like a useful refactoring. Please consider my comments as the only peculiarities I observed.

packages/marshal/src/helpers/copyArray.js Outdated Show resolved Hide resolved
packages/marshal/src/helpers/error.js Show resolved Hide resolved
packages/marshal/src/helpers/passStyleHelpers.js Outdated Show resolved Hide resolved
packages/marshal/src/passStyleOf.js Outdated Show resolved Hide resolved
packages/marshal/src/structure.js Outdated Show resolved Hide resolved
packages/marshal/src/structure.js Outdated Show resolved Hide resolved
@erights erights enabled auto-merge (squash) August 4, 2021 02:02
@erights erights merged commit e48f286 into master Aug 4, 2021
@erights erights deleted the markm-modular-passables branch August 4, 2021 02:12
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.

2 participants