That is, not only when a hook/handler returns AP_FILTER_ERROR.
Graham (initially) and I made some changes to trunk (backport proposed
in r1665737) so that no internal module should return anything but
AP_FILTER_ERROR in this case, but we can't do much for third-party
modules.
That way ap_die() could become a safe gard against unintentional
double responses (hence HRS) for the same request, in any case since
it is always called at the end.
Something like:
Index: modules/http/http_request.c
===
--- modules/http/http_request.c(revision 1665647)
+++ modules/http/http_request.c(working copy)
@@ -75,6 +75,7 @@ static void update_r_in_filters(ap_filter_t *f,
static void ap_die_r(int type, request_rec *r, int recursive_error)
{
+ap_filter_t *next;
char *custom_response;
request_rec *r_1st_err = r;
@@ -83,38 +84,34 @@ static void ap_die_r(int type, request_rec *r, int
return;
}
+/*
+ * Check if we still have the ap_http_header_filter in place. If
+ * this is the case we should not ignore the error here because
+ * it means that we have not sent any response at all. Otherwise,
+ * don't send two responses for the same request.
+ */
+next = r-output_filters;
+while (next (next-frec != ap_http_header_filter_handle)) {
+next = next-next;
+}
+if (!next) {
+return;
+}
+
+/*
+ * AP_FILTER_ERROR or any invalid status, send an internal server
+ * error instead.
+ */
if (!ap_is_HTTP_VALID_RESPONSE(type)) {
-ap_filter_t *next;
-
-/*
- * Check if we still have the ap_http_header_filter in place. If
- * this is the case we should not ignore the error here because
- * it means that we have not sent any response at all and never
- * will. This is bad. Sent an internal server error instead.
- */
-next = r-output_filters;
-while (next (next-frec != ap_http_header_filter_handle)) {
- next = next-next;
-}
-
-/*
- * If next != NULL then we left the while above because of
- * next-frec == ap_http_header_filter
- */
-if (next) {
-if (type != AP_FILTER_ERROR) {
-ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01579)
- Invalid response status %i, type);
-}
-else {
-ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02831)
- Response from AP_FILTER_ERROR);
-}
-type = HTTP_INTERNAL_SERVER_ERROR;
-}
-else {
-return;
-}
+if (type != AP_FILTER_ERROR) {
+ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01579)
+ Invalid response status %i, type);
+}
+else {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02831)
+ Response from AP_FILTER_ERROR);
+}
+type = HTTP_INTERNAL_SERVER_ERROR;
}
/*
--
This is not a great cycles loss since this path is not a fast one
already (!OK !DONE = ap_send_error_response() or
ap_internal_redirect()), and the inconditional loop is not really
heavy...
Thoughts?
Regards,
Yann.
