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

[RFC] ID uniqueness should be enforced for both Single result and Streaming operation #95

Closed
hoangvvo opened this issue Jan 12, 2021 · 3 comments · Fixed by #96
Closed
Labels
bug Something isn't working documentation Improvements or additions to documentation good first issue Issue tackles a good aspect released Has been released and published

Comments

@hoangvvo
Copy link

hoangvvo commented Jan 12, 2021

Story

We have already talked about this once in graphql/graphql-over-http#140 (comment), but I have one confusion.

Let's consider a scenerio of a faulty client (which is usually the culprit for conflict id cases anyway) sending both a mutation and a subscription using the same id.

The client first requests a subscription with id="a". Client does not want to end this subscription yet.

Then the client then sends a mutation with id="a". This is obvious not a problem for the server. However, when the server returns the result of that mutation via type=next,id=a, the client cannot tell if the incoming result is for Subscription with id="a" or Mutation with id="a".

Then the server proceeds to send type=Complete,id=a. However, in this case, it also accidentally makes the client thinks that it is unsubscribed from the the initial subscription query.

@enisdenjo enisdenjo added documentation Improvements or additions to documentation good first issue Issue tackles a good aspect labels Jan 12, 2021
@enisdenjo
Copy link
Owner

enisdenjo commented Jan 12, 2021

Wow, excellent example! I haven't thought about that. That most certainly can lead to unintended behaviour and possible vector attacks.

Following this revelation, enforcing ID uniqueness across all operations is a must. I will adapt the Protocol and the library accordingly.

@enisdenjo enisdenjo added the bug Something isn't working label Jan 12, 2021
@enisdenjo
Copy link
Owner

I'll even classify this as a bug since it could already happen in the server from a rouge client. If you first issue a long running single result operation, and then a subscription, the behaviour you described will occur.

@enisdenjo
Copy link
Owner

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@enisdenjo enisdenjo added the released Has been released and published label Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation good first issue Issue tackles a good aspect released Has been released and published
Projects
None yet
2 participants