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

Request error context is not reset #5454

Closed
kh4mell opened this issue Oct 16, 2020 · 1 comment · Fixed by #5456
Closed

Request error context is not reset #5454

kh4mell opened this issue Oct 16, 2020 · 1 comment · Fixed by #5456

Comments

@kh4mell
Copy link

kh4mell commented Oct 16, 2020

Jetty 9.4.22.v20191022

Java openjdk 11.0.8 2020-07-14 LTS

OS CentOS Linux 7

I'm using jetty with pax-web 7.2.14 (Karaf 4.2.8).
I created bundle fragment and registered custom error handler using jetty.xml:

   <Configure id="Server" class="org.eclipse.jetty.server.Server">
      ...
        <Call name="setErrorHandler">
            <Arg>
                <New class="com.company.jetty.error.handler.ServerErrorPageHandler"/>
            </Arg>
        </Call>
       ....
   </Configure>

In most cases it works correctly, however there is a race condition: sometimes I receive default jetty error page.

Scenario:

  • Access non-existing context-path (no application registed for the context-path): GET /applications/application/999999

Expected:

  • Error page generated by com.company.jetty.error.handler.ServerErrorPageHandler is returned

Actual:

  • Error page generated by org.eclipse.jetty.servlet.ErrorPageErrorHandler is returned.

During debugging I noticed that org.eclipse.jetty.server.Request#_errorContext is not null while org.eclipse.jetty.server.Request#_context and org.eclipse.jetty.server.Request#_contextPath are null.
Path of the _errorContext is a random path of the one of registered applications (it's not /applications/application/999999 but for example /platform/api).

There is no _errorContext cleanup in org.eclipse.jetty.server.Request#setContext method.

    public void setContext(Context context)
    {
        _newContext = _context != context;
        if (context == null)
            _context = null;
        else
        {
            _context = context;
            _errorContext = context;
        }
    }

Is there a defect with _errorContext cleanup?

@gregw gregw added this to To do in Jetty 9.4.33 via automation Oct 16, 2020
gregw added a commit that referenced this issue Oct 16, 2020
Reset the error context when a request is recycled.
@gregw gregw changed the title Race condition while using custom ErrorHandler Request error context is not reset Oct 16, 2020
@gregw
Copy link
Contributor

gregw commented Oct 16, 2020

Thanks for the report. The issue is not a race, simply that we do not reset the errorContext when a request is recycled. This is normally not a problem as errors frequently close the connection and thus the request is not recycled. But it can be reused in some circumstances and I have reproduced and fixed. PR coming....

@gregw gregw moved this from To do to Review in progress in Jetty 9.4.33 Oct 16, 2020
joakime pushed a commit that referenced this issue Oct 16, 2020
Reset the error context when a request is recycled.
joakime added a commit that referenced this issue Oct 16, 2020
@joakime joakime linked a pull request Oct 16, 2020 that will close this issue
Jetty 9.4.33 automation moved this from Review in progress to Done Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Jetty 9.4.33
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants