From 7e00b58db52293361c7d47f6c1f4a0bb5cba53e6 Mon Sep 17 00:00:00 2001
From: JianBo He <HeeeJianBo@gmail.com>
Date: Tue, 17 Mar 2020 17:56:34 +0800
Subject: [PATCH] Remove the CONNACK's `Maximum-QoS` key if it equal to 2
 (#3321)

---
 src/emqx_channel.erl            | 24 ++++++++++++++++--------
 test/emqx_channel_SUITE.erl     |  3 +--
 test/mqtt_protocol_v5_SUITE.erl |  2 +-
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/emqx_channel.erl b/src/emqx_channel.erl
index a60f148f1..03544b9da 100644
--- a/src/emqx_channel.erl
+++ b/src/emqx_channel.erl
@@ -1182,14 +1182,22 @@ enrich_connack_caps(AckProps, #channel{conninfo   = #{proto_ver := ?MQTT_PROTO_V
       shared_subscription   := Shared,
       wildcard_subscription := Wildcard
      } = emqx_mqtt_caps:get_caps(Zone),
-    AckProps#{'Maximum-QoS'         => MaxQoS,
-              'Retain-Available'    => flag(Retain),
-              'Maximum-Packet-Size' => MaxPktSize,
-              'Topic-Alias-Maximum' => MaxAlias,
-              'Wildcard-Subscription-Available'   => flag(Wildcard),
-              'Subscription-Identifier-Available' => 1,
-              'Shared-Subscription-Available'     => flag(Shared)
-             };
+    NAckProps = AckProps#{'Retain-Available'    => flag(Retain),
+                          'Maximum-Packet-Size' => MaxPktSize,
+                          'Topic-Alias-Maximum' => MaxAlias,
+                          'Wildcard-Subscription-Available'   => flag(Wildcard),
+                          'Subscription-Identifier-Available' => 1,
+                          'Shared-Subscription-Available'     => flag(Shared)
+                         },
+    %% MQTT 5.0 - 3.2.2.3.4:
+    %% It is a Protocol Error to include Maximum QoS more than once,
+    %% or to have a value other than 0 or 1. If the Maximum QoS is absent,
+    %% the Client uses a Maximum QoS of 2.
+    case MaxQoS =:= 2 of
+        true -> NAckProps;
+        _ -> NAckProps#{'Maximum-QoS' => MaxQoS}
+    end;
+
 enrich_connack_caps(AckProps, _Channel) -> AckProps.
 
 %%--------------------------------------------------------------------
diff --git a/test/emqx_channel_SUITE.erl b/test/emqx_channel_SUITE.erl
index e3859a15d..b52d9ed07 100644
--- a/test/emqx_channel_SUITE.erl
+++ b/test/emqx_channel_SUITE.erl
@@ -522,8 +522,7 @@ t_enrich_connack_caps(_) ->
                    'Topic-Alias-Maximum' := 10,
                    'Wildcard-Subscription-Available' := 1,
                    'Subscription-Identifier-Available' := 1,
-                   'Shared-Subscription-Available' := 1,
-                   'Maximum-QoS' := ?QOS_2
+                   'Shared-Subscription-Available' := 1
                   }, AckProps),
     ok = meck:unload(emqx_mqtt_caps).
 
diff --git a/test/mqtt_protocol_v5_SUITE.erl b/test/mqtt_protocol_v5_SUITE.erl
index ec4499bf5..3c758f993 100644
--- a/test/mqtt_protocol_v5_SUITE.erl
+++ b/test/mqtt_protocol_v5_SUITE.erl
@@ -475,7 +475,7 @@ t_connack_max_qos_allowed(_) ->
 
     {ok, Client5} = emqtt:start_link([{proto_ver, v5}]),
     {ok, Connack5} = emqtt:connect(Client5),
-    ?assertEqual(2, maps:get('Maximum-QoS', Connack5)),  %% [MQTT-3.2.2-9]
+    ?assertEqual(undefined, maps:get('Maximum-QoS', Connack5, undefined)),  %% [MQTT-3.2.2-9]
     ok = emqtt:disconnect(Client5),
     waiting_client_process_exit(Client5),