[GitHub] [cxf] ashakirin opened a new pull request #713: CXF-8359: fixed mask for XML elements containing attributes

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] ashakirin opened a new pull request #713: CXF-8359: fixed mask for XML elements containing attributes

GitBox

ashakirin opened a new pull request #713:
URL: https://github.com/apache/cxf/pull/713


    Fixed mask for XML elements containing attributes


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] reta commented on a change in pull request #713: CXF-8359: fixed mask for XML elements containing attributes

GitBox

reta commented on a change in pull request #713:
URL: https://github.com/apache/cxf/pull/713#discussion_r511619713



##########
File path: rt/features/logging/src/main/java/org/apache/cxf/ext/logging/MaskSensitiveHelper.java
##########
@@ -70,15 +71,19 @@ public String maskSensitiveElements(
             final Message message,
             final String originalLogString) {
         if (replacementsXML.isEmpty() && replacementsJSON.isEmpty()
-                || message == null

Review comment:
       Why this condition was removed? It is exactly what will be returned on line #80 but with unnecessary use of `Optional::ofNullable(message)`?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] reta commented on a change in pull request #713: CXF-8359: fixed mask for XML elements containing attributes

GitBox
In reply to this post by GitBox

reta commented on a change in pull request #713:
URL: https://github.com/apache/cxf/pull/713#discussion_r511619713



##########
File path: rt/features/logging/src/main/java/org/apache/cxf/ext/logging/MaskSensitiveHelper.java
##########
@@ -70,15 +71,19 @@ public String maskSensitiveElements(
             final Message message,
             final String originalLogString) {
         if (replacementsXML.isEmpty() && replacementsJSON.isEmpty()
-                || message == null

Review comment:
       Why this condition was removed? It is exactly what will be returned on line https://github.com/apache/cxf/pull/713/files#diff-042c9128e111e120c63cbb7404cdc5d2d2e3c4377b40e01c097d4b8e7315191dR80 but with unnecessary use of `Optional::ofNullable(message)`?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] asfgit merged pull request #713: CXF-8359: fixed mask for XML elements containing attributes

GitBox
In reply to this post by GitBox

asfgit merged pull request #713:
URL: https://github.com/apache/cxf/pull/713


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [cxf] ashakirin commented on a change in pull request #713: CXF-8359: fixed mask for XML elements containing attributes

GitBox
In reply to this post by GitBox

ashakirin commented on a change in pull request #713:
URL: https://github.com/apache/cxf/pull/713#discussion_r516340812



##########
File path: rt/features/logging/src/main/java/org/apache/cxf/ext/logging/MaskSensitiveHelper.java
##########
@@ -70,15 +71,19 @@ public String maskSensitiveElements(
             final Message message,
             final String originalLogString) {
         if (replacementsXML.isEmpty() && replacementsJSON.isEmpty()
-                || message == null

Review comment:
       Matter of taste, for me Optional alternative is a bit compacter




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]