allthingssecurity commented on code in PR #27690:
URL: https://github.com/apache/camel/pull/27690#discussion_r4245779999
##########
components/camel-knative/camel-knative-http/src/main/java/org/apache/camel/component/knative/http/KnativeHttpProducer.java:
##########
@@ -102,9 +105,26 @@ public boolean process(Exchange exchange, AsyncCallback
callback) {
headers.add(HttpHeaders.CONTENT_TYPE, contentType);
}
+ final TypeConverter tc = exchange.getContext().getTypeConverter();
for (Map.Entry<String, Object> entry :
exchange.getMessage().getHeaders().entrySet()) {
- if
(!headerFilterStrategy.applyFilterToCamelHeaders(entry.getKey(),
entry.getValue(), exchange)) {
- headers.add(entry.getKey(), entry.getValue().toString());
+ final String key = entry.getKey();
+ final Object value = entry.getValue();
+ if (value instanceof Iterable || value instanceof Object[]) {
Review Comment:
Confirmed and fixed in c7ec6a285855, the same way as in #27689: only a
`java.util.Collection` or an `Object[]` is
split (`HttpUtil.applyHeaderValue`, called from
KnativeHttpProducer.java:112). A `JsonObject` and a `JsonArray`
are one header with their JSON again, as on main; a `JsonArray` is
deliberately not split (a List is).
New tests `producerSendsAJsonObjectHeaderAsItsJson`,
`producerSendsAJsonArrayHeaderAsItsJson` and
`producerSendsAMapHeaderAsOneValue`. With the previous head (632bb7932918)
the first two fail with
`["a=b", "c=d"]` and `["a", "b"]`; all three pass with the producer of main.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-knative/camel-knative-http/src/main/java/org/apache/camel/component/knative/http/KnativeHttpConsumer.java:
##########
@@ -331,14 +333,22 @@ private HttpServerResponse
toHttpResponse(HttpServerRequest request, Message mes
for (Map.Entry<String, Object> entry :
message.getHeaders().entrySet()) {
final String key = entry.getKey();
final Object value = entry.getValue();
-
- for (Object it :
org.apache.camel.support.ObjectHelper.createIterable(value, null)) {
- String headerValue = tc.convertTo(String.class, it);
- if (headerValue == null) {
- continue;
- }
- if (!headerFilterStrategy.applyFilterToCamelHeaders(key,
headerValue, message.getExchange())) {
- response.putHeader(key, headerValue);
+ if (value instanceof Iterable || value instanceof Object[]) {
+ // a collection or an object array has one value per
element (must not use a delimiter, and allow
+ // empty values); any other value, also a byte[] or
another primitive array, is a single value
+ final Iterator<?> it =
org.apache.camel.support.ObjectHelper.createIterator(value, null, true);
+ HttpUtil.applyHeader(headerFilterStrategy,
message.getExchange(), it, tc, key, (multiValues, prev) -> {
+ if (multiValues != null) {
+ // putHeader replaces the header, so all the
values must be given at once
+ response.putHeader(key, multiValues);
+ } else if (prev != null) {
+ response.putHeader(key, prev);
+ }
+ });
+ } else if
(!headerFilterStrategy.applyFilterToCamelHeaders(key, value,
message.getExchange())) {
Review Comment:
Both points are right; I checked them against main with new tests.
Blank String: on main `createIterable(value, null)` gives no element for a
blank String, so the reply sent no
header, and the previous head sent an empty header. c7ec6a285855 keeps the
old behaviour (KnativeHttpConsumer.java:335,
a blank String is skipped). `consumerRepliesWithoutAHeaderForABlankValue`
passes on main, fails with the previous
head and passes now. No change for users, so nothing to document.
Map: on main the reply iterated the entries and `putHeader` replaced the
header each time, so only the last entry
was sent, as `c=d` for `{a=b, c=d}`. The same happened to a `JsonObject`
(`c=d`) and a `JsonArray` (last
element, `b`). That last-entry result is not something to keep, so the reply
now sends these values as the
producer does and always did: one header with the `toString()` (`{a=b,
c=d}`, JSON for the Vert.x types). Tests
`consumerRepliesWithAMapHeaderAsOneValue`, `...AJsonObjectHeaderAsItsJson`,
`...AJsonArrayHeaderAsItsJson` fail on
main with `["c=d"]`, `["c=d"]`, `["b"]`. This remaining change is in the
upgrade guide entry. If you prefer one
`key=value` header per entry for a Map in the reply, say so and I will
change it; that would differ from the
producer.
_Claude Code on behalf of allthingssecurity_
--
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]
