--- /dev/null
+ *) core: add `final_resp_passed` flag to request_rec to allow
+ ap_die() to judge if it can send out a response. Bump mmn.
+ Enable test cases that check errors during response body to
+ appear as error on client side.
+ [Stefan Eissing]
\ No newline at end of file
* 20211221.12 (2.5.1-dev) Add cmd_parms->regex
* 20211221.13 (2.5.1-dev) Add hook token_checker to check for authorization other
* than username / password. Add autht_provider structure.
+ * 20211221.14 (2.5.1-dev) Add request_rec->final_resp_passed bit
*/
#define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20211221
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 13 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 14 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
* to conclude that no body is there.
*/
int body_indeterminate;
+ /** Whether a final (status >= 200) RESPONSE BUCKET has been passed down
+ * the output filters already. Relevant for ap_die().
+ * TODO: compact elsewhere
+ */
+ unsigned int final_resp_passed:1;
};
/**
if (ctx->final_status && ctx->final_header_only) {
/* The final RESPONSE has already been sent or is in front of `bcontent`
- * in the brigade. For a header_only respsone, remove all content buckets
+ * in the brigade. For a header_only respone, remove all content buckets
* up to the first EOS. On seeing EOS, we remove ourself and are done.
* NOTE that `header_only` responses never generate trailes.
*/
if (!APR_BRIGADE_EMPTY(b)) {
rv = ap_pass_brigade(f->next, b);
}
+ r->final_resp_passed = 1;
return rv;
}
int status;
eb = e->data;
status = eb->status;
- apr_brigade_cleanup(b);
- ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, r,
- "ap_http_header_filter error bucket, die with %d and error",
- status);
- /* This will invoke us again */
- ctx->dying = 1;
- ap_die(status, r);
- return AP_FILTER_ERROR;
+ if (r->final_resp_passed) {
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, r,
+ "ap_http_header_filter error bucket, should "
+ "die with status=%d but final response already "
+ "underway", status);
+ ap_remove_output_filter(f);
+ APR_BUCKET_REMOVE(e);
+ apr_brigade_cleanup(b);
+ APR_BRIGADE_INSERT_TAIL(b, e);
+ e = ap_bucket_eoc_create(c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(b, e);
+ e = apr_bucket_eos_create(c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(b, e);
+ c->aborted = 1;
+ return ap_pass_brigade(f->next, b);
+ }
+ else {
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, r,
+ "ap_http_header_filter error bucket, die with %d and error",
+ status);
+ apr_brigade_cleanup(b);
+ /* This will invoke us again */
+ ctx->dying = 1;
+ ap_die(status, r);
+ return AP_FILTER_ERROR;
+ }
}
}
}
rv = ap_pass_brigade(f->next, b);
out:
+ if (ctx->final_status)
+ r->final_resp_passed = 1;
if (recursive_error) {
return AP_FILTER_ERROR;
}
return;
}
- 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 we have already passed the final response down the
+ * output filter chain, we cannot generate a second final
+ * response here.
+ */
+ if (r->final_resp_passed) {
+ return;
+ }
- /*
- * 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;
+ if (!ap_is_HTTP_VALID_RESPONSE(type)) {
+ if (type != AP_FILTER_ERROR) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01579)
+ "Invalid response status %i", type);
}
else {
- return;
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02831)
+ "Response from AP_FILTER_ERROR");
}
+ type = HTTP_INTERNAL_SERVER_ERROR;
}
/*
assert e['bytes_rx_I'] > 0
assert e['bytes_resp_B'] == 100*1024*1024
assert e['bytes_tx_O'] > 1024
- assert e['bytes_tx_O'] < 5*1024*1024 # curl buffers, but not that much
+ assert e['bytes_tx_O'] < 10*1024*1024 # curl buffers, but not that much
found = True
break
assert found, f'request not found in {self.LOGFILE}'
# produce an error during response body
def test_h2_500_31(self, env, repeat):
- if env.httpd_is_at_least("2.5.0"):
- pytest.skip("needs fix in core protocol handling")
url = env.mkurl("https", "cgi", "/proxy/h2test/error?body_error=timeout")
r = env.curl_get(url)
assert r.exit_code != 0, r
# produce an error, fail to generate an error bucket
def test_h2_500_32(self, env, repeat):
- if env.httpd_is_at_least("2.5.0"):
- pytest.skip("needs fix in core protocol handling")
url = env.mkurl("https", "cgi", "/proxy/h2test/error?body_error=timeout&error_bucket=0")
r = env.curl_get(url)
assert r.exit_code != 0, r
"data-1k", "data-10k", "data-100k", "data-1m",
])
def test_h2_601_03_echo_fail_early(self, env, name):
- if env.httpd_is_at_least("2.5.0"):
- pytest.skip("needs mod_proxy_http2 fix")
fpath = os.path.join(env.gen_dir, name)
url = env.mkurl("https", "cgi", "/h2proxy/h2test/echo?fail_after=512")
r = env.curl_upload(url, fpath, options=[])
"data-1k", "data-10k", "data-100k", "data-1m",
])
def test_h2_601_04_echo_fail_late(self, env, name):
- if env.httpd_is_at_least("2.5.0"):
- pytest.skip("needs mod_proxy_http2 fix")
fpath = os.path.join(env.gen_dir, name)
url = env.mkurl("https", "cgi", f"/h2proxy/h2test/echo?fail_after={os.path.getsize(fpath)}")
r = env.curl_upload(url, fpath, options=[])
assert r.exit_code == 92 or r.response["status"] == 502
def test_h2_601_05_echo_fail_many(self, env):
- if env.httpd_is_at_least("2.5.0"):
- pytest.skip("needs mod_proxy_http2 fix")
count = 200
fpath = os.path.join(env.gen_dir, "data-100k")
args = [env.curl, '--parallel', '--parallel-max', '20']