belgif / rest-problem-java

Java library for RFC 9457 Problems with support for standard problem types of the Belgif REST guide (https://www.belgif.be/specification/rest/api-guide/#error-handling)
https://belgif.github.io/rest-problem-java/
Apache License 2.0
3 stars 0 forks source link

Bug: java exceptions shown in detailfield #95

Closed smals-mavh closed 1 month ago

smals-mavh commented 1 month ago

In some cases it seems that there are java exceptions or classes shown when using spring boot: Example: This integration test returns:

{
    "type": "urn:problem-type:belgif:badRequest",
    "href": "https://www.belgif.be/specification/rest/api-guide/problems/badRequest.html",
    "title": "Bad Request",
    "status": 400,
    "detail": "The input message is incorrect",
    "issues": [
        {
            "type": "urn:problem-type:belgif:input-validation:schemaViolation",
            "title": "Input value is invalid with respect to the schema",
            "detail": "Required request body is missing: public org.springframework.http.ResponseEntity<java.lang.String> io.github.belgif.rest.problem.FrontendController.beanValidationBody(io.github.belgif.rest.problem.model.Model)",
            "in": "body"
        }
    ]
}

When entering a wrong date, it returns:

{
    "type": "urn:problem-type:belgif:badRequest",
    "href": "https://www.belgif.be/specification/rest/api-guide/problems/badRequest.html",
    "title": "Bad Request",
    "status": 400,
    "detail": "The input message is incorrect",
    "issues": [
        {
            "type": "urn:problem-type:belgif:input-validation:schemaViolation",
            "title": "Input value is invalid with respect to the schema",
            "detail": "JSON parse error: Cannot deserialize value of type `java.time.LocalDate` from String \"12/07/1991\": Failed to deserialize java.time.LocalDate: (java.time.format.DateTimeParseException) Text '12/07/1991' could not be parsed at index 0",
            "in": "body"
        }
    ]
}

Both these requests are not compliant with the openapi, so, they could also be catched by an apigateway or the atlassian schema validator for example.

pvdbosch commented 1 month ago

IMO by default, responses should be sanitized ( [err-sanit] ) and no implementation internals should be returned, even at the cost of hiding some useful information. A completely detailed Problem message can be difficult to provide if the error from the underlying framework doesn't allow it.

If the API would need to report more detailed error information, it could use a schema validation library (see #97). For organization-internal APIs, where sanitization is less risky, maybe a configuration flag could be provided.

jpraet commented 1 month ago

Both examples are triggered by a HttpMessageNotReadableException thrown by Spring Framework, and converted to a problem here:

https://github.com/belgif/rest-problem-java/blob/e8f435d0ff4b11b291a12bf57507f3c85cd7b170/belgif-rest-problem-spring/src/main/java/io/github/belgif/rest/problem/spring/RoutingExceptionsHandler.java#L44-L51

So I guess we could just omit the "detail" in these problems (which we currently populate with the exception message)? We would probably need to log the exception then, otherwise this information would get lost.

Note that these exceptions are already thrown by Spring Framework before entering your controller. So if you want to plugin a schema validation library, you will need to make sure you can intercept the request earlier in the call chain.

pvdbosch commented 1 month ago

Both examples are triggered by a HttpMessageNotReadableException thrown by Spring Framework, and converted to a problem here:

So I guess we could just omit the "detail" in these problems (which we currently populate with the exception message)? We would probably need to log the exception then, otherwise this information would get lost.

We could try to include some limited level of detail based on the error cause, like attempted here: https://github.com/zalando/problem-spring-web/issues/1

jpraet commented 1 month ago

It looks to me like the zalando problem-spring-web library also exposes the exception message in the problem detail:

https://github.com/zalando/problem-spring-web/blob/da31acb922e486cebeff37d9fbd96322d70e4fc8/problem-spring-common/src/main/java/org/zalando/problem/spring/common/AdviceTrait.java#L81

pvdbosch commented 1 month ago

Yes, I just referenced the Zalando issue's discussion about using the exception's cause in order to include more detailed info in the problem. But I'd sanitize the message.

jpraet commented 1 month ago

Interpreting and transforming the HttpMessageNotReadableException message / cause to a sanitized message seems impractical.

There are many places where the exception is thrown in the Spring Framework, with many different possible exception messages: https://github.com/search?q=org%3Aspring-projects+%22throw+new+HttpMessageNotReadableException%22&type=code&p=1

jpraet commented 1 month ago

PR #110 returns following sanitized detail messages for HttpMessageNotReadableException examples above:

Any other HttpMessageNotReadableException will currently have no problem detail property.

Original HttpMessageNotReadableException with full details and stacktrace is logged on INFO level for problem analysis.