Jetty

Honor application's Cache-Control on 4xx responses

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 6.1.22
  • Fix Version/s: 6.1.23, 7.0.2
  • Component/s: HTTP
  • Labels:
    None
  • Number of attachments :
    0

Description

Here is my code

response.setHeader("Cache-Control", "public,max-age=31536000");
response.sendError(HttpServletResponse.SC_NOT_FOUND);

Expected outcome

HTTP/1.1 404 Not Found
Cache-Control: public,max-age=31536000

Actual outcome

HTTP/1.1 404 Not Found
Cache-Control: must-revalidate,no-cache,no-store

It would be ideal to detect that the application has set the Cache-Control header, and honor it.

Activity

Hide
Greg Wilkins added a comment -

Added setCacheControl to the error handler, so that you can now do
context.getErrorHandler().setCacheControl(null);
to turn off the default cache control.

If need be, we can create a context init parameter that defaultweb.xml can set to also control this. If that is desirable, reopen this issue.

regards

Show
Greg Wilkins added a comment - Added setCacheControl to the error handler, so that you can now do context.getErrorHandler().setCacheControl(null); to turn off the default cache control. If need be, we can create a context init parameter that defaultweb.xml can set to also control this. If that is desirable, reopen this issue. regards
Hide
Subbu Allamaraju added a comment -

Unfortunately, this would require Jetty specific coding.

Since the app has chosen to set a Cache-Control header before calling response.sendError(), why not just honor that without doing anything else?

Show
Subbu Allamaraju added a comment - Unfortunately, this would require Jetty specific coding. Since the app has chosen to set a Cache-Control header before calling response.sendError(), why not just honor that without doing anything else?
Hide
Greg Wilkins added a comment -

Subbu,

I'm a bit cautious about changing behaviour for other application,
so I don't want to change the default behaviour.

Note that within the servlet spec you can create an error page that actually generates the page, and it is free to set cache control headers. In this case, you want your app to set headers that apply to an error page that jetty is generating - so I'm not exactly sure that is the right thing todo. The Jetty error handler should be free to set headers describing the content - it might choose to display a time for example.

Can you tell me more about why this matters to your application and how your application is deployed? Would a jetty specific init parameter be considered "jetty specific coding"?

Show
Greg Wilkins added a comment - Subbu, I'm a bit cautious about changing behaviour for other application, so I don't want to change the default behaviour. Note that within the servlet spec you can create an error page that actually generates the page, and it is free to set cache control headers. In this case, you want your app to set headers that apply to an error page that jetty is generating - so I'm not exactly sure that is the right thing todo. The Jetty error handler should be free to set headers describing the content - it might choose to display a time for example. Can you tell me more about why this matters to your application and how your application is deployed? Would a jetty specific init parameter be considered "jetty specific coding"?
Hide
Subbu Allamaraju added a comment -

Hi Greg,

I see your concern. However, by just honoring the app's call, Jetty's behavior will be aligned with Tomcat. The following scriptlet in a JSP on Tomcat (default installation)

<%
response.setHeader("Cache-Control", "public;max-age=3600");
response.sendError(404);
%>

sets the expected Cache-Control header on the 404 response. The only use case for this behavior is to promote negative caching.

If backwards compat is your key concern, they I don't have a better alternative suggestion.

Show
Subbu Allamaraju added a comment - Hi Greg, I see your concern. However, by just honoring the app's call, Jetty's behavior will be aligned with Tomcat. The following scriptlet in a JSP on Tomcat (default installation) <% response.setHeader("Cache-Control", "public;max-age=3600"); response.sendError(404); %> sets the expected Cache-Control header on the 404 response. The only use case for this behavior is to promote negative caching. If backwards compat is your key concern, they I don't have a better alternative suggestion.
Hide
Greg Wilkins added a comment -

Subbu,

sorry but tomcat is not the reference implementation and I'm not that interested in copying them exactly on this sort of stuff.

The difference is not actually in sendError, both containers will preserve headers.

It is that jetty's default error handler set's the cache control assuming that errors are transient, so you don't cache a 404 or 500 and see that after the problem has been fixed.

Maybe on tomcat errors are more persistent so it's best to cache them

But seriously, neither container is right/wrong. Both are entitled to generate error pages how they like. Jetty chooses to generate non cacheable error pages.

There is a standard mechanism to replace the default error page generation, so you can easily take control of error pages if you don't want to depend on container specific behaviour.

I could check to see if the cache-control was already set, but I have no way of telling if it was generated from code like:

try

{ response.setHeader("Cache-Control", "public;max-age=360000"); generatePageThatCanBeCached(response); }

catch(Exception e)

{ response.sendError(404); }

in this case, the cache control header is set, but actually applies
to the generated page and should not be applied to the error page.

Show
Greg Wilkins added a comment - Subbu, sorry but tomcat is not the reference implementation and I'm not that interested in copying them exactly on this sort of stuff. The difference is not actually in sendError, both containers will preserve headers. It is that jetty's default error handler set's the cache control assuming that errors are transient, so you don't cache a 404 or 500 and see that after the problem has been fixed. Maybe on tomcat errors are more persistent so it's best to cache them But seriously, neither container is right/wrong. Both are entitled to generate error pages how they like. Jetty chooses to generate non cacheable error pages. There is a standard mechanism to replace the default error page generation, so you can easily take control of error pages if you don't want to depend on container specific behaviour. I could check to see if the cache-control was already set, but I have no way of telling if it was generated from code like: try { response.setHeader("Cache-Control", "public;max-age=360000"); generatePageThatCanBeCached(response); } catch(Exception e) { response.sendError(404); } in this case, the cache control header is set, but actually applies to the generated page and should not be applied to the error page.
Hide
Subbu Allamaraju added a comment -

I see. The right way (which most servlet based frameworks/apps don't do) is

try {
// Check for client errors.
if(isValid(request)) { // Happy path leading to 2xx or 3xx ... }
else { // This is a 4xx error. Add negative caching response.setHeader("Cache-Control", "public;max-age=3600"); response.sendError(4xx); }
}
catch(SomeException se) {
// This is a 5xx error. Either set no-cache or add a short max-age with or without a Retry-After
response.setHeader("Cache-Control", "no-cache:no-store");
response.sendError(5xx);
}

I agree that dealing with such nuances in the servlet container is difficult, and that's why I prefer not making any assumptions about the app's intent.

Since this is already closed, I am just leaving my notes for future reference.

Show
Subbu Allamaraju added a comment - I see. The right way (which most servlet based frameworks/apps don't do) is try { // Check for client errors. if(isValid(request)) { // Happy path leading to 2xx or 3xx ... } else { // This is a 4xx error. Add negative caching response.setHeader("Cache-Control", "public;max-age=3600"); response.sendError(4xx); } } catch(SomeException se) { // This is a 5xx error. Either set no-cache or add a short max-age with or without a Retry-After response.setHeader("Cache-Control", "no-cache:no-store"); response.sendError(5xx); } I agree that dealing with such nuances in the servlet container is difficult, and that's why I prefer not making any assumptions about the app's intent. Since this is already closed, I am just leaving my notes for future reference.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: