From 1121dcbfdc7c7acc32476248d26344493dc23a6e Mon Sep 17 00:00:00 2001 From: hns Date: Thu, 18 May 2006 20:54:08 +0000 Subject: [PATCH] * Consider conditional GET headers in RequestTrans.equals(). This fixes a bug where Mozilla/Firefox displayed an empty page when fetching the same page with different headers within a short timeframe. * Fix Last-Modified handler heading which was broken in more than one way. * Don't generate ETag headers for error pages. * Rename ResponseTrans.writeErrorReport() to reportError(). * Set response status to 500 (internal server error) in ResponseTrans.reportError(). --- src/helma/framework/RequestTrans.java | 41 ++++++++++--------- src/helma/framework/ResponseTrans.java | 28 +++++++------ src/helma/framework/core/Application.java | 2 +- .../framework/core/RequestEvaluator.java | 11 +++-- src/helma/servlet/AbstractServletClient.java | 12 +++--- 5 files changed, 49 insertions(+), 45 deletions(-) diff --git a/src/helma/framework/RequestTrans.java b/src/helma/framework/RequestTrans.java index c56b3013..96713046 100644 --- a/src/helma/framework/RequestTrans.java +++ b/src/helma/framework/RequestTrans.java @@ -56,7 +56,7 @@ public class RequestTrans implements Serializable { private String session; // the map of form and cookie data - private final Map values; + private final Map values = new SystemMap(); // the HTTP request method private String method; @@ -65,7 +65,7 @@ public class RequestTrans implements Serializable { private long ifModifiedSince = -1; // set of ETags the client sent with If-None-Match header - private Set etags; + private final Set etags = new HashSet(); // when was execution started on this request? private final long startTime; @@ -83,7 +83,6 @@ public class RequestTrans implements Serializable { this.path = path; this.request = null; this.response = null; - values = new SystemMap(); startTime = System.currentTimeMillis(); } @@ -96,7 +95,6 @@ public class RequestTrans implements Serializable { this.request = request; this.response = response; this.path = path; - values = new SystemMap(); startTime = System.currentTimeMillis(); } @@ -166,25 +164,33 @@ public class RequestTrans implements Serializable { * detect multiple identic requests. */ public int hashCode() { - if (session == null || path == null) + if (session == null || path == null) { return super.hashCode(); - return 17 + (37 * session.hashCode()) + - (37 * path.hashCode()); + } else { + return 17 + (37 * session.hashCode()) + + (37 * path.hashCode()); + } } /** - * A request is considered equal to another one if it has the same user, path, - * and request data. This is used to evaluate multiple simultanous requests only once + * A request is considered equal to another one if it has the same method, + * path, session, request data, and conditional get data. This is used to + * evaluate multiple simultanous identical requests only once. */ public boolean equals(Object what) { - try { - RequestTrans other = (RequestTrans) what; - - return (session.equals(other.session) && path.equalsIgnoreCase(other.path) && - values.equals(other.getRequestData())); - } catch (Exception x) { - return false; + if (what instanceof RequestTrans) { + if (session == null || path == null) { + return super.equals(what); + } else { + RequestTrans other = (RequestTrans) what; + return (session.equals(other.session) + && path.equalsIgnoreCase(other.path) + && values.equals(other.values) + && ifModifiedSince == other.ifModifiedSince + && etags.equals(other.etags)); + } } + return false; } /** @@ -284,11 +290,8 @@ public class RequestTrans implements Serializable { * @param etagHeader ... */ public void setETags(String etagHeader) { - etags = new HashSet(); - if (etagHeader.indexOf(",") > -1) { StringTokenizer st = new StringTokenizer(etagHeader, ", \r\n"); - while (st.hasMoreTokens()) etags.add(st.nextToken()); } else { diff --git a/src/helma/framework/ResponseTrans.java b/src/helma/framework/ResponseTrans.java index 18c745d1..2df7805d 100644 --- a/src/helma/framework/ResponseTrans.java +++ b/src/helma/framework/ResponseTrans.java @@ -477,10 +477,11 @@ public final class ResponseTrans extends Writer implements Serializable { * @param appName the application name * @param message the error message */ - public void writeErrorReport(String appName, String message) { + public void reportError(String appName, String message) { if (reqtrans.isXmlRpc()) { writeXmlRpcError(new RuntimeException(message)); } else { + status = 500; write("

"); write("Error in application "); write(appName); @@ -576,19 +577,22 @@ public final class ResponseTrans extends Writer implements Serializable { } } - // if etag is not set, calc MD5 digest and check it, but only if not a redirect - if (etag == null && lastModified == -1 && redir == null) { + boolean autoETags = "true".equals(app.getProperty("autoETags", "true")); + // if etag is not set, calc MD5 digest and check it, but only if + // not a redirect or error + if (autoETags && + etag == null && + lastModified == -1 && + status == 200 && + redir == null) { try { digest = MessageDigest.getInstance("MD5"); - // if (contentType != null) // digest.update (contentType.getBytes()); byte[] b = digest.digest(response); - etag = "\"" + new String(Base64.encode(b)) + "\""; - // only set response to 304 not modified if no cookies were set - if (reqtrans != null && reqtrans.hasETag(etag) && countCookies() == 0) { + if (reqtrans.hasETag(etag) && countCookies() == 0) { response = new byte[0]; notModified = true; } @@ -671,13 +675,12 @@ public final class ResponseTrans extends Writer implements Serializable { * @param modified the Last-Modified header in milliseconds */ public void setLastModified(long modified) { - if ((modified > -1) && (reqtrans != null) && - (reqtrans.getIfModifiedSince() >= modified)) { + // date headers don't do milliseconds, round to seconds + lastModified = (modified / 1000) * 1000; + if (reqtrans.getIfModifiedSince() == lastModified) { notModified = true; throw new RedirectException(null); } - - lastModified = modified; } /** @@ -696,8 +699,7 @@ public final class ResponseTrans extends Writer implements Serializable { */ public void setETag(String value) { etag = (value == null) ? null : ("\"" + value + "\""); - - if ((etag != null) && (reqtrans != null) && reqtrans.hasETag(etag)) { + if (etag != null && reqtrans.hasETag(etag)) { notModified = true; throw new RedirectException(null); } diff --git a/src/helma/framework/core/Application.java b/src/helma/framework/core/Application.java index 5a65515e..2505445b 100644 --- a/src/helma/framework/core/Application.java +++ b/src/helma/framework/core/Application.java @@ -674,7 +674,7 @@ public final class Application implements IPathElement, Runnable { } catch (Exception x) { errorCount += 1; res = new ResponseTrans(this, req); - res.writeErrorReport(name, x.getMessage()); + res.reportError(name, x.getMessage()); } finally { if (primaryRequest) { activeRequests.remove(req); diff --git a/src/helma/framework/core/RequestEvaluator.java b/src/helma/framework/core/RequestEvaluator.java index 6e1eb931..a4094672 100644 --- a/src/helma/framework/core/RequestEvaluator.java +++ b/src/helma/framework/core/RequestEvaluator.java @@ -248,7 +248,6 @@ public final class RequestEvaluator implements Runnable { currentElement = root; requestPath.add(null, currentElement); - for (int i = 0; i < ntokens; i++) { if (currentElement == null) { throw new FrameworkException("Object not found."); @@ -496,7 +495,7 @@ public final class RequestEvaluator implements Runnable { Thread.sleep((long) (base + (Math.random() * base * 2))); } catch (InterruptedException interrupt) { // we got interrrupted, create minimal error message - res.writeErrorReport(app.getName(), error); + res.reportError(app.getName(), error); done = true; // and release resources and thread rtx = null; @@ -508,7 +507,7 @@ public final class RequestEvaluator implements Runnable { error = "Application too busy, please try again later"; // error in error action. use traditional minimal error message - res.writeErrorReport(app.getName(), error); + res.reportError(app.getName(), error); done = true; } } catch (Throwable x) { @@ -557,7 +556,7 @@ public final class RequestEvaluator implements Runnable { } } else { // error in error action. use traditional minimal error message - res.writeErrorReport(app.getName(), error); + res.reportError(app.getName(), error); done = true; } } @@ -695,7 +694,7 @@ public final class RequestEvaluator implements Runnable { app.logEvent("Stopping Thread for Request " + app.getName() + "/" + req.getPath()); stopTransactor(); res.reset(); - res.writeErrorReport(app.getName(), "Request timed out"); + res.reportError(app.getName(), "Request timed out"); } session.commit(this); @@ -894,7 +893,7 @@ public final class RequestEvaluator implements Runnable { this.reqtype = reqtype; req = new RequestTrans(reqtypeName, functionName); session = new Session(functionName, app); - res = new ResponseTrans(app, getRequest()); + res = new ResponseTrans(app, req); result = null; exception = null; } diff --git a/src/helma/servlet/AbstractServletClient.java b/src/helma/servlet/AbstractServletClient.java index 218eb3bf..29b40e23 100644 --- a/src/helma/servlet/AbstractServletClient.java +++ b/src/helma/servlet/AbstractServletClient.java @@ -332,7 +332,7 @@ public abstract class AbstractServletClient extends HttpServlet { } // write response - writeResponse(request, response, restrans); + writeResponse(request, response, reqtrans, restrans); } catch (Exception x) { try { if (debug) { @@ -352,8 +352,9 @@ public abstract class AbstractServletClient extends HttpServlet { } } - void writeResponse(HttpServletRequest req, HttpServletResponse res, - ResponseTrans hopres) throws IOException { + protected void writeResponse(HttpServletRequest req, HttpServletResponse res, + RequestTrans hopreq, ResponseTrans hopres) + throws IOException { if (hopres.getForward() != null) { sendForward(res, req, hopres); return; @@ -389,9 +390,8 @@ public abstract class AbstractServletClient extends HttpServlet { // set last-modified header to now long modified = hopres.getLastModified(); - if (modified > -1) { - res.setDateHeader("Last-Modified", System.currentTimeMillis()); + res.setDateHeader("Last-Modified", modified); } res.setContentLength(hopres.getContentLength()); @@ -498,7 +498,7 @@ public abstract class AbstractServletClient extends HttpServlet { long lastModified = (file.lastModified() / 1000) * 1000; long ifModifiedSince = req.getDateHeader("If-Modified-Since"); if (lastModified == ifModifiedSince) { - res.setStatus(304); + res.setStatus(HttpServletResponse.SC_NOT_MODIFIED); return; } int length = (int) file.length();