Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exceptions in this module are bypassing @ControllerAdvice #1727

Open
nkavian opened this issue Nov 15, 2020 · 0 comments
Open

Exceptions in this module are bypassing @ControllerAdvice #1727

nkavian opened this issue Nov 15, 2020 · 0 comments
Assignees
Labels

Comments

@nkavian
Copy link

nkavian commented Nov 15, 2020

Describe the bug
When this module throws an exception, it's somehow bypassing @ControllerAdvice and renders the stack trace into the browser. The expected behavior is that the configured @ControllerAdvice should be called like all other exceptions so that I can display an appropriate error HTML. Rendering the stack trace is also against security best practices.

To Reproduce

  1. Create a controller advice to catch all Throwables, something like:
    @ExceptionHandler(Throwable.class)
    @ResponseStatus(code = HttpStatus.INTERNAL_SERVER_ERROR)
    public Object exception(final HttpServletRequest request, final Exception exception) {
        System.out.println("this is never called");
        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build();
    }
  1. Load your sample JDBC application.
  2. While running, rename the the attributes table so it no longer exists; i.e. we are trying to cause any exception, but assume this scenario is like a database or network connectivity issue.
  3. Browse to your app.
  4. Watch a stack trace being rendered to the screen and the above controller advice method is never called.
  5. If I perform the same steps with other tables I use in my application, the above method is called and I am able to render an appropriate HTML error page.
  6. Looks like bugsnag's unhandled exceptions is not even called so I won't even know there was a user experience problem or error. Since a session is processed very early in the filtering, if there is a failure, it will cause me to be blind. Let's say the database is down, your bug here will trigger, and I would not have received a bugsnag alert, for let's say, some other table not being accessible which I do receive alerts on.

Expected behavior
The expected behavior is that the configured @ControllerAdvice should be called like all other exceptions so that I can display an appropriate error HTML.

In terms of PCI security, devops are obligated to hide application server versions. So the above error is cause not only the stack trace but also prints in the footer Apache Tomcat/9.0.17. The purpose behind this PCI security item is that hackers will try to sniff application versions so that they can find bugs affecting that application version and attack.

Support ControllerAdvice handling as well as support unhandled exceptions correctly so that Bugsnag can detect it. Just to reiterate, you display a stack trace in the browser for customers to see, you send it to the console output/log file which is not monitored and likely deleted when the cluster is redeployed, and so this amounts to silently failing and swallowing the errors. This might be okay only in a development environment when running the code locally..

@nkavian nkavian added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 15, 2020
@eleftherias eleftherias self-assigned this Nov 17, 2020
@eleftherias eleftherias added in: jdbc and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants