feature-2774 webhook connector can now handle json array in request body #2803
Conversation
|
@mathias-vandaele Thanks for tackling this. I think we can solve is issue in potentially simpler way. The expected type of the |
mathias-vandaele
force-pushed
the
2774-webhook-connector-cannot-handle-json-array-in-request-body
branch
from
aa0f440
to
1970216
Compare
@sbuettner I made a way simpler solution :D |
| private static <T> Map<T, T> checkedCastToMap(Object o, Class<T> tClass) { | ||
| if (o instanceof Map<?, ?> map) { | ||
| for (Map.Entry<?, ?> entry : map.entrySet()) { | ||
| if (!(tClass.isInstance(entry.getValue())) || !(tClass.isInstance(entry.getKey()))) { | ||
| return Map.of(); | ||
| } | ||
| } | ||
| return (Map<T, T>) map; | ||
| } | ||
| return Map.of(); | ||
| } |
There was a problem hiding this comment.
As we cant calculate the signature of an empty map anyway. What about skipping it when the payload is not of type map?
There was a problem hiding this comment.
If we have a String[] couldn't we use the values to calculate the signature?
I don't know where it is used, so I might be wrong
There was a problem hiding this comment.
As we cant calculate the signature of an empty map anyway. What about skipping it when the payload is not of type map?
Yes let me refactor it to match this behavior
If we have a
String[]couldn't we use the values to calculate the signature? I don't know where it is used, so I might be wrong
@johnBgood That would be easy to do, but I don't have the big picture for this feature, can it be send as an array? but yes I though about it at first
There was a problem hiding this comment.
Not send as an array, but we could build the signature in a similar way of what we do with Maps, basically concatenating values
There was a problem hiding this comment.
So the given list would be in this format ["key1", "value1", "key2", "value2"] @johnBgood ?
There was a problem hiding this comment.
Yes exactly :) Conceptually it would be more ["value1", "value2"...] but that's the spirit yes.
Instead of adding keys/values from a Map (see below), we would simply add the array's values.
for (String key : sortedKeys) {
builder.append(key);
String value = signatureData.get(key);
builder.append(value == null ? "" : value);
}
| for (String key : sortedKeys) { | ||
| builder.append(key); | ||
| String value = signatureData.get(key); | ||
| builder.append(value == null ? "" : value); | ||
| } | ||
| return builder.toString(); | ||
| } | ||
|
|
||
| private static <T> Map<T, T> checkedCastToMap(Object o, Class<T> tClass) { |
There was a problem hiding this comment.
Looks like we don't have any other use cases than the tClass being a String. I'd suggest removing the generics then for visibility
| for (String key : sortedKeys) { | ||
| builder.append(key); | ||
| String value = signatureData.get(key); | ||
| builder.append(value == null ? "" : value); | ||
| } | ||
| return builder.toString(); | ||
| } | ||
|
|
||
| private static <T> Map<T, T> checkedCastToMap(Object o, Class<T> tClass) { | ||
| if (o instanceof Map<?, ?> map) { |
There was a problem hiding this comment.
if this is true do we really have to iterate over the entrySet?
| private static <T> Map<T, T> checkedCastToMap(Object o, Class<T> tClass) { | ||
| if (o instanceof Map<?, ?> map) { | ||
| for (Map.Entry<?, ?> entry : map.entrySet()) { | ||
| if (!(tClass.isInstance(entry.getValue())) || !(tClass.isInstance(entry.getKey()))) { | ||
| return Map.of(); | ||
| } | ||
| } | ||
| return (Map<T, T>) map; | ||
| } | ||
| return Map.of(); | ||
| } |
There was a problem hiding this comment.
If we have a String[] couldn't we use the values to calculate the signature?
I don't know where it is used, so I might be wrong
…case, signature cant be done
…case, signature cant be done. Correct after comments of johnBGood
…HMAC signature, array or object, tests have also been added
mathias-vandaele
force-pushed
the
2774-webhook-connector-cannot-handle-json-array-in-request-body
branch
from
68200b9
to
1370169
Compare
Description
The function which was handling the deserialization of the body from in incoming request could only create a map, if a array of primitive or object was passed, an exception was raised.
This function was called in many places, and something would be casted into a
Map<String, Object>Sometime into anObjectIn order to manage current and future feature. I created 3 wrapping classes that all hold the body in its different forms:
Map<String, Object>List<Map<String, Object>>List<Object>They all Implement the
RawWebHookBodythat holds all the methods that can interact with the bodyRelated issues
closes #2774