smallrye-graphql: Stop leaking exception information in errors

It seems like the following extensions are added to errors by default:

"extensions": {
  "exception": "java.lang.NullPointerException",
  "classification": "DataFetchingException",
  "code": "null-pointer"
}

I think this should be considered information leakage and improper error handling. See OWASP: https://owasp.org/www-community/Improper_Error_Handling

I think the @ErrorCode annotation is useful, because it lets GraphQL clients show actionable errors, but I think it is worth arguing that only very specific exceptions should have error codes. NPE is usually not something the end user can do anything about and should not have a code.

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 26 (12 by maintainers)

Most upvoted comments

I based it on: https://github.com/smallrye/smallrye-graphql/issues/772#issuecomment-832509614

I think we have multiple issues,

the ability to turn of the exception details we add to extensions. (this issue)
the ability to contribute (key-value) to the extensions (#299)
the ability to control graphql-java errors (new issue)

phillip-kruger on May 5, 2021

Yea, so for those, it will be a breaking change (in the sense that they will have to add a config) but I think that is the correct default. I’ll make a change a.s.a.p

phillip-kruger on May 5, 2021

I agree that we should not show the error details in the extensions by default. I suggest we add a config to switch this feature on (like it is currently) and off, and making the default off. w.d.y.t ?

phillip-kruger on May 5, 2021