Skip to content
This repository has been archived by the owner on Feb 11, 2020. It is now read-only.

Fix for unsubscribe before subscribe scenario #148

Closed
wants to merge 1 commit into from

Conversation

mocheng
Copy link
Contributor

@mocheng mocheng commented Jun 14, 2014

If mqtt client send unsubscribe message before any subscribe message. The (!sub || !sub.handler) expression would raise an exception.

If mqtt client send unsubscribe message before any subscribe message. The (!sub || !sub.handler) expression would raise an exception.
@mcollina
Copy link
Collaborator

Can you please add a unit test for this? Thanks!

@mocheng
Copy link
Contributor Author

mocheng commented Jun 21, 2014

Each subscriptions element has a "handler" field. The (!sub && !sub.handler) is same as (!sub). There is no way to tweak the client subscriptons field in UT.

@mcollina
Copy link
Collaborator

I agree. I think we might want to introduce a subscription object.
See https://github.com/mcollina/mosca/blob/master/lib/client.js#L402 how it is instantiated.

Something like:

function Subscription(qos, handler) {
  this.qos = qos
  this.handler = handler
}

@mcollina
Copy link
Collaborator

Any updates on this?

@mocheng mocheng closed this Jun 24, 2014
@mocheng mocheng deleted the patch-1 branch June 24, 2014 00:53
@mcollina
Copy link
Collaborator

Why closing?

@mocheng
Copy link
Contributor Author

mocheng commented Jun 24, 2014

Oops. Seems I closed the wrong issue.

I recalled that I found this issue while unsubscribing a topic before any subscriptions. There should be a unit test case covering this scenario.

Anyway, mosca server should not crash on unexpected client behaviour.

I'll submit a pull request.

@mocheng mocheng mentioned this pull request Jun 24, 2014
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.

2 participants