PATCH 58177
Hi List,
The patch allows an HTTP client to send trailing headers to an origin
server through an httpd in the HTTP proxy mode.
It filters headers which are not declared in the Trailer header.
The patch is applicable to 2.4.16 and 2.4.17.
--
Daneel Yaitskov
diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index 319721f..36912ed 100644
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -196,6 +196,39 @@ static void add_te_chunked(apr_pool_t *p,
APR_BRIGADE_INSERT_TAIL(header_brigade, e);
}
+static void add_trailer_line(const char * key, const char *val, apr_pool_t *p,
+ apr_bucket_alloc_t *bucket_alloc,
+ apr_bucket_brigade *bb)
+{
+apr_bucket *e;
+char *buf;
+
+buf = apr_pstrcat(p, key, ": ", val, CRLF, NULL);
+ap_xlate_proto_to_ascii(buf, strlen(buf));
+
+e = apr_bucket_immortal_create(buf, strlen(buf), bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(bb, e);
+}
+
+static void forward_declared_trailers(apr_pool_t *p, const request_rec *r,
+ apr_bucket_alloc_t *bucket_alloc,
+ apr_bucket_brigade *bb)
+{
+const char *declared_trailers = apr_table_get(r->headers_in, "Trailer");
+if (declared_trailers) {
+char *mod_trailers = apr_pstrcat(p, declared_trailers, NULL);
+char *key;
+char *last = NULL;
+while ((key = apr_strtok(mod_trailers, ", ", &last))) {
+const char *value = apr_table_get(r->trailers_in, key);
+if (value) {
+add_trailer_line(key, value, p, bucket_alloc, bb);
+}
+mod_trailers = last;
+}
+}
+}
+
static void add_cl(apr_pool_t *p,
apr_bucket_alloc_t *bucket_alloc,
apr_bucket_brigade *header_brigade,
@@ -344,10 +377,18 @@ static int stream_reqbody_chunked(apr_pool_t *p,
bb = input_brigade;
}
-e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF
+if (apr_is_empty_table(r->trailers_in)) {
+e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF
/* */
ASCII_CRLF,
5, bucket_alloc);
+} else {
+e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF,
+ 3, bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(bb, e);
+forward_declared_trailers(p, r, bucket_alloc, bb);
+e = apr_bucket_immortal_create(ASCII_CRLF, 2, bucket_alloc);
+}
APR_BRIGADE_INSERT_TAIL(bb, e);
if (apr_table_get(r->subprocess_env, "proxy-sendextracrlf")) {
@@ -932,7 +973,18 @@ skip_body:
e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(header_brigade, e);
}
-
+/** pass tailer header and enforce chunked */
+if (old_te_val) {
+const char *trailer = apr_table_get(r->headers_in, "Trailer");
+if (trailer) {
+char *buf = apr_pstrcat(p, "Trailer: ", trailer, CRLF, NULL);
+ap_xlate_proto_to_ascii(buf, strlen(buf));
+APR_BRIGADE_INSERT_TAIL(header_brigade,
+apr_bucket_pool_create(buf, strlen(buf),
+ p, bucket_alloc));
+rb_method = RB_STREAM_CHUNKED;
+}
+}
/* send the request body, if any. */
switch(rb_method) {
case RB_STREAM_CHUNKED:
Re: svn commit: r1711479 - in /httpd/httpd/trunk/server/mpm: event/event.c prefork/prefork.c worker/worker.c
Hi Jan, On Fri, Oct 30, 2015 at 3:07 PM, wrote: > Author: jkaluza > Date: Fri Oct 30 14:07:28 2015 > New Revision: 1711479 > > URL: http://svn.apache.org/viewvc?rev=1711479&view=rev > Log: > Fix crash in ap_mpm_pod_check call caused by NULL dereference of its parameter > when starting httpd as single process (httpd -X). > > Modified: > httpd/httpd/trunk/server/mpm/event/event.c > httpd/httpd/trunk/server/mpm/prefork/prefork.c > httpd/httpd/trunk/server/mpm/worker/worker.c Is that change also needed for event and worker? The old code (previous to r1705492 in 2.4.x) did not create the POD in one-process mode for those. Regards, Yann.
AW: svn commit: r1711479 - in /httpd/httpd/trunk/server/mpm: event/event.c prefork/prefork.c worker/worker.c
> -Ursprüngliche Nachricht- > Von: Yann Ylavic [mailto:ylavic@gmail.com] > Gesendet: Freitag, 30. Oktober 2015 15:40 > An: dev@httpd.apache.org > Betreff: Re: svn commit: r1711479 - in /httpd/httpd/trunk/server/mpm: > event/event.c prefork/prefork.c worker/worker.c > > Hi Jan, > > On Fri, Oct 30, 2015 at 3:07 PM, wrote: > > Author: jkaluza > > Date: Fri Oct 30 14:07:28 2015 > > New Revision: 1711479 > > > > URL: http://svn.apache.org/viewvc?rev=1711479&view=rev > > Log: > > Fix crash in ap_mpm_pod_check call caused by NULL dereference of its > parameter > > when starting httpd as single process (httpd -X). > > > > Modified: > > httpd/httpd/trunk/server/mpm/event/event.c > > httpd/httpd/trunk/server/mpm/prefork/prefork.c > > httpd/httpd/trunk/server/mpm/worker/worker.c > > Is that change also needed for event and worker? > The old code (previous to r1705492 in 2.4.x) did not create the POD in > one-process mode for those. We only call ap_mpm_pod_check in prefork and motorz. So why is this needed for worker and event? Regards Rüdiger
Re: svn commit: r1711479 - in /httpd/httpd/trunk/server/mpm: event/event.c prefork/prefork.c worker/worker.c
On Fri, Oct 30, 2015 at 4:04 PM, Plüm, Rüdiger, Vodafone Group wrote: > > >> -Ursprüngliche Nachricht- >> Von: Yann Ylavic [mailto:ylavic@gmail.com] >> Gesendet: Freitag, 30. Oktober 2015 15:40 >> An: dev@httpd.apache.org >> Betreff: Re: svn commit: r1711479 - in /httpd/httpd/trunk/server/mpm: >> event/event.c prefork/prefork.c worker/worker.c >> >> Hi Jan, >> >> On Fri, Oct 30, 2015 at 3:07 PM, wrote: >> > Author: jkaluza >> > Date: Fri Oct 30 14:07:28 2015 >> > New Revision: 1711479 >> > >> > URL: http://svn.apache.org/viewvc?rev=1711479&view=rev >> > Log: >> > Fix crash in ap_mpm_pod_check call caused by NULL dereference of its >> parameter >> > when starting httpd as single process (httpd -X). >> > >> > Modified: >> > httpd/httpd/trunk/server/mpm/event/event.c >> > httpd/httpd/trunk/server/mpm/prefork/prefork.c >> > httpd/httpd/trunk/server/mpm/worker/worker.c >> >> Is that change also needed for event and worker? >> The old code (previous to r1705492 in 2.4.x) did not create the POD in >> one-process mode for those. > > We only call ap_mpm_pod_check in prefork and motorz. We use the podx variant for worker and event. > So why is this needed for > worker and event? Sorry, I may have not been clear enough. My point is that this change/commit is *not* needed for event and worker, because (AFAICT) we don't touch the POD(x) in one-process mode, so we don't need to create it for these MPMs. Regards, Yann.
RE: merging Apache context
Hi Bill (this is a resend to include the dev and user communities, per your
instructions):
Thanks very much for the prompt response. I believe we have covered all of the
steps you indicate below. Attached please find a tar file that contains a
simple C++ module and a makefile to build it.
Our test server http.conf has a LoadModule line for the module and then the
following set of directory sections:
dirConfig /foo
dirConfig /
dirConfig /foo/bar
When we execute the server, we see this on the console:
# bin/apachectl start
Created dirConfig for context: unset
Created dirConfig for context: /foo/
Inserted dirConfig with message: /foo
Created dirConfig for context: /
Inserted dirConfig with message: /
Created dirConfig for context: /foo/bar/
Inserted dirConfig with message: /foo/bar
Created dirConfig for context: unset
And this in the log:
Merged config: unset:/
…
hookFunc: for request /foo/bar/foo.html the context is: unset:/
We do not see any other messages. The file /foo/bar/foo.html exists and the
browser displays it. This suggests that the merge sequence for the last
directory section stopped with the configuration for the “/” section. Perhaps
you could point out what would cause this behavior?
Thanks:
Adam
From: William A. Rowe Jr. [mailto:wr...@pivotal.io]
Sent: Thursday, October 29, 2015 6:04 PM
To: PAN, JIN
Cc: wr...@apache.org; Greenberg, Adam
Subject: Re: merging Apache context
Hi Jin,
there might be more than one thing going on here.
First, it is critical that a directive belonging to the module occurs in each
of the blocks you are merging.
Remember httpd is not going to even create a config section, never mind merge
them, for every module whose directives do not appear in a given -
this is what makes httpd so efficient.
Second, if there is a bug in the cmd handler, your ctx member might not be
correctly updated for a section, same is true for a bug in the create or merge
function.
Third, httpd does perform some optimization, it may premerge global configs and
may resume a merge from previously merged sections when they are encountered in
a subrequest.
Is the resulting ->ctx member correct for the resulting context?
Is it simply that the merge isn't called as often as expected? Optimization
may be the cause.
Is the cmd record for your directive set to OR_ACCESS (telling httpd that it is
a per-dir and not per-server config?)
Is there a bug in your create code that is returning NULL instead of a newly
initialized config section?
If we look at the example of mod_dir.c, here are the key points...
AP_DECLARE_MODULE(dir) = {
STANDARD20_MODULE_STUFF,
create_dir_config, /* create per-directory config structure */
merge_dir_configs, /* merge per-directory config structures */
All is well, we have a create + merge handler...
static void *create_dir_config(apr_pool_t *p, char *dummy)
{
dir_config_rec *new = apr_pcalloc(p, sizeof(dir_config_rec));
new->index_names = NULL;
new->do_slash = MODDIR_UNSET;
new->checkhandler = MODDIR_UNSET;
new->redirect_index = REDIRECT_UNSET;
return (void *) new;
}
the correct structure size is created and members initialized to empty (e.g.
'unset') - the new allocation is returned.
static void *merge_dir_configs(apr_pool_t *p, void *basev, void *addv)
{
dir_config_rec *new = apr_pcalloc(p, sizeof(dir_config_rec));
dir_config_rec *base = (dir_config_rec *)basev;
dir_config_rec *add = (dir_config_rec *)addv;
new->index_names = add->index_names ? add->index_names : base->index_names;
new->do_slash =
(add->do_slash == MODDIR_UNSET) ? base->do_slash : add->do_slash;
new->checkhandler =
(add->checkhandler == MODDIR_UNSET) ? base->checkhandler :
add->checkhandler;
new->redirect_index=
(add->redirect_index == REDIRECT_UNSET) ? base->redirect_index :
add->redirect_index;
new->dflt = add->dflt ? add->dflt : base->dflt;
return new;
}
A new config is created, the various per-dir values updated, and the resulting
new allocation is returned.
static const command_rec dir_cmds[] =
{
...
AP_INIT_RAW_ARGS("DirectoryIndex", add_index, NULL, DIR_CMD_PERMS,
"a list of file names"),
The DIR_CMD_PERMS (defined as OR_INDEXES) assures httpd that this is a per-dir
config directive allowed wherever the 'AllowOverride Indexes' is set.
static const char *add_index(cmd_parms *cmd, void *dummy, const char *arg)
{
dir_config_rec *d = dummy;
const char *t, *w;
int count = 0;
if (!d->index_names) {
d->index_names = apr_array_make(cmd->pool, 2, sizeof(char *));
}
t = arg;
while ((w = ap_getword_conf(cmd->pool, &t)) && w[0]) {
if (count == 0 && !strcasecmp(w, "disabled")) {
/* peek to see if "disabled" is first in a series of arguments */
const char *tt = t;
const char *ww = ap_
Re: merging Apache context
On Fri, Oct 30, 2015 at 5:23 PM, Greenberg, Adam wrote: > Hi Bill (this is a resend to include the dev and user communities, per > your instructions): > > Replied over on users@ since this seems to be a using-the-API sort of discussion.
