You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by "sstremler (via GitHub)" <gi...@apache.org> on 2025/01/31 22:19:49 UTC
[PR] [CXF-9105] add synchronized to the principal iteration [cxf]
sstremler opened a new pull request, #2245:
URL: https://github.com/apache/cxf/pull/2245
https://issues.apache.org/jira/browse/CXF-9105
--
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@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CXF-9105] add synchronized to the principal iteration [cxf]
Posted by "sstremler (via GitHub)" <gi...@apache.org>.
sstremler commented on PR #2245:
URL: https://github.com/apache/cxf/pull/2245#issuecomment-2628556571
> Thank you @sstremler , a few comments but LGTM otherwise!
Thank you, I added the changes you suggested.
--
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@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CXF-9105] add synchronized to the principal iteration [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #2245:
URL: https://github.com/apache/cxf/pull/2245#discussion_r1938068328
##########
rt/features/logging/src/main/java/org/apache/cxf/ext/logging/event/DefaultLogEventMapper.java:
##########
@@ -125,31 +126,36 @@ private String getPrincipal(Message message) {
return null;
}
- private String getJAASPrincipal() {
- StringBuilder principals = new StringBuilder();
- Iterator<? extends Object> principalIt = getJAASPrincipals();
- while (principalIt.hasNext()) {
- principals.append(principalIt.next());
- if (principalIt.hasNext()) {
- principals.append(',');
+ public String getConcatenatedJAASPrincipals() {
Review Comment:
```suggestion
private String getConcatenatedJAASPrincipals() {
```
--
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@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CXF-9105] add synchronized to the principal iteration [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta merged PR #2245:
URL: https://github.com/apache/cxf/pull/2245
--
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@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CXF-9105] add synchronized to the principal iteration [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on code in PR #2245:
URL: https://github.com/apache/cxf/pull/2245#discussion_r1938070843
##########
rt/features/logging/src/main/java/org/apache/cxf/ext/logging/event/DefaultLogEventMapper.java:
##########
@@ -125,31 +126,36 @@ private String getPrincipal(Message message) {
return null;
}
- private String getJAASPrincipal() {
- StringBuilder principals = new StringBuilder();
- Iterator<? extends Object> principalIt = getJAASPrincipals();
- while (principalIt.hasNext()) {
- principals.append(principalIt.next());
- if (principalIt.hasNext()) {
- principals.append(',');
+ public String getConcatenatedJAASPrincipals() {
+ StringBuilder principalsStringBuilder = new StringBuilder();
+ Set<Principal> principals = getJAASPrincipals();
Review Comment:
I believe we could do:
```
if (principals.isEmpty()){
return null;
}
```
since `getJAASPrincipals()` may return the empty set which does not need synchronization (moreover, it is backed by static instance).
--
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@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CXF-9105] add synchronized to the principal iteration [cxf]
Posted by "reta (via GitHub)" <gi...@apache.org>.
reta commented on PR #2245:
URL: https://github.com/apache/cxf/pull/2245#issuecomment-2628539485
Thank you @sstremler , a few comments but LGTM otherwise!
--
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@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org