-
Notifications
You must be signed in to change notification settings - Fork 450
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
fix(cli):fix topic-alias-maximum error in cli connect #1444
Conversation
let count = 0 | ||
sender._write = (line, _enc, cb) => { | ||
const { topic, opts, protobufPath, protobufMessageName, format } = pubOpts | ||
count++ | ||
let omitTopic = opts.properties?.topicAlias && count >= 2 | ||
const publishMessage = processPublishMessage(line.trim(), protobufPath, protobufMessageName, format) | ||
client.publish(topic, publishMessage, opts, cb) | ||
client.publish(omitTopic ? '' : topic, publishMessage, opts, cb) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for? If a topic alias is set after a line break, do we no longer need to enter or use the original publish topic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only need to send the topic and topic alias together for the first time. The second time, we can directly use the topic alias to send, and the topic is set to "". The current multi-line send will carry the topic and topic alias each time, which does not comply with the mqtt protocol specification.
PR Checklist
If you have any questions, you can refer to the Contributing Guide
What is the current behavior?
It's normal to use --topic-alias-maximum when pub.
Issue Number
#1398
What is the new behavior?
Does this PR introduce a breaking change?
Specific Instructions
Are there any specific instructions or things that should be known prior to review?
Other information