ffang commented on code in PR #3620:
URL: https://github.com/apache/cxf/pull/3620#discussion_r4390011656
##########
rt/transports/http/src/main/java/org/apache/cxf/transport/http/Headers.java:
##########
@@ -516,30 +521,90 @@ && isResponseBodyAvailable()) {
boolean addHeaders = MessageUtils.getContextualBoolean(message,
ADD_HEADERS_PROPERTY, false);
for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
String header = entry.getKey();
- List<?> headerList = entry.getValue();
-
+ List<String> values = responseHeaderValues(header,
entry.getValue(), addHeaders);
if (addHeaders || isSingleHeader(header)) {
- for (int i = 0; i < headerList.size(); i++) {
- Object headerObject = headerList.get(i);
- if (headerObject != null) {
- response.addHeader(header,
headerObjectToString(headerObject));
- }
+ for (String value : values) {
+ response.addHeader(header, value);
}
} else {
- StringBuilder sb = new StringBuilder();
- for (int i = 0; i < headerList.size(); i++) {
- Object headerObject = headerList.get(i);
- if (headerObject != null) {
- sb.append(headerObjectToString(headerObject));
- }
-
- if (i + 1 < headerList.size()) {
- sb.append(',');
- }
+ response.setHeader(header, values.get(0));
+ }
+ }
+ }
+
+ static Map<String, List<String>> getResponseHeaders(Message message,
HttpServletResponse response) {
+ Map<String, List<String>> protocol = new
TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ Map<String, List<String>> original = CastUtils.cast((Map<?,
?>)message.get(Message.PROTOCOL_HEADERS));
+ if (original != null) {
+ protocol.putAll(original);
+ }
+ return new Headers(message, protocol).getResponseHeaders(response);
Review Comment:
> I believe the default constructor will extract `PROTOCOL_HEADERS`:
I'd prefer to keep this one off the `Headers(Message)` constructor. It calls
`getSetProtocolHeaders(message)`, which writes to the message: it stores an
empty `PROTOCOL_HEADERS` map when there isn't one, and replaces a `HashMap`
with a `TreeMap` copy. The view is read whenever logging runs, so reading it
shouldn't change the message. Code still holding the original `HashMap` could
also lose headers it adds afterwards. That's why it copies the headers instead.
I added a Javadoc line explaining this.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
