You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/03/17 12:21:06 UTC

[GitHub] [rocketmq] chaiyx opened a new pull request #3997: [ISSUE #3996] fix the acl content of TopicRequestHeader

chaiyx opened a new pull request #3997:
URL: https://github.com/apache/rocketmq/pull/3997


   #3996


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] caigy commented on a change in pull request #3997: [ISSUE #3996] fix the acl content of TopicRequestHeader

Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #3997:
URL: https://github.com/apache/rocketmq/pull/3997#discussion_r839168023



##########
File path: acl/src/main/java/org/apache/rocketmq/acl/common/AclClientRPCHook.java
##########
@@ -69,7 +72,11 @@ public void doAfterResponse(String remoteAddr, RemotingCommand request, Remoting
             if (null != header) {
                 Field[] fields = fieldCache.get(header.getClass());
                 if (null == fields) {
-                    fields = header.getClass().getDeclaredFields();
+                    Set<Field> fieldList = new HashSet<Field>();

Review comment:
       But the order of fields put into returning SortedMap still depends on the order of `fieldList`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls edited a comment on pull request #3997: [ISSUE #3996] fix the acl content of TopicRequestHeader

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3997:
URL: https://github.com/apache/rocketmq/pull/3997#issuecomment-1070899767


   
   [![Coverage Status](https://coveralls.io/builds/47457609/badge)](https://coveralls.io/builds/47457609)
   
   Coverage increased (+0.005%) to 49.495% when pulling **a98f6cd7c7a0c53dc8f4df10d7b3aee399def4cc on chaiyx:static-topic-dev** into **a17fa7605cfbd266e4468e6fe2d6cf6711572111 on apache:5.0.0-alpha**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls commented on pull request #3997: [ISSUE #3996] fix the acl content of TopicRequestHeader

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3997:
URL: https://github.com/apache/rocketmq/pull/3997#issuecomment-1070899767


   
   [![Coverage Status](https://coveralls.io/builds/47457609/badge)](https://coveralls.io/builds/47457609)
   
   Coverage increased (+0.005%) to 49.495% when pulling **a98f6cd7c7a0c53dc8f4df10d7b3aee399def4cc on chaiyx:static-topic-dev** into **a17fa7605cfbd266e4468e6fe2d6cf6711572111 on apache:5.0.0-alpha**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] chaiyx commented on a change in pull request #3997: [ISSUE #3996] fix the acl content of TopicRequestHeader

Posted by GitBox <gi...@apache.org>.
chaiyx commented on a change in pull request #3997:
URL: https://github.com/apache/rocketmq/pull/3997#discussion_r831801102



##########
File path: acl/src/main/java/org/apache/rocketmq/acl/common/AclClientRPCHook.java
##########
@@ -69,7 +72,11 @@ public void doAfterResponse(String remoteAddr, RemotingCommand request, Remoting
             if (null != header) {
                 Field[] fields = fieldCache.get(header.getClass());
                 if (null == fields) {
-                    fields = header.getClass().getDeclaredFields();
+                    Set<Field> fieldList = new HashSet<Field>();

Review comment:
       at the end of the function, the value of the header will be put into a SortedMap. so it doesn't matter to use HashSet here.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] caigy commented on a change in pull request #3997: [ISSUE #3996] fix the acl content of TopicRequestHeader

Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #3997:
URL: https://github.com/apache/rocketmq/pull/3997#discussion_r831699606



##########
File path: acl/src/main/java/org/apache/rocketmq/acl/common/AclClientRPCHook.java
##########
@@ -69,7 +72,11 @@ public void doAfterResponse(String remoteAddr, RemotingCommand request, Remoting
             if (null != header) {
                 Field[] fields = fieldCache.get(header.getClass());
                 if (null == fields) {
-                    fields = header.getClass().getDeclaredFields();
+                    Set<Field> fieldList = new HashSet<Field>();

Review comment:
       `parseRequestContent` is used for signature checks, so the order of fields matters. I think a sorted set should be adopted here.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls commented on pull request #3997: [ISSUE #3996] fix the acl content of TopicRequestHeader

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3997:
URL: https://github.com/apache/rocketmq/pull/3997#issuecomment-1070899767


   
   [![Coverage Status](https://coveralls.io/builds/47457609/badge)](https://coveralls.io/builds/47457609)
   
   Coverage increased (+0.005%) to 49.495% when pulling **a98f6cd7c7a0c53dc8f4df10d7b3aee399def4cc on chaiyx:static-topic-dev** into **a17fa7605cfbd266e4468e6fe2d6cf6711572111 on apache:5.0.0-alpha**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls edited a comment on pull request #3997: [ISSUE #3996] fix the acl content of TopicRequestHeader

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3997:
URL: https://github.com/apache/rocketmq/pull/3997#issuecomment-1070899767


   
   [![Coverage Status](https://coveralls.io/builds/47457609/badge)](https://coveralls.io/builds/47457609)
   
   Coverage increased (+0.005%) to 49.495% when pulling **a98f6cd7c7a0c53dc8f4df10d7b3aee399def4cc on chaiyx:static-topic-dev** into **a17fa7605cfbd266e4468e6fe2d6cf6711572111 on apache:5.0.0-alpha**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org