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

feat(bridge): support incoming mosquitto bridge connections #584

Merged
merged 7 commits into from
Mar 4, 2021
Merged

feat(bridge): support incoming mosquitto bridge connections #584

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ce460b8
feat(bridge): Support incoming mosquitto bridge connections
phil-mitchell Feb 23, 2021
4b5ffaa
clarify that Bridge Protocol is supported only for incoming connections
phil-mitchell Feb 23, 2021
dc586d2
Respond to some code review comments
phil-mitchell Feb 24, 2021
45b4f0c
Merge branch 'master' into master
phil-mitchell Feb 25, 2021
b4ad77d
Move the retain flag clearing into the publish function
phil-mitchell Feb 26, 2021
1c1e1f2
Revert previous behaviour change and adjust test
phil-mitchell Feb 27, 2021
5e593e9
Merge branch 'master' into master
robertsLando Mar 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ Other info:

- The repo [aedes-tests](https://github.com/moscajs/aedes-tests) is used to test aedes with clusters and different emitters/persistences. Check its source code to have a starting point on how to work with clusters

## Bridge connections

Normally, when publishing a message, the `retain` flag is consumed by Aedes and
then set to `false` so that recipients of the message do not retain it as well.
Copy link
Member

Choose a reason for hiding this comment

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

That's not really correct, it is set to false because Aedes can be used in a cluster env, in such cases the mqemitter is used to share a message between different broker instances, the retain is set to false to prevent other instances to store it again

Copy link
Member

Choose a reason for hiding this comment

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

@mcollina

Copy link
Collaborator

Choose a reason for hiding this comment

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

@robertsLando that's correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably also true, but my understanding of the MQTT spec is that the retain flag needs to be false when received through a subscribe.

http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html

It MUST set the RETAIN flag to 0 when a PUBLISH Packet is sent to a Client because it matches an established subscription regardless of how the flag was set in the message it received [MQTT-3.3.1-9].

Copy link
Member

Choose a reason for hiding this comment

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

First of all I would add this comment there, then sincerly I dunno what's the best way to continue as both seems to have a reson to be accepted. I see no reference on bridge retain handling on specs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bridge retain handling is outside of the standard, so the only spec about bridge retain handling is here: https://github.com/mqtt/mqtt.github.io/wiki/bridge_protocol

However, MQTT v5 does mention retain handling: https://docs.oasis-open.org/mqtt/mqtt/v5.0/mqtt-v5.0.html

The setting of the RETAIN flag in an Application Message forwarded by the Server from an established connection is controlled by the Retain As Published subscription option. Refer to section 3.8.3.1 for a definition of the Subscription Options.
· If the value of Retain As Published subscription option is set to 0, the Server MUST set the RETAIN flag to 0 when forwarding an Application Message regardless of how the RETAIN flag was set in the received PUBLISH packet [MQTT-3.3.1-12].
· If the value of Retain As Published subscription option is set to 1, the Server MUST set the RETAIN flag equal to the RETAIN flag in the received PUBLISH packet [MQTT-3.3.1-13].


Brokers that support the [Bridge Protocol][bridge_protocol] can connect to
Aedes. When connecting with this special protocol, subscriptions work as usual
excecpt that the `retain` flag in the packet is propagated as-is.

## Exensions

- [aedes-logging]: Logging module for Aedes, based on Pino
Expand Down
20 changes: 16 additions & 4 deletions lib/handlers/subscribe.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,19 @@ function SubAck (packet, granted) {
function Subscription (qos, func, rh, rap, nl) {
this.qos = qos
this.func = func

// retain-handling indicates how retained messages should be
// handled when a new subscription is created
// (see [MQTT-3.3.1-9] through [MQTT-3.3.1-11])
this.rh = rh

// retain-as-published indicates whether to leave the retain flag as-is (true)
// or to clear it before sending to subscriptions (false) default false
// (see [MQTT-3.3.1-12] through [MQTT-3.3.1-13])
this.rap = rap

// no-local indicates that a client should not receive its own
// messages (see [MQTT-3.8.3-3])
this.nl = nl
}

Expand Down Expand Up @@ -128,14 +139,15 @@ function addSubs (sub, done) {
const rh = this.rh
const rap = this.rap
const nl = this.nl
const deliverFunc = qos > 0 ? client.deliverQoS : client.deliver0
let func = qos > 0 ? client.deliverQoS : client.deliver0

let func = function handlePacketSubscription (_packet, cb) {
if (!rap) {
if (!rap) {
robertsLando marked this conversation as resolved.
Show resolved Hide resolved
const deliverFunc = func
func = function handlePacketSubscription (_packet, cb) {
_packet = new Packet(_packet, broker)
_packet.retain = false
deliverFunc(_packet, cb)
}
deliverFunc(_packet, cb)
}
phil-mitchell marked this conversation as resolved.
Show resolved Hide resolved

// [MQTT-4.7.2-1]
Expand Down