diff --git a/packets/packets.go b/packets/packets.go
index 36125d737c5d9e9a451ece864c340cc1c13fda3c..0e0671706a813c1a38fe93903e74d2680bb12907 100644
--- a/packets/packets.go
+++ b/packets/packets.go
@@ -412,6 +412,10 @@ }
}
if pk.Connect.UsernameFlag { // [MQTT-3.1.3-12]
+ if offset >= len(buf) { // we are at the end of the packet
+ return ErrProtocolViolationFlagNoUsername // [MQTT-3.1.2-17]
+ }
+
pk.Connect.Username, offset, err = decodeBytes(buf, offset)
if err != nil {
return ErrMalformedUsername
@@ -449,10 +453,6 @@ }
if len(pk.Connect.Username) > math.MaxUint16 {
return ErrProtocolViolationUsernameTooLong
- }
-
- if pk.Connect.UsernameFlag && len(pk.Connect.Username) == 0 {
- return ErrProtocolViolationFlagNoUsername // [MQTT-3.1.2-17]
}
if !pk.Connect.UsernameFlag && len(pk.Connect.Username) > 0 {
diff --git a/packets/tpackets.go b/packets/tpackets.go
index d8bc0cacd8648e4fa8db7b18fd8d4cbfaef16e33..934ce05e169a27adb94fe92a78a93b99b76cb2d9 100644
--- a/packets/tpackets.go
+++ b/packets/tpackets.go
@@ -71,8 +71,8 @@ TConnectInvalidPasswordTooLong
TConnectInvalidWillFlagNoPayload
TConnectInvalidWillFlagQosOutOfRange
TConnectInvalidWillSurplusRetain
+ TPublishInvalidTopicAlias
// TPacketCase contains data for cross-checking the encoding and decoding
-type TPacketCase struct {
TConnectSpecInvalidUTF8D800
TConnectSpecInvalidUTF8DFFF
TConnectSpecInvalidUTF80000
@@ -499,6 +499,43 @@ WillQos: 1,
},
},
},
+ {
+ Case: TConnectZeroByteUsername,
+ Desc: "username flag but 0 byte username",
+ Group: "decode",
+ RawBytes: []byte{
+ Connect << 4, 23, // Fixed header
+ 0, 4, // Protocol Name - MSB+LSB
+ 'M', 'Q', 'T', 'T', // Protocol Name
+ 5, // Protocol Version
+ 130, // Packet Flags
+ 0, 30, // Keepalive
+ 5, // length
+ 17, 0, 0, 0, 120, // Session Expiry Interval (17)
+ 0, 3, // Client ID - MSB+LSB
+ 'z', 'e', 'n', // Client ID "zen"
+ 0, 0, // Username MSB+LSB
+ },
+ Packet: &Packet{
+ FixedHeader: FixedHeader{
+ Type: Connect,
+ Remaining: 23,
+ },
+ ProtocolVersion: 5,
+ Connect: ConnectParams{
+ ProtocolName: []byte("MQTT"),
+ Clean: true,
+ Keepalive: 30,
+ ClientIdentifier: "zen",
+ Username: []byte{},
+ UsernameFlag: true,
+ },
+ Properties: Properties{
+ SessionExpiryInterval: uint32(120),
+ SessionExpiryIntervalFlag: true,
+ },
+ },
+ },
// Fail States
{
@@ -623,6 +660,24 @@ 0, 9, // Will Message MSB+LSB
'n', 'o', 't', ' ', 'a', 'g', 'a', 'i', 'n',
0, 5, // Username MSB+LSB
'm', 'o', 'c',
+ },
+ },
+
+ {
+ Case: TConnectInvalidFlagNoUsername,
+ Desc: "username flag with no username bytes",
+ Group: "decode",
+ FailFirst: ErrProtocolViolationFlagNoUsername,
+ RawBytes: []byte{
+ Connect << 4, 17, // Fixed header
+ 0, 4, // Protocol Name - MSB+LSB
+ 'M', 'Q', 'T', 'T', // Protocol Name
+ 5, // Protocol Version
+ 130, // Flags
+ 0, 20, // Keepalive
+ 0,
+ 0, 3, // Client ID - MSB+LSB
+ 'z', 'e', 'n', // Client ID "zen"
},
},
{
@@ -782,20 +837,6 @@ ProtocolName: []byte("MQTT"),
ClientIdentifier: func() string {
return string(make([]byte, 65536))
}(),
- },
- },
- },
- {
- Case: TConnectInvalidFlagNoUsername,
- Desc: "has username flag but no username",
- Group: "validate",
- Expect: ErrProtocolViolationFlagNoUsername,
- Packet: &Packet{
- FixedHeader: FixedHeader{Type: Connect},
- ProtocolVersion: 4,
- Connect: ConnectParams{
- ProtocolName: []byte("MQTT"),
- UsernameFlag: true,
},
},
},
diff --git a/server_test.go b/server_test.go
index d6fdaabfb26ac1ad93acd07cf4f5b2d7aff03362..31905ea17b3df6c9da62fd6908ff300d57d826d6 100644
--- a/server_test.go
+++ b/server_test.go
@@ -748,6 +748,33 @@
r.Close()
}
+// See https://github.com/mochi-co/mqtt/issues/178
+func TestServerEstablishConnectionZeroByteUsernameIsValid(t *testing.T) {
+ s := newServer()
+
+ r, w := net.Pipe()
+ o := make(chan error)
+ go func() {
+ o <- s.EstablishConnection("tcp", r)
+ }()
+
+ go func() {
+ w.Write(packets.TPacketData[packets.Connect].Get(packets.TConnectZeroByteUsername).RawBytes)
+ w.Write(packets.TPacketData[packets.Disconnect].Get(packets.TDisconnect).RawBytes)
+ }()
+
+ // receive the connack error
+ go func() {
+ _, err := io.ReadAll(w)
+ require.NoError(t, err)
+ }()
+
+ err := <-o
+ require.NoError(t, err)
+
+ r.Close()
+}
+
func TestServerEstablishConnectionInvalidConnectAckFailure(t *testing.T) {
s := newServer()
