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.

Sign up for GitHub

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

Jump to bottom

[GraphQL October 2021] Forbid __typename on subscription root #1001

Merged
merged 10 commits into from Dec 13, 2021
Merged

[GraphQL October 2021] Forbid __typename on subscription root #1001

merged 10 commits into from Dec 13, 2021

Conversation

ilslv
Copy link
Member

@ilslv ilslv commented Dec 8, 2021 •
edited by tyranron

Part of #1000

Synopsis

RFC

Solution

Add validation rule, that errors on __typename query on subscription root.

Checklist

  • Created PR:
    • In draft mode
    • Name contains Draft: prefix
    • Name contains issue reference
    • Has assignee
  • Documentation is updated (if required)
  • Tests are updated (if required)
  • Changes conform code style
  • CHANGELOG entry is added (if required)
  • FCM (final commit message) is posted
    • and approved
  • Review is completed and changes are approved
  • Before merge:
    • Milestone is set
    • PR's name and description are correct and up-to-date
    • Draft: prefix is removed
    • All temporary labels are removed

@ilslv ilslv added enhancement Improvement of existing features or bugfix semver::breaking Breaking change in terms of SemVer labels Dec 8, 2021
@ilslv ilslv self-assigned this Dec 8, 2021
@ilslv ilslv mentioned this pull request Dec 9, 2021
Closed
6 tasks
@ilslv
Copy link
Member Author

ilslv commented Dec 9, 2021 •
edited

FCM

Forbid `__typename` on subscription root (#1001, #1000)

Additionally:
- upgrade `actix` betas in `juniper_actix`

@ilslv ilslv changed the title Draft: [GraphQL October 2021] Forbid __typename on subscription root [GraphQL October 2021] Forbid __typename on subscription root Dec 9, 2021
@ilslv ilslv marked this pull request as ready for review December 9, 2021 12:11
@ilslv ilslv requested review from tyranron and removed request for tyranron December 9, 2021 12:12
tyranron
tyranron requested changes Dec 9, 2021
View reviewed changes
Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

Anything else seems good.

tyranron
tyranron reviewed Dec 9, 2021
View reviewed changes
juniper/src/validation/rules/fields_on_correct_type.rs
if let OperationType::Subscription = operation.item.operation_type {
for selection in &operation.item.selection_set {
if let Selection::Field(field) = selection {
if field.item.name.item == "__typename" {
Copy link
Member

Choose a reason for hiding this comment

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

We should add tests fully covering this in this module below along with others.

@ilslv ilslv requested a review from tyranron December 13, 2021 08:20
tyranron
tyranron approved these changes Dec 13, 2021
View reviewed changes
Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

Anything else is nice 👍

juniper_actix/Cargo.toml Outdated
@@ -13,10 +13,10 @@ subscriptions = ["juniper_graphql_ws", "tokio"]

[dependencies]
actix = "0.12"
actix-http = "3.0.0-beta.13"
actix-http = "3.0.0-beta.15"
Copy link
Member

Choose a reason for hiding this comment

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

Please, disolve such thing to a separate PR next time.

@tyranron tyranron merged commit e264cf5 into master Dec 13, 2021
@tyranron tyranron deleted the forbid_typename_on_subscription_root branch December 13, 2021 12:27
@tyranron tyranron added this to the 0.16.0 milestone Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyranron tyranron tyranron approved these changes

Assignees

@ilslv ilslv

Labels
enhancement Improvement of existing features or bugfix semver::breaking Breaking change in terms of SemVer
Projects
None yet
Milestone
0.16.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ilslv @tyranron