Kong / unirest-java

Unirest in Java: Simplified, lightweight HTTP client library.
http://kong.github.io/unirest-java/
MIT License
2.59k stars 593 forks source link

[Interceptor] Logging the RequestBody #376

Closed FireDrunk closed 3 years ago

FireDrunk commented 3 years ago

Describe the bug RequestBody.toString() doesn't provide any usable information

To Reproduce Steps to reproduce the behavior:

  1. Create an Interceptor
  2. Override the onRequest method
  3. Try to log the RequestBody

Expected behavior Something... usable.

Environmental Data:

Additional context The problem is that it's almost impossible to disect the object. It potentially has things like Multi/Uniparts, or some basic fields. That's exactly the type of body I want to log. Is there any example on how to log the fields set by the .field() function?

ryber commented 3 years ago

The problem is, a multi-part form potentially has files which may be quite large and are somewhat unknowable without going through the entire process twice (once for the logger and once for the real thing. At the very least you could never log exactly what was sent since the parts and their part ID's would be different. Similarly, other types of bodies will also support binary streams which couldn't be logged (meaningfully). I might be able to produce a simple summary of the body for simple field types, but it's not going to be exactly what is sent over the wire.

FireDrunk commented 3 years ago

Understandable. I think it might be nice to have a way to 'read' wether that's the case. I've tried doing this with the isMultipart() and the isUnipart() functions, but that doesn't tell me what information is readable afterwards. Upon looking inside the object I see something like an ApacheRequestBody with a Mode that is set to "BROWSER_COMPATIBLE".

This still doesn't give me any information on what's inside.

I think a function like printNestedFields() and some more isSomething() and hasSomething() functions might come in handy.


I've looked a bit deeper into the code, and found out that getting the actual string representation (if you ignore file uploads) is a bit hard. If i'm correct, the inside of the RequestBody consists of a ContentBody created by the ApacheBodymapper.mapToMultipart() function. This function generates the body using the FormBodyPartBuilder class which returns an implementation of a ContentBody.

In the process we lose all information on the contents, in the end it's just an InputStream. If we would like to intercept this, I think adding an interceptor to this Body build part is required, or the object in transfer should contain information about it's contents (which is not very useful if you're not debugging 😁 )

For now I've come up with this:

StringBuilder body = new StringBuilder();
if (request.getBody().isPresent()) {
    Body requestBody = request.getBody().get();

    log.debug("Mode: {}", requestBody.getMode());
    if (requestBody.getMode() == MultipartMode.BROWSER_COMPATIBLE) {
        for (BodyPart part : requestBody.multiParts()) {
            if (!part.isFile() && part.getPartType() == String.class) {
                body.append(String.format("\t%s: %s\n", part.getName(), part.getValue()));
            }
        };
    }
    else {
        body.append("Non Browser Compatible mode detected, could not log!");
    }
}

Ofcourse it's not flawless, but it gets the job done. The String.format is ofcourse a bit weird, because I'd rather log the actual generated body, but that's near impossible.

ryber commented 3 years ago

You shouldn't need to look at the mode at at, in fact, looking at any Apache part of unirest is discouraged as future versions may remove it as a dependency. The multiparts will simply return an empty collection if it's not a multipart request. This should do the same thing as above with fewer parts:

new Interceptor() {
            @Override
            public void onRequest(HttpRequest<?> request, Config config) {
                request.getBody().ifPresent(b -> {
                    b.multiParts().forEach(part -> {
                        if(part.getPartType().equals(String.class)){
                            System.out.println(part.getName() + "="+ part.getValue());
                        }
                    });
                });
            }
        });

I will take this to look into some methods to make logging and inspecting specific fields easier.

FireDrunk commented 3 years ago

@ryber Given example doesn't log anything in my case.

ryber commented 3 years ago

in 3.11.03 there are a few things that can help.

  1. All of the request parts have sane toString() representations. for example a param part will toString as key=value. Files toString as the file name.
  2. the request summary interface which is passed to the interceptors has a asString() method which will return an approximation of the request suitable for logging. Again, binary data is excluded, and multipart forms will have different part IDs but it will mostly be correct. see https://github.com/Kong/unirest-java/blob/8789e9208d73e9d3c471b90bcd699fd75115f768/unirest/src/test/java/BehaviorTests/BodyLogSummaryTest.java
  3. There is now a method to get a body part by name https://github.com/Kong/unirest-java/blob/main/unirest/src/test/java/kong/unirest/BodyTest.java#L33-L42
  4. I added a test showing collecting parts from the interceptor: https://github.com/Kong/unirest-java/blob/main/unirest/src/test/java/BehaviorTests/InterceptorTest.java#L162-L179