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

Use reference-equality when omitting validation rules during composition. #3338

Conversation

abernix
Copy link
Member

@abernix abernix commented Sep 25, 2019

The previous technique for deciding which validation rules from specifiedSDLRules to by-pass during composition was leveraging a string-comparison against Function.prototype.name.

As shown in #3335, that technique breaks down under minification, when function names are often munged to shorter alternatives.

As an alternative, we can import the rules and check them for reference equality with greater success, since those will not be affected by minification.

While the actual bug in #3335 was not in this code, this code poses the same hazard and would likely be affected as well (eventually, at least).

…ion.

The previous technique for deciding which validation rules from
`specifiedSDLRules` to by-pass during
composition was leveraging a string-comparison against
`Function.prototype.name`.

As shown in #3335, that
technique breaks down under minification, when function names are often
munged to shorter alternatives.

As an alternative, we can import the rules and check them for reference
equality with greater success, since those will not be affected by
minification.

While the actual bug in #3335 was _not_ in this code, this code poses the
same hazard and would likely be affected as well (eventually, at least).
@abernix abernix force-pushed the abernix/filter-rules-on-reference-equality-rather-than-string-eq branch from 706c3ee to cd55231 Compare September 25, 2019 14:50
@jbaxleyiii jbaxleyiii self-requested a review September 25, 2019 14:58
@abernix abernix merged commit 0bad61d into master Sep 27, 2019
abernix added a commit to apollographql/apollo-tooling that referenced this pull request Sep 30, 2019
Similar in spirit to apollographql/apollo-server#3338.

The technique previously used for removing rules from the standard
`specifiedRules` was leveraging a check on `Function.prototype.name`, rather
than doing direct object equality.  While that does generally work, thanks
to the more recent standardization of `Function.prototype.name`, it still
breaks down under some of the more aggressive minification techniques since
a function's name is not guaranteed to remain the same.

Fixes: apollographql/apollo-server#3335
trevor-scheer pushed a commit to apollographql/apollo-tooling that referenced this pull request Oct 2, 2019
…1551)

Use object equality when filtering rules in `buildSchemaFromSDL`

Similar in spirit to apollographql/apollo-server#3338.

The technique previously used for removing rules from the standard
`specifiedRules` was leveraging a check on `Function.prototype.name`, rather
than doing direct object equality.  While that does generally work, thanks
to the more recent standardization of `Function.prototype.name`, it still
breaks down under some of the more aggressive minification techniques since
a function's name is not guaranteed to remain the same.

Fixes: apollographql/apollo-server#3335
@abernix abernix deleted the abernix/filter-rules-on-reference-equality-rather-than-string-eq branch February 25, 2020 21:02
abernix added a commit to apollographql/federation that referenced this pull request Sep 4, 2020
…ion. (apollographql/apollo-server#3338)

* Use reference-equality when omitting validation rules during composition.

The previous technique for deciding which validation rules from
`specifiedSDLRules` to by-pass during
composition was leveraging a string-comparison against
`Function.prototype.name`.

As shown in apollographql/apollo-server#3335, that
technique breaks down under minification, when function names are often
munged to shorter alternatives.

As an alternative, we can import the rules and check them for reference
equality with greater success, since those will not be affected by
minification.

While the actual bug in apollographql/apollo-server#3335 was _not_ in this code, this code poses the
same hazard and would likely be affected as well (eventually, at least).

* Add changelog entry

Apollo-Orig-Commit-AS: apollographql/apollo-server@0bad61d
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants