Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix error-page for POST,PUT, etc. #1498

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

thomasmey
Copy link

the view controller error in authz-config.xml forwards to the resp. JSP,
but the error page only supports GET method.

Install a real error controller that at least logs all error messages
and then forwards to the view controller, so not handled exceptions for
e.g. POST request can actually be found.

the view controller error in authz-config.xml forwards to the resp. JSP,
but the error page only supports GET method.

Install a real error controller that at least logs all error messages
and then forwards to the view controller, so not handled exceptions for
e.g. POST request can actually be found.
@codecov-io
Copy link

codecov-io commented Jul 19, 2019

Codecov Report

Merging #1498 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1498   +/-   ##
=========================================
  Coverage     25.21%   25.21%           
  Complexity      912      912           
=========================================
  Files           209      209           
  Lines         11689    11689           
  Branches       2116     2116           
=========================================
  Hits           2947     2947           
  Misses         8227     8227           
  Partials        515      515

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc6bd4b...e2d1824. Read the comment docs.

<error-page>
<location>/error</location>
<location>/errorController</location>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we still map this to /error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm not sure this is possbile without breaking things. Some other jsp pages directly redirect/forwards to /error when changing the above "catch all" error location these wont get catch. Or do I misunderstand. Let me ask the otherway around:

  • under what path should the ErrorController be reachable?
  • under what path should the error.jsp be reachable?

another thing that should be done is to move the controller logic from error.jsp in the ErrorController.

<artifactId>servlet-api</artifactId>
<version>2.5</version>
<artifactId>javax.servlet-api</artifactId>
<version>3.0.1</version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other changes that updating the servlet version would bring to the underlying project?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this gives the static defines like RequestDispatcher.ERROR_MESSAGE as web.xml uses 3.0 syntax already.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants