* 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().
This commit is contained in:
hns 2006-05-18 20:54:08 +00:00
parent c8a3c3d702
commit 1121dcbfdc
5 changed files with 49 additions and 45 deletions

View file

@ -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 {

View file

@ -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("<html><body><h3>");
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);
}

View file

@ -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);

View file

@ -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;
}

View file

@ -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();