The The sed
@@ -122,8 +121,7 @@ page.
POST data)sed command
diff --git a/modules/filters/mod_sed.c b/modules/filters/mod_sed.c
index 193a41254ff..424dd0e1c07 100644
--- a/modules/filters/mod_sed.c
+++ b/modules/filters/mod_sed.c
@@ -54,6 +54,14 @@ typedef struct sed_filter_ctxt
apr_size_t bufsize;
apr_pool_t *tpool;
int numbuckets;
+ /* Whether anything has been handed to the next filter yet. Once it
+ * has, a failure can no longer be turned into an error response.
+ */
+ int passed;
+ /* Sticky failure of the evaluation, once the response is beyond
+ * rescue: the rest of the body is dropped but its metadata is not.
+ */
+ apr_status_t evalerr;
} sed_filter_ctxt;
module AP_MODULE_DECLARE_DATA sed_module;
@@ -119,6 +127,7 @@ static apr_status_t append_bucket(sed_filter_ctxt* ctx, char* buf, apr_size_t sz
if (ctx->numbuckets >= MAX_TRANSIENT_BUCKETS) {
b = apr_bucket_flush_create(ctx->r->connection->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
+ ctx->passed = 1;
status = ap_pass_brigade(ctx->f->next, ctx->bb);
apr_brigade_cleanup(ctx->bb);
clear_ctxpool(ctx);
@@ -272,11 +281,32 @@ static apr_status_t init_context(ap_filter_t *f, sed_expr_config *sed_cfg, int u
return APR_SUCCESS;
}
+/* keep_metadata
+ * The response has been broken part-way through. Whatever is left in bb is
+ * unfiltered content which must not be sent, but its metadata -- above all
+ * the EOS -- still has to reach the next filter, or the response is never
+ * terminated.
+ */
+static void keep_metadata(sed_filter_ctxt *ctx, apr_bucket_brigade *bb)
+{
+ while (!APR_BRIGADE_EMPTY(bb)) {
+ apr_bucket *b = APR_BRIGADE_FIRST(bb);
+ APR_BUCKET_REMOVE(b);
+ if (APR_BUCKET_IS_METADATA(b)) {
+ APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
+ }
+ else {
+ apr_bucket_destroy(b);
+ }
+ }
+}
+
/* Entry function for Sed output filter */
static apr_status_t sed_response_filter(ap_filter_t *f,
apr_bucket_brigade *bb)
{
apr_bucket *b;
+ apr_status_t rv;
apr_status_t status = APR_SUCCESS;
sed_config *cfg = ap_get_module_config(f->r->per_dir_config,
&sed_module);
@@ -291,8 +321,12 @@ static apr_status_t sed_response_filter(ap_filter_t *f,
if (ctx == NULL) {
+ if (APR_BRIGADE_EMPTY(bb)) {
+ return ap_pass_brigade(f->next, bb);
+ }
+
if (APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(bb))) {
- /* no need to run sed filter for Head requests */
+ /* No body to filter */
ap_remove_output_filter(f);
return ap_pass_brigade(f->next, bb);
}
@@ -305,6 +339,19 @@ static apr_status_t sed_response_filter(ap_filter_t *f,
ctx->bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
}
+ else if (ctx->evalerr != APR_SUCCESS) {
+ /* The evaluation failed for an earlier brigade of this response,
+ * after some of it had already gone out. Nothing more can be
+ * filtered, but the metadata still has to be carried through so
+ * that the EOS ends the response.
+ */
+ keep_metadata(ctx, bb);
+ if (!APR_BRIGADE_EMPTY(ctx->bb)) {
+ ap_pass_brigade(f->next, ctx->bb);
+ apr_brigade_cleanup(ctx->bb);
+ }
+ return ctx->evalerr;
+ }
/* Here is the main logic. Iterate through all the buckets, read the
* content of the bucket, call sed_eval_buffer on the data.
@@ -328,49 +375,72 @@ static apr_status_t sed_response_filter(ap_filter_t *f,
*/
while (!APR_BRIGADE_EMPTY(bb)) {
b = APR_BRIGADE_FIRST(bb);
- if (APR_BUCKET_IS_EOS(b)) {
- /* Now clean up the internal sed buffer */
- sed_finalize_eval(&ctx->eval, ctx);
- status = flush_output_buffer(ctx);
- if (status != APR_SUCCESS) {
- break;
+ if (APR_BUCKET_IS_METADATA(b)) {
+ if (APR_BUCKET_IS_EOS(b)) {
+ /* Now clean up the internal sed buffer */
+ sed_finalize_eval(&ctx->eval, ctx);
}
- /* Move the eos bucket to ctx->bb brigade */
- APR_BUCKET_REMOVE(b);
- APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
- }
- else if (APR_BUCKET_IS_FLUSH(b)) {
+ /* Flush what has been generated so far, so that the metadata
+ * keeps its place in the stream, and move it across. Buckets
+ * this filter has no opinion on are carried through rather
+ * than dropped: an error bucket has to reach the filters which
+ * act on it.
+ */
status = flush_output_buffer(ctx);
if (status != APR_SUCCESS) {
break;
}
- /* Move the flush bucket to ctx->bb brigade */
APR_BUCKET_REMOVE(b);
APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
}
else {
- if (!APR_BUCKET_IS_METADATA(b)) {
- const char *buf = NULL;
- apr_size_t bytes = 0;
+ const char *buf = NULL;
+ apr_size_t bytes = 0;
- status = apr_bucket_read(b, &buf, &bytes, APR_BLOCK_READ);
- if (status == APR_SUCCESS) {
- status = sed_eval_buffer(&ctx->eval, buf, bytes, ctx);
- }
- if (status != APR_SUCCESS) {
- ap_log_rerror(APLOG_MARK, APLOG_ERR, status, f->r, APLOGNO(10394) "error evaluating sed on output");
- break;
- }
+ status = apr_bucket_read(b, &buf, &bytes, APR_BLOCK_READ);
+ if (status == APR_SUCCESS) {
+ status = sed_eval_buffer(&ctx->eval, buf, bytes, ctx);
+ }
+ if (status != APR_SUCCESS) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, status, f->r, APLOGNO(10394) "error evaluating sed on output");
+ break;
}
apr_bucket_delete(b);
}
}
+
+ /* Flush whatever did evaluate, even after a failure: it is valid output
+ * and the most of the response the client can still be given.
+ */
+ rv = flush_output_buffer(ctx);
if (status == APR_SUCCESS) {
- status = flush_output_buffer(ctx);
+ status = rv;
}
+
+ if (status != APR_SUCCESS) {
+ if (!ctx->passed) {
+ /* None of the response has been written, so the caller can
+ * still turn this into a proper error response. Say nothing
+ * more than that it failed.
+ */
+ apr_brigade_cleanup(ctx->bb);
+ clear_ctxpool(ctx);
+ return status;
+ }
+ /* Too late for that: part of the response is already on its way,
+ * so it has to be ended as the truncated response it has become.
+ * Dropping the EOS here instead leaves the header filters unrun
+ * and the client waiting for a response that never finishes.
+ */
+ ctx->evalerr = status;
+ keep_metadata(ctx, bb);
+ }
+
if (!APR_BRIGADE_EMPTY(ctx->bb)) {
+ ctx->passed = 1;
+ rv = ap_pass_brigade(f->next, ctx->bb);
if (status == APR_SUCCESS) {
- status = ap_pass_brigade(f->next, ctx->bb);
+ status = rv;
}
apr_brigade_cleanup(ctx->bb);
}
diff --git a/modules/filters/regexp.c b/modules/filters/regexp.c
index 4acccca6765..f07fee06eb4 100644
--- a/modules/filters/regexp.c
+++ b/modules/filters/regexp.c
@@ -535,6 +535,13 @@ static int _advance(char *lp, char *ep, step_vars_storage *vars)
bbeg = vars->braslist[epint];
ct = vars->braelist[epint] - bbeg;
ep++;
+ if (ct == 0) {
+ /* The capture matched nothing, so repeating it consumes
+ * nothing: it can only match once, here. Both loops below
+ * step lp by ct and would never make progress.
+ */
+ continue;
+ }
curlp = lp;
while (ecmp(bbeg, lp, ct))
lp += ct;
diff --git a/modules/filters/sed1.c b/modules/filters/sed1.c
index 21d1cc78a50..f693a2a459d 100644
--- a/modules/filters/sed1.c
+++ b/modules/filters/sed1.c
@@ -73,6 +73,8 @@ static apr_status_t command(sed_eval_t *eval, sed_reptr_t *ipc,
step_vars_storage *step_vars);
static apr_status_t wline(sed_eval_t *eval, char *buf, apr_size_t sz);
static apr_status_t arout(sed_eval_t *eval);
+static void eval_errf(sed_eval_t *eval, const char *fmt, ...)
+ __attribute__((format(printf,2,3)));
static void eval_errf(sed_eval_t *eval, const char *fmt, ...)
{
@@ -414,7 +416,7 @@ apr_status_t sed_eval_buffer(sed_eval_t *eval, const char *buf, apr_size_t bufsz
/* Commands were not finalized properly. */
const char* error = sed_get_finalize_error(eval->commands, eval->pool);
if (error) {
- eval_errf(eval, error);
+ eval_errf(eval, "%s", error);
return APR_EGENERAL;
}
}
@@ -777,7 +779,12 @@ static apr_status_t command(sed_eval_t *eval, sed_reptr_t *ipc,
switch(ipc->command) {
case ACOM:
- if (eval->aptr >= &eval->abuf[SED_ABUFSIZE]) {
+ /* One slot has to be left for the NULL which terminates abuf,
+ * or writing it runs off the end of the array and over aptr
+ * itself -- after which the next append writes through a NULL
+ * pointer.
+ */
+ if (eval->aptr >= &eval->abuf[SED_ABUFSIZE - 1]) {
eval_errf(eval, SEDERR_TMAMES, eval->lnum);
} else {
*eval->aptr++ = ipc;
@@ -874,6 +881,13 @@ static apr_status_t command(sed_eval_t *eval, sed_reptr_t *ipc,
continue;
}
if (!isprint(*p1 & 0377)) {
+ /* The three octal digits have to come off the byte
+ * value, not off a sign-extended char: 0xff is
+ * \377, and shifting it as a negative number gave
+ * "\/77".
+ */
+ unsigned char uc = (unsigned char)*p1;
+
*p2++ = '\\';
if (p2 >= eval->lcomend) {
*p2 = '\\';
@@ -883,7 +897,7 @@ static apr_status_t command(sed_eval_t *eval, sed_reptr_t *ipc,
return rv;
p2 = eval->genbuf;
}
- *p2++ = (*p1 >> 6) + '0';
+ *p2++ = (uc >> 6) + '0';
if (p2 >= eval->lcomend) {
*p2 = '\\';
rv = wline(eval, eval->genbuf,
@@ -892,7 +906,7 @@ static apr_status_t command(sed_eval_t *eval, sed_reptr_t *ipc,
return rv;
p2 = eval->genbuf;
}
- *p2++ = ((*p1 >> 3) & 07) + '0';
+ *p2++ = ((uc >> 3) & 07) + '0';
if (p2 >= eval->lcomend) {
*p2 = '\\';
rv = wline(eval, eval->genbuf,
@@ -901,7 +915,8 @@ static apr_status_t command(sed_eval_t *eval, sed_reptr_t *ipc,
return rv;
p2 = eval->genbuf;
}
- *p2++ = (*p1++ & 07) + '0';
+ *p2++ = (uc & 07) + '0';
+ p1++;
if (p2 >= eval->lcomend) {
*p2 = '\\';
rv = wline(eval, eval->genbuf,
@@ -993,7 +1008,8 @@ static apr_status_t command(sed_eval_t *eval, sed_reptr_t *ipc,
break;
case RCOM:
- if (eval->aptr >= &eval->abuf[SED_ABUFSIZE]) {
+ /* See ACOM: the terminating NULL needs a slot of its own. */
+ if (eval->aptr >= &eval->abuf[SED_ABUFSIZE - 1]) {
eval_errf(eval, SEDERR_TMRMES, eval->lnum);
} else {
*eval->aptr++ = ipc;
diff --git a/test/modules/filters/env.py b/test/modules/filters/env.py
index c78a8fac0b2..c0aba5f25c3 100644
--- a/test/modules/filters/env.py
+++ b/test/modules/filters/env.py
@@ -13,6 +13,7 @@ def __init__(self, env: 'HttpdTestEnv'):
super().__init__(env=env)
self.add_source_dir(os.path.dirname(inspect.getfile(FiltersTestSetup)))
self.add_modules(["substitute", "sed"])
+ self.add_cgi_module()
class FiltersTestEnv(HttpdTestEnv):
diff --git a/test/modules/filters/htdocs/test1/cgi/echo.py b/test/modules/filters/htdocs/test1/cgi/echo.py
new file mode 100644
index 00000000000..985c38ec2c6
--- /dev/null
+++ b/test/modules/filters/htdocs/test1/cgi/echo.py
@@ -0,0 +1,12 @@
+#!/usr/bin/env python3
+# Echo the request body back verbatim, so an InputSed test can see exactly
+# what the input filter handed to the handler. Read to EOF rather than
+# CONTENT_LENGTH bytes: mod_sed rewrites the body but not the header, so the
+# two need not agree.
+import sys
+
+body = sys.stdin.buffer.read()
+print("Content-Type: text/plain")
+print()
+sys.stdout.flush()
+sys.stdout.buffer.write(body)
diff --git a/test/modules/filters/htdocs/test1/cgi/nobody.py b/test/modules/filters/htdocs/test1/cgi/nobody.py
new file mode 100644
index 00000000000..d2146834926
--- /dev/null
+++ b/test/modules/filters/htdocs/test1/cgi/nobody.py
@@ -0,0 +1,5 @@
+#!/usr/bin/env python3
+# Headers and nothing else, so the output filters see a response whose body
+# is a lone EOS.
+print("Content-Type: text/html")
+print()
diff --git a/test/modules/filters/test_003_sed.py b/test/modules/filters/test_003_sed.py
new file mode 100644
index 00000000000..be402359c04
--- /dev/null
+++ b/test/modules/filters/test_003_sed.py
@@ -0,0 +1,358 @@
+import os
+import re
+
+import pytest
+
+from pyhttpd.conf import HttpdConf
+
+# mod_sed implements the Solaris 10 sed language over the response (OutputSed)
+# or the request body (InputSed). Almost none of it was covered before, so
+# these tests walk the commands the module documents plus the paths mod_sed.c
+# itself has: the 8000-byte output buffer, the transient-bucket flush, the
+# empty-body short circuit and the error path.
+
+# The document most tests run against. Three lines, trailing newline.
+DOC = "one monday two\nthree sunday four\nmonday monday monday\n"
+
+# huge.html: enough ordinary lines to push mod_sed past MAX_TRANSIENT_BUCKETS
+# (50 buckets of MODSED_OUTBUF_SIZE, so 400000 bytes) and make it flush what it
+# has to the client, then a single line past the 8 MB (MAX_BUF_SIZE) a line may
+# grow to, which fails the evaluation.
+HUGE_HEAD_LINES = 500
+HUGE_LINE_LEN = 900
+HUGE_HEAD_LEN = HUGE_HEAD_LINES * (HUGE_LINE_LEN + 1)
+HUGE_TAIL_LEN = 9 * 1024 * 1024
+
+# For a test whose failure mode is "never answers" rather than "answers
+# wrongly". pyhttpd only gives curl --connect-timeout.
+BOUNDED = ["--max-time", "30"]
+
+
+class TestSed:
+
+ @pytest.fixture(autouse=True, scope='class')
+ def _class_scope(self, env):
+ self.docdir = os.path.join(env.server_dir, "htdocs", "test1")
+ self.write(env, "sed.html", DOC)
+ # Same text with no newline after the last line.
+ self.write(env, "nonl.html", DOC[:-1])
+ self.write(env, "empty.html", "")
+ # One line per 4 KB, well past MODSED_OUTBUF_SIZE and past the 50
+ # transient buckets mod_sed flushes at.
+ self.write(env, "big.html",
+ "".join(f"line {i:06d} monday {'x' * 4000}\n"
+ for i in range(500)))
+ # A byte from each interesting class for the l command: printable,
+ # tab, DEL and three high-bit bytes.
+ self.write_bytes(env, "bytes.html", b"a\tb\x7fc\xffd\x80e\xa9f\n")
+ self.write(env, "insert.txt", "INSERTED\n")
+ # First line has nothing for \(a*\) to capture, second has "aa".
+ self.write(env, "backref.html", "b\naab\n")
+ head = "".join(
+ f"line {i:06d} monday ".ljust(HUGE_LINE_LEN, "y") + "\n"
+ for i in range(HUGE_HEAD_LINES))
+ assert len(head) == HUGE_HEAD_LEN
+ self.write(env, "huge.html", head + "z" * HUGE_TAIL_LEN + "\n")
+
+ @staticmethod
+ def write(env, name, text):
+ TestSed.write_bytes(env, name, text.encode())
+
+ @staticmethod
+ def write_bytes(env, name, data):
+ path = os.path.join(env.server_dir, "htdocs", "test1", name)
+ with open(path, "wb") as f:
+ f.write(data)
+
+ @staticmethod
+ def sed_path(env, name):
+ """The path of a document as an "r" command has to spell it: sed
+ reads a backslash in a filename as an escape and drops it, so a
+ Windows path only survives with forward slashes."""
+ return os.path.join(env.server_dir, "htdocs", "test1",
+ name).replace(os.sep, "/")
+
+ def configure(self, env, exprs, extra="", input_sed=False):
+ if isinstance(exprs, str):
+ exprs = [exprs]
+ directive = "InputSed" if input_sed else "OutputSed"
+ lines = "\n".join(f' {directive} "{e}"' for e in exprs)
+ conf = HttpdConf(env, extras={
+ f"test1.{env.http_tld}": f"""
+