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

DefaultSubscriptionRegistry should prevent duplicate Subscriptions per subscription id [SPR-15229] #19794

Closed
spring-issuemaster opened this issue Feb 7, 2017 · 6 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Feb 7, 2017

David Fuelling opened SPR-15229 and commented

It appears that the equals() and hashcode() methods have not been implemented in the private class DefaultSubscriptionRegistry.Subscription. This has the effect of allowing duplicate subscriptions with the same identifier into the Set<Subscription> held by each key in SessionSubscriptionInfo.destinationLookup. This seems like a bug, although it's possible that STOMP allows for duplicate subscriptions using the same subscription-Id?


Affects: 4.3.6

Referenced from: commits f219680, ba0484f

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 8, 2017

Rossen Stoyanchev commented

You can have multiple subscriptions (same destination) but subscription id's must be unique. Any idea what's causing two subscription messages with the same destination and id? I wouldn't expect that to happen. That said we should add the hashcode and equals in any case.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 8, 2017

David Fuelling commented

Thanks for the quick response.

I'm implementing a Websocket subprotocol that has the concept of Subscriptions, but none of the other things in STOMP like framing, fallback to SockJS, heartbeat, etc. My implementation is patterned off of the StompSubProtocolHandler because I didn't want to rewrite all of the subscription logic in the STOMP classes, so my implementation is internally using a lot of the SIMP header features that are built into the current STOMP implementation's subscription logic.

In my case, I ended up creating two "subscriptions" using my subprotocol, and couldn't figure out why multiple "subscribe" requests were adding a new subscription every time (duplicating any notifications sent by the system). I narrowed it down to the Set of Subscriptions.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 14, 2017

David Fuelling commented

@Rossen, you indicated that you would not expect two subscription messages with the same destination and id. Can you elaborate on that? I'm not terribly familiar with STOMP, but couldn't a client send a subscribe request again, perhaps in the event of a timeout or network failure or some other retry mechanism in the client?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 21, 2017

Rossen Stoyanchev commented

Not sure if you saw the link to the spec above? It says for the SUBSCRIBE id header:

Since a single connection can have multiple open subscriptions with a server,
an id header MUST be included in the frame to uniquely identify the subscription. 
The id header allows the client and server to relate subsequent MESSAGE or
UNSUBSCRIBE frames to the original subscription.

In other words the same client can subscribe more than once on the same destination. The subscription id helps to correlate messages broadcast from the server to the correct subscription handler.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 17, 2017

Askar Ibragimov commented

Hello, I have a question.

In my program (Spring 4.3.3) that is a websocket/stomp server I have noticed double-messaging behaviour. This is very difficult for me to reproduce, but I went thru Release Notes and found this ticket.

What I observe is : I send (I believe) one message to spring and i get two on client:

[INFO ] 2017-02-28 09:32:57.473 [WebSocketSimpleContainer@2065407355-59] MessagingHelper - Frame, headers: {"x-sequenceNo":["5"],"destination":["/user/userui1/streaming/info/1/10"],"content-type":["application/json;charset=UTF-8"],"subscription":["0"],"message-id":["9842b5d1778e490fae4661fb230314f5-4"],"content-length":["982"]}
[INFO ] 2017-02-28 09:32:57.478 [WebSocketSimpleContainer@2065407355-59] MessagingHelper - Frame, headers: {"x-sequenceNo":["5"], "destination":["/user/userui1/streaming/info/1/10"],"content-type":["application/json;charset=UTF-8"],"subscription":["0"],"message-id":["9842b5d1778e490fae4661fb230314f5-5"],"content-length":["982"]}

where x-sequenceNo is my own header that I attach to each message and that is supposed to be stricly unique; indeed the message payload (not shown) is the same.

I am currently seeking for possible causes of the issue and would like to confirm/deny that the #19794 is relevant to the observed behaviour. Could you please let me know?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 27, 2017

Rossen Stoyanchev commented

It's unlikely this ticket has to do with it. Assuming the subscriptions are made with a STOMP client conforming to the spec then multiple subscriptions on the same session should have unique subscription id's. For this ticket we made a change to the storage of subscriptions to reflect the fact they should be unique but in practice, again, it should never be otherwise as long as the client complies to the spec.

That said your output shows the same subscription id so either the client is misbehaving or there is indeed some issue. It's hard to tell from just two lines of output and with on further context. Please do not add more details here since this issue is attached to releases and hence permanently closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.