Incorrect logging protobuf requests/responces

[juncevich]

Useless converting raw bytes from request(response) bodies broke protobuf model deserialization.

Description

Protobuf response body incorrect deserializing, because FilteredHttpRequest(FilteredHttpResponse).getBody() converts body to string and after that convert string to byte array. Which cause to brake model from body. For example, if i have raw body bytes array size 170, response.getBody() array of bytes size will 250, because it was converted to string.

Expected Behavior

FilteredHttpRequest.getBody() gives raw bytes, without converting to string.

Actual Behavior

Possible Fix

getBody must contains raw bytes from response

[juncevich]

Can fix it ASAP.

[skjolber]

@juncevich out of interest, which web server are you running?

[juncevich]

@skjolber Netty with Micronaut.

[skjolber]

I wonder if gRPC logging might be better served with a gRPC-centric module. At least thats how we're doing it internally (emulating the Logbook output format).

Also the HTTP 2 does not follow a strict request-response any more, with 1-n, n-1 or n-n combinations, not sure whether logbook will work as-is now @whiskeysierra?

[juncevich]

I've offered not to convert body to string in method byte[] getBody(). I suppose when when someone asked body in bytes - bytes must be raw. There is no need to convert body to string and after it convert to bytes. If i want body as string a can use String getBody(). I mean that problem not in gRPC, and fix in byte[] getBody() will fix the problem.

[whiskeysierra]

Actual Behavior

Is that the default implementation right now?

[juncevich]

Yep

logbook/logbook-core/src/main/java/org/zalando/logbook/FilteredHttpRequest.java

Line 78 in c54d0cb

public byte[] getBody() throws IOException {

[whiskeysierra]

Mmh, I see a bigger problem now.
The idea of filtering (the content or body) is done using a BodyFilter:

logbook/logbook-api/src/main/java/org/zalando/logbook/BodyFilter.java

Lines 11 to 13 in 39cc999

	public interface BodyFilter {
	String filter(@Nullable final String contentType, final String body);

As you can see, the interface that we defined here takes a String and returns a String.
This would need to change from String to byte[].

[juncevich]

@whiskeysierra Will we fix these problems? I can do it)

[whiskeysierra]

Will we fix these problems? I can do it)

I'd love to, but it sounds like a breaking change, tbh.

[whiskeysierra]

Useless converting raw bytes from request(response) bodies broke protobuf model deserialization.

@juncevich Just curious, do you just use protobuf over HTTP? I'm thinking how to integrate Logbook with gRPC and I'm looking for ways to do that cleanly.

[skjolber]

FYI we are using grpc-java and grpc-netty with subclasses of io.grpc.ServerInterceptor.