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

hmq does not close connection when receiving invalid utf-8 #104

Open
stapelberg opened this issue Jan 7, 2021 · 2 comments
Open

hmq does not close connection when receiving invalid utf-8 #104

stapelberg opened this issue Jan 7, 2021 · 2 comments

Comments

@stapelberg
Copy link
Contributor

Steps to reproduce:

  1. Run hmq
  2. Run mosquitto_sub -t '#'
  3. Run this Go program:
package main

import (
	"fmt"
	"log"

	mqtt "github.com/eclipse/paho.mqtt.golang"
)

func publishInvalid() error {
	opts := mqtt.NewClientOptions().AddBroker("tcp://localhost:1883")
	opts.SetClientID("dhcp4d")
	mqttClient := mqtt.NewClient(opts)
	if token := mqttClient.Connect(); token.Wait() && token.Error() != nil {
		return fmt.Errorf("MQTT connection failed: %v", token.Error())
	}

	token := mqttClient.Publish("foo/valid", 0, true /* retained */, []byte("payload"))
	token.Wait()

	token = mqttClient.Publish("foo/invalid/\xec\x8c\x04\x10\x27", 0, true /* retained */, []byte("payload"))
	token.Wait()

	return nil
}

func main() {
	if err := publishInvalid(); err != nil {
		log.Fatal(err)
	}
}

Now, mosquitto_sub is in an infinite reconnect-loop, and hmq will print:

{"level":"info","timestamp":"2021-01-07T22:45:12.810+0100","logger":"broker","caller":"broker/broker.go:190","msg":"Start Listening client on ","hp":"0.0.0.0:1883"}
{"level":"info","timestamp":"2021-01-07T22:45:13.853+0100","logger":"broker","caller":"broker/broker.go:269","msg":"read connect from ","clientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:17.454+0100","logger":"broker","caller":"broker/broker.go:269","msg":"read connect from ","clientID":"dhcp4d"}
{"level":"error","timestamp":"2021-01-07T22:45:17.454+0100","logger":"broker","caller":"broker/client.go:170","msg":"read packet error: ","error":"EOF","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"error","timestamp":"2021-01-07T22:45:17.454+0100","logger":"broker","caller":"broker/client.go:170","msg":"read packet error: ","error":"EOF","ClientID":"dhcp4d"}
{"level":"info","timestamp":"2021-01-07T22:45:18.455+0100","logger":"broker","caller":"broker/broker.go:269","msg":"read connect from ","clientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:18.455+0100","logger":"broker","caller":"broker/client.go:514","msg":"process retain  message: ","packet":"SUBSCRIBE: dup: false qos: 1 retain: false rLength: 6 MessageID: 2 topics: [#]","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:18.455+0100","logger":"broker","caller":"broker/client.go:514","msg":"process retain  message: ","packet":"SUBSCRIBE: dup: false qos: 1 retain: false rLength: 6 MessageID: 2 topics: [#]","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"error","timestamp":"2021-01-07T22:45:18.455+0100","logger":"broker","caller":"broker/client.go:170","msg":"read packet error: ","error":"EOF","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:19.457+0100","logger":"broker","caller":"broker/broker.go:269","msg":"read connect from ","clientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"error","timestamp":"2021-01-07T22:45:19.457+0100","logger":"broker","caller":"broker/client.go:170","msg":"read packet error: ","error":"EOF","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:19.457+0100","logger":"broker","caller":"broker/client.go:514","msg":"process retain  message: ","packet":"SUBSCRIBE: dup: false qos: 1 retain: false rLength: 6 MessageID: 3 topics: [#]","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:19.457+0100","logger":"broker","caller":"broker/client.go:514","msg":"process retain  message: ","packet":"SUBSCRIBE: dup: false qos: 1 retain: false rLength: 6 MessageID: 3 topics: [#]","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:20.458+0100","logger":"broker","caller":"broker/broker.go:269","msg":"read connect from ","clientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:20.458+0100","logger":"broker","caller":"broker/client.go:514","msg":"process retain  message: ","packet":"SUBSCRIBE: dup: false qos: 1 retain: false rLength: 6 MessageID: 4 topics: [#]","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:20.458+0100","logger":"broker","caller":"broker/client.go:514","msg":"process retain  message: ","packet":"SUBSCRIBE: dup: false qos: 1 retain: false rLength: 6 MessageID: 4 topics: [#]","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"error","timestamp":"2021-01-07T22:45:20.458+0100","logger":"broker","caller":"broker/client.go:170","msg":"read packet error: ","error":"EOF","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:21.459+0100","logger":"broker","caller":"broker/broker.go:269","msg":"read connect from ","clientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:21.459+0100","logger":"broker","caller":"broker/client.go:514","msg":"process retain  message: ","packet":"SUBSCRIBE: dup: false qos: 1 retain: false rLength: 6 MessageID: 5 topics: [#]","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"info","timestamp":"2021-01-07T22:45:21.459+0100","logger":"broker","caller":"broker/client.go:514","msg":"process retain  message: ","packet":"SUBSCRIBE: dup: false qos: 1 retain: false rLength: 6 MessageID: 5 topics: [#]","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
{"level":"error","timestamp":"2021-01-07T22:45:21.459+0100","logger":"broker","caller":"broker/client.go:170","msg":"read packet error: ","error":"EOF","ClientID":"mosq-jauBjjAjwJcc1Sy865"}
^C2021/01/07 22:45:22 signal received, broker closed. interrupt

As per mqttjs/mqtt-packet#59, hmq should instead verify that topic names are valid utf-8, and close the connection when a client sends invalid utf-8.

chowyu08 pushed a commit that referenced this issue Feb 26, 2021
* chore: ignore .pre-commit-config.yaml

Signed-off-by: Lucas Vieira <lucas.engen.cc@gmail.com>

* fix: 🐛 perform validation on control packet fields (#104)

Signed-off-by: Lucas Vieira <lucas.engen@outlook.com>

* feat: ❇️ add handling of null UTF-8 encoded character

Signed-off-by: Lucas Vieira <lucas.engen@outlook.com>
@lucasvmx
Copy link
Contributor

@chowyu08 this issue can be closed

1 similar comment
@lucasvmx
Copy link
Contributor

@chowyu08 this issue can be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants