Skip to content

Commit 777c55c

Browse files
authored
Fix protocol implementation when certain packets are bigger than 129 bytes (#30)
For some packets, the remaining length was encoded using `chr()`, but if the packet size exceeded 129 bytes (i.e. 127 bytes remaining length), it would become incorrectly encoded. This changes fixes the problem by always using the proper encoding method `encodeMessageLength()`. Also drops the buffer length counter as it is faster to just check for `strlen()`.
1 parent e88d452 commit 777c55c

File tree

1 file changed

+26
-49
lines changed

1 file changed

+26
-49
lines changed

src/MQTTClient.php

Lines changed: 26 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,6 @@ protected function performConnectionHandshake(
200200
): void
201201
{
202202
try {
203-
$i = 0;
204203
$buffer = '';
205204

206205
// protocol header
@@ -213,48 +212,33 @@ protected function performConnectionHandshake(
213212
$buffer .= chr(0x64); // protocol name: d
214213
$buffer .= chr(0x70); // protocol name: p
215214
$buffer .= chr(0x03); // protocol version (3.1)
216-
$i += 9;
217215

218216
// connection flags
219-
$flags = $this->buildConnectionFlags($username, $password, $sendCleanSessionFlag);
220-
$buffer .= chr($flags);
221-
$i++;
217+
$buffer .= chr($this->buildConnectionFlags($username, $password, $sendCleanSessionFlag));
222218

223219
// keep alive settings
224220
$buffer .= chr($this->settings->getKeepAlive() >> 8);
225221
$buffer .= chr($this->settings->getKeepAlive() & 0xff);
226-
$i += 2;
227222

228223
// client id (connection identifier)
229-
$clientIdPart = $this->buildLengthPrefixedString($this->clientId);
230-
$buffer .= $clientIdPart;
231-
$i += strlen($clientIdPart);
224+
$buffer .= $this->buildLengthPrefixedString($this->clientId);
232225

233226
// last will topic and message
234227
if ($this->settings->hasLastWill()) {
235-
$topicPart = $this->buildLengthPrefixedString($this->settings->getLastWillTopic());
236-
$buffer .= $topicPart;
237-
$i += strlen($topicPart);
238-
239-
$messagePart = $this->buildLengthPrefixedString($this->settings->getLastWillMessage());
240-
$buffer .= $messagePart;
241-
$i += strlen($messagePart);
228+
$buffer .= $this->buildLengthPrefixedString($this->settings->getLastWillTopic());
229+
$buffer .= $this->buildLengthPrefixedString($this->settings->getLastWillMessage());
242230
}
243231

244232
// credentials
245233
if ($username !== null) {
246-
$usernamePart = $this->buildLengthPrefixedString($username);
247-
$buffer .= $usernamePart;
248-
$i += strlen($usernamePart);
234+
$buffer .= $this->buildLengthPrefixedString($username);
249235
}
250236
if ($password !== null) {
251-
$passwordPart = $this->buildLengthPrefixedString($password);
252-
$buffer .= $passwordPart;
253-
$i += strlen($passwordPart);
237+
$buffer .= $this->buildLengthPrefixedString($password);
254238
}
255239

256240
// message type and message length
257-
$header = chr(0x10) . chr($i);
241+
$header = chr(0x10) . $this->encodeMessageLength(strlen($buffer));
258242

259243
// send the connection message
260244
$this->logger->info('Sending connection handshake to MQTT broker.');
@@ -458,33 +442,28 @@ protected function publishMessage(
458442
call_user_func($handler, $this, $topic, $message, $messageId, $qualityOfService, $retain);
459443
}
460444

461-
$i = 0;
462445
$buffer = '';
463446

464-
$topicPart = $this->buildLengthPrefixedString($topic);
465-
$buffer .= $topicPart;
466-
$i += strlen($topicPart);
447+
$buffer .= $this->buildLengthPrefixedString($topic);
467448

468449
if ($messageId !== null) {
469450
$buffer .= $this->encodeMessageId($messageId);
470-
$i += 2;
471451
}
472452

473453
$buffer .= $message;
474-
$i += strlen($message);
475454

476455
$command = 0x30;
477456
if ($retain) {
478-
$command += 1 << 0;
457+
$command |= 1 << 0;
479458
}
480459
if ($qualityOfService > 0) {
481-
$command += $qualityOfService << 1;
460+
$command |= $qualityOfService << 1;
482461
}
483462
if ($isDuplicate) {
484-
$command += 1 << 3;
463+
$command |= 1 << 3;
485464
}
486465

487-
$header = chr($command) . $this->encodeMessageLength($i);
466+
$header = chr($command) . $this->encodeMessageLength(strlen($buffer));
488467

489468
$this->writeToSocket($header . $buffer);
490469
}
@@ -524,21 +503,19 @@ public function subscribe(string $topic, callable $callback, int $qualityOfServi
524503
'qos' => $qualityOfService,
525504
]);
526505

527-
$i = 0;
528-
$buffer = '';
529506
$messageId = $this->nextMessageId();
530-
$buffer .= $this->encodeMessageId($messageId);
531-
$i += 2;
532507

533-
$topicPart = $this->buildLengthPrefixedString($topic);
534-
$buffer .= $topicPart;
535-
$i += strlen($topicPart);
536-
$buffer .= chr($qualityOfService);
537-
$i++;
508+
$buffer = '';
509+
510+
$buffer .= $this->encodeMessageId($messageId);
511+
512+
$buffer .= $this->buildLengthPrefixedString($topic);
513+
514+
$buffer .= chr($qualityOfService);
538515

539516
$this->repository->addNewTopicSubscription($topic, $callback, $messageId, $qualityOfService);
540517

541-
$header = chr(0x82) . chr($i);
518+
$header = chr(0x82) . $this->encodeMessageLength(strlen($buffer));
542519

543520
$this->writeToSocket($header . $buffer);
544521
}
@@ -576,15 +553,15 @@ protected function sendUnsubscribeRequest(int $messageId, string $topic, bool $i
576553
'is_duplicate' => $isDuplicate,
577554
]);
578555

579-
$buffer = $this->encodeMessageId($messageId);
580-
$i = 2;
556+
$buffer = '';
557+
558+
$buffer .= $this->encodeMessageId($messageId);
581559

582-
$topicPart = $this->buildLengthPrefixedString($topic);
583-
$buffer .= $topicPart;
584-
$i += strlen($topicPart);
560+
$buffer .= $this->buildLengthPrefixedString($topic);
585561

586562
$command = 0xa2 | ($isDuplicate ? 1 << 3 : 0);
587-
$header = chr($command) . chr($i);
563+
564+
$header = chr($command) . $this->encodeMessageLength(strlen($buffer));
588565

589566
$this->writeToSocket($header . $buffer);
590567
}

0 commit comments

Comments
 (0)