From 1797aadbe769c08f998f33eb0baf0632b866bb3e Mon Sep 17 00:00:00 2001
From: Gilbert <gilbertw@emqx.io>
Date: Sat, 22 Dec 2018 18:47:38 +0800
Subject: [PATCH] Fix unsuback compat (#2102)

---
 src/emqx_protocol.erl            | 31 +++++++++++++++----------------
 src/emqx_reason_codes.erl        |  4 +++-
 test/emqx_reason_codes_tests.erl |  4 ++++
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/emqx_protocol.erl b/src/emqx_protocol.erl
index 7c182c755..13eff95bf 100644
--- a/src/emqx_protocol.erl
+++ b/src/emqx_protocol.erl
@@ -283,8 +283,8 @@ preprocess_properties(Packet = #mqtt_packet{
                                                           topic_name = <<>>,
                                                           properties = #{'Topic-Alias' := AliasId}}
                                  },
-                      PState = #pstate{proto_ver = ?MQTT_PROTO_V5, 
-                                       topic_aliases = Aliases, 
+                      PState = #pstate{proto_ver = ?MQTT_PROTO_V5,
+                                       topic_aliases = Aliases,
                                        topic_alias_maximum = #{from_client := TopicAliasMaximum}}) ->
     case AliasId =< TopicAliasMaximum of
         true ->
@@ -300,7 +300,7 @@ preprocess_properties(Packet = #mqtt_packet{
                                                   topic_name = Topic,
                                                   properties = #{'Topic-Alias' := AliasId}}
                                },
-                      PState = #pstate{proto_ver = ?MQTT_PROTO_V5, 
+                      PState = #pstate{proto_ver = ?MQTT_PROTO_V5,
                                        topic_aliases = Aliases,
                                        topic_alias_maximum = #{from_client := TopicAliasMaximum}}) ->
     case AliasId =< TopicAliasMaximum of
@@ -521,11 +521,7 @@ connack({?RC_SUCCESS, SP, PState}) ->
 
 connack({ReasonCode, PState = #pstate{proto_ver = ProtoVer}}) ->
     emqx_hooks:run('client.connected', [credentials(PState), ReasonCode, attrs(PState)]),
-    ReasonCode1 = if ProtoVer =:= ?MQTT_PROTO_V5 ->
-                         ReasonCode;
-                     true ->
-                         emqx_reason_codes:compat(connack, ReasonCode)
-                  end,
+    [ReasonCode1] = reason_codes_compat(connack, [ReasonCode], ProtoVer),
     _ = deliver({connack, ReasonCode1}, PState),
     {error, emqx_reason_codes:name(ReasonCode1, ProtoVer), PState}.
 
@@ -633,15 +629,10 @@ deliver({pubrec, PacketId, ReasonCode}, PState) ->
     send(?PUBREC_PACKET(PacketId, ReasonCode), PState);
 
 deliver({suback, PacketId, ReasonCodes}, PState = #pstate{proto_ver = ProtoVer}) ->
-    send(?SUBACK_PACKET(PacketId,
-                        if ProtoVer =:= ?MQTT_PROTO_V5 ->
-                               ReasonCodes;
-                           true ->
-                               [emqx_reason_codes:compat(suback, RC) || RC <- ReasonCodes]
-                        end), PState);
+    send(?SUBACK_PACKET(PacketId, reason_codes_compat(suback, ReasonCodes, ProtoVer)), PState);
 
-deliver({unsuback, PacketId, ReasonCodes}, PState) ->
-    send(?UNSUBACK_PACKET(PacketId, ReasonCodes), PState);
+deliver({unsuback, PacketId, ReasonCodes}, PState = #pstate{proto_ver = ProtoVer}) ->
+    send(?UNSUBACK_PACKET(PacketId, reason_codes_compat(unsuback, ReasonCodes, ProtoVer)), PState);
 
 %% Deliver a disconnect for mqtt 5.0
 deliver({disconnect, ReasonCode}, PState = #pstate{proto_ver = ?MQTT_PROTO_V5}) ->
@@ -981,3 +972,11 @@ do_acl_deny_action(?SUBSCRIBE_PACKET(_PacketId, _Properties, _RawTopicFilters),
     end;
 do_acl_deny_action(_PubSupPacket, _ReasonCode, PState) ->
     {ok, PState}.
+
+%% Reason code compat
+reason_codes_compat(_PktType, ReasonCodes, ?MQTT_PROTO_V5) ->
+    ReasonCodes;
+reason_codes_compat(unsuback, _ReasonCodes, _ProtoVer) ->
+    undefined;
+reason_codes_compat(PktType, ReasonCodes, _ProtoVer) ->
+    [emqx_reason_codes:compat(PktType, RC) || RC <- ReasonCodes].
diff --git a/src/emqx_reason_codes.erl b/src/emqx_reason_codes.erl
index c6bdd849d..5b7f7e42f 100644
--- a/src/emqx_reason_codes.erl
+++ b/src/emqx_reason_codes.erl
@@ -140,4 +140,6 @@ compat(connack, 16#9D) -> ?CONNACK_SERVER;
 compat(connack, 16#9F) -> ?CONNACK_SERVER;
 
 compat(suback, Code) when Code =< ?QOS_2 -> Code;
-compat(suback, Code) when Code >= 16#80  -> 16#80.
+compat(suback, Code) when Code >= 16#80  -> 16#80;
+
+compat(unsuback, _Code) -> undefined.
diff --git a/test/emqx_reason_codes_tests.erl b/test/emqx_reason_codes_tests.erl
index 29f7ad081..a8e352c83 100644
--- a/test/emqx_reason_codes_tests.erl
+++ b/test/emqx_reason_codes_tests.erl
@@ -112,6 +112,10 @@ compat_test() ->
     (((codes_test(suback))
         ([0,1,2, 16#80]))
        ([0,1,2, 16#80]))
+      (fun emqx_reason_codes:compat/2),
+    (((codes_test(unsuback))
+        ([0, 1, 2]))
+       ([undefined, undefined, undefined]))
       (fun emqx_reason_codes:compat/2).
 
 codes_test(AsistVar) ->