bytebeamio / rumqtt

The MQTT ecosystem in rust
Apache License 2.0
1.64k stars 252 forks source link

fix(rumqtt): validate topic and topic filter #813

Open arunanshub opened 8 months ago

arunanshub commented 8 months ago

Type of change

Checklist:

Fixes #595

swanandx commented 8 months ago

note that it won't fix #595 as broker still doesn't perform the checks, and doing that in separate PR is preferable. I would recommend to remove that from PR description so it won't automatically close the issue if this PR is merged.

arunanshub commented 8 months ago

note that it won't fix https://github.com/bytebeamio/rumqtt/issues/595 as broker still doesn't perform the checks

It is still an ongoing work.

Since broker will perform the same task, I will implement it in this PR itself.

coveralls commented 8 months ago

Pull Request Test Coverage Report for Build 8519279552

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
rumqttc/src/v5/mqttbytes/mod.rs 20 21 95.24%
rumqttd/src/protocol/v4/publish.rs 0 3 0.0%
rumqttd/src/protocol/v4/unsubscribe.rs 0 3 0.0%
rumqttd/src/protocol/v5/publish.rs 0 3 0.0%
rumqttd/src/protocol/v5/subscribe.rs 0 3 0.0%
rumqttd/src/protocol/v5/unsubscribe.rs 0 3 0.0%
rumqttc/src/v5/mqttbytes/v5/subscribe.rs 0 4 0.0%
rumqttd/src/protocol/v4/subscribe.rs 0 4 0.0%
rumqttc/src/mqttbytes/v4/subscribe.rs 0 5 0.0%
rumqttd/src/protocol/mod.rs 1 28 3.57%
<!-- Total: 45 164 27.44% -->
Totals Coverage Status
Change from base Build 8432798356: 36.5%
Covered Lines: 6017
Relevant Lines: 16488

💛 - Coveralls
arunanshub commented 7 months ago

Hello @flxo. The hot paths that you mention use generic functions (eg. impl AsRef<str>). According to this discussion on Reddit, in release builds the generic functions are inlined. Furthermore, according to matklad on his blog, private functions are always inlined. I believe that all of these factors should help with performance.

Also feel free to review the changes and suggest any scope of improvements, should you happen to find any.

flxo commented 7 months ago

Hello @flxo. The hot paths that you mention use generic functions (eg. impl AsRef<str>). According to this discussion on Reddit, in release builds the generic functions are inlined. Furthermore, according to matklad on his blog, private functions are always inlined. I believe that all of these factors should help with performance.

Sure - this all helps. I'm just curious if someone used e.g criterion to verify that best effort is done.

Also feel free to review the changes and suggest any scope of improvements, should you happen to find any.

Sure.