handler pass to php
Hello all, We have written a handler for apache that talks to our point of sales software. At this point the POS software renders a lot of the html itself and sends it back through apache. We would however, like to be able to integrate PHP to offload some of this work. The problem right now is that once the handler catches the request it renders and sends its through never reaching PHP. This is the same whether or not PHP is built as a fcgi or apache2handler. I'm pretty sure the problem is that our handler is not passing any info forward. Its taking the request sending it to our software and returning the rendered page. Would the apache modules book cover this or are there any examples of playing nice with the other plugins? Thanks. ~Jeremy
Re: handler pass to php
I'd consider having your handler fire off a subrequest. The book should cover this topic, but you can also look in the source for some examples (or google ap_run_sub_req and review the results). You just want the php handler set for it. Joe On Tue, Oct 14, 2014 at 3:41 PM, Jeremy Thompson jer...@warehousesports.com wrote: Hello all, We have written a handler for apache that talks to our point of sales software. At this point the POS software renders a lot of the html itself and sends it back through apache. We would however, like to be able to integrate PHP to offload some of this work. The problem right now is that once the handler catches the request it renders and sends its through never reaching PHP. This is the same whether or not PHP is built as a fcgi or apache2handler. I'm pretty sure the problem is that our handler is not passing any info forward. Its taking the request sending it to our software and returning the rendered page. Would the apache modules book cover this or are there any examples of playing nice with the other plugins? Thanks. ~Jeremy
Re: svn commit: r1628919 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_substitute.c
Hi, this patch is in the backport proposal for 2.4.x. See my remarks below. The only one that worse it is the one for comparison on new varbuf length either with or with = Best regards, CJ Le 02/10/2014 11:50, rj...@apache.org a écrit : Author: rjung Date: Thu Oct 2 09:50:24 2014 New Revision: 1628919 URL: http://svn.apache.org/r1628919 Log: mod_substitute: Make maximum line length configurable. Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/modules/filters/mod_substitute.c Modified: httpd/httpd/trunk/modules/filters/mod_substitute.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_substitute.c?rev=1628919r1=1628918r2=1628919view=diff == --- httpd/httpd/trunk/modules/filters/mod_substitute.c (original) +++ httpd/httpd/trunk/modules/filters/mod_substitute.c Thu Oct 2 09:50:24 2014 @@ -33,6 +33,13 @@ #define APR_WANT_STRFUNC #include apr_want.h +/* + * We want to limit the memory usage in a way that is predictable. + * Therefore we limit the resulting length of the line. + * This is the default value. + */ +#define AP_SUBST_MAX_LINE_LENGTH (128*MAX_STRING_LEN) Why not use directly 1048576 or (1024*1024) or MBYTE defined below), should MAX_STRING_LEN change one day? #define SEDRMPATBCKT(b, offset, tmp_b, patlen) do { \ apr_bucket_split(b, offset); \ @@ -143,9 +152,9 @@ static apr_status_t do_pattmatch(ap_filt const char *repl; /* * space_left counts how many bytes we have left until the - * line length reaches AP_SUBST_MAX_LINE_LENGTH. + * line length reaches max_line_length. */ -apr_size_t space_left = AP_SUBST_MAX_LINE_LENGTH; +apr_size_t space_left = cfg-max_line_length; apr_size_t repl_len = strlen(script-replacement); while ((repl = apr_strmatch(script-pattern, buff, bytes))) { @@ -161,7 +170,7 @@ static apr_status_t do_pattmatch(ap_filt * are constanting allocing space and copying * strings. */ -if (vb.strlen + len + repl_len AP_SUBST_MAX_LINE_LENGTH) +if (vb.strlen + len + repl_len cfg-max_line_length) why there... return APR_ENOMEM; ap_varbuf_strmemcat(vb, buff, len); ap_varbuf_strmemcat(vb, script-replacement, repl_len); @@ -228,21 +237,21 @@ static apr_status_t do_pattmatch(ap_filt int left = bytes; const char *pos = buff; char *repl; -apr_size_t space_left = AP_SUBST_MAX_LINE_LENGTH; +apr_size_t space_left = cfg-max_line_length; while (!ap_regexec_len(script-regexp, pos, left, AP_MAX_REG_MATCH, regm, 0)) { apr_status_t rv; have_match = 1; if (script-flatten !force_quick) { /* copy bytes before the match */ -if (vb.strlen + regm[0].rm_so = AP_SUBST_MAX_LINE_LENGTH) +if (vb.strlen + regm[0].rm_so = cfg-max_line_length) and = here ? return APR_ENOMEM; if (regm[0].rm_so 0) ap_varbuf_strmemcat(vb, pos, regm[0].rm_so); @@ -629,6 +641,44 @@ static const char *set_pattern(cmd_parms return NULL; } +#define KBYTE 1024 +#define MBYTE 1048576 +#define GBYTE 1073741824 + +static const char *set_max_line_length(cmd_parms *cmd, void *cfg, const char *arg) +{ +subst_dir_conf *dcfg = (subst_dir_conf *)cfg; +apr_off_t max; +char *end; +apr_status_t rv; + +rv = apr_strtoff(max, arg, end, 10); +if (rv == APR_SUCCESS) { +if ((*end == 'K' || *end == 'k') !end[1]) { +max *= KBYTE; +} +else if ((*end == 'M' || *end == 'm') !end[1]) { +max *= MBYTE; +} +else if ((*end == 'G' || *end == 'g') !end[1]) { +max *= GBYTE; +} +else if (*end /* neither empty nor [Bb] */ + ((*end != 'B' *end != 'b') || end[1])) { +rv = APR_EGENERAL; +} +} + +if (rv != APR_SUCCESS || max 0) +{ +return SubstituteMaxLineLength must be a non-negative integer optionally + suffixed with 'k', 'm' or 'g'.; and 'b' ? +} +dcfg-max_line_length = (apr_size_t)max; +dcfg-max_line_length_set = 1; +return NULL; +} +
Re: svn commit: r1627749 - in /httpd/httpd/branches/2.4.x: ./ CHANGES STATUS modules/cache/cache_util.c
Hello Jim and Jan, I am considering a proposal of backporting this fix to the 2.2 branch. At first look, this fix doesn't apply to 2.2 code. But I noticed that the pertinent code has been refactored between 2.2 and 2.4. The same problem exists in 2.2, but just in a different location. In 2.2, the problem is in the store_headers function in modules/cache/mod_disk_cache.c. Are either of you interested in working a patch for this? Otherwise, I will look at it myself in a few days. Thanks, Mike Rumph On 9/26/2014 4:00 AM, j...@apache.org wrote: Author: jim Date: Fri Sep 26 11:00:14 2014 New Revision: 1627749 URL: http://svn.apache.org/r1627749 Log: Merge r1624234 from trunk: SECURITY (CVE-2014-3581): Fix a mod_cache NULL pointer deference in Content-Type handling. mod_cache: Avoid a crash when Content-Type has an empty value. PR56924. Submitted By: Mark Montague mark catseye.org Reviewed By: Jan Kaluza Submitted by: jkaluza Reviewed/backported by: jim Modified: httpd/httpd/branches/2.4.x/ (props changed) httpd/httpd/branches/2.4.x/CHANGES httpd/httpd/branches/2.4.x/STATUS httpd/httpd/branches/2.4.x/modules/cache/cache_util.c Propchange: httpd/httpd/branches/2.4.x/ -- Merged /httpd/httpd/trunk:r1624234 Modified: httpd/httpd/branches/2.4.x/CHANGES URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original) +++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Fri Sep 26 11:00:14 2014 @@ -2,6 +2,10 @@ Changes with Apache 2.4.11 + *) SECURITY: CVE-2014-3581 (cve.mitre.org) + mod_cache: Avoid a crash when Content-Type has an empty value. + PR 56924. [Mark Montague mark catseye.org, Jan Kaluza] + *) mod_cache: Avoid sending 304 responses during failed revalidations PR56881. [Eric Covener] Modified: httpd/httpd/branches/2.4.x/STATUS URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/STATUS (original) +++ httpd/httpd/branches/2.4.x/STATUS Fri Sep 26 11:00:14 2014 @@ -102,11 +102,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mod_cache: CVE-2014-3581 - Avoid a crash when Content-Type has an empty - value. PR56924. - trunk patch: http://svn.apache.org/r1624234 - 2.4.x patch: trunk works (modulo CHANGES) - +1: jkaluza, jim, ylavic PATCHES PROPOSED TO BACKPORT FROM TRUNK: Modified: httpd/httpd/branches/2.4.x/modules/cache/cache_util.c URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/cache/cache_util.c?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/modules/cache/cache_util.c (original) +++ httpd/httpd/branches/2.4.x/modules/cache/cache_util.c Fri Sep 26 11:00:14 2014 @@ -1258,8 +1258,10 @@ apr_table_t *cache_merge_headers_out(req if (r-content_type !apr_table_get(headers_out, Content-Type)) { -apr_table_setn(headers_out, Content-Type, - ap_make_content_type(r, r-content_type)); +const char *ctype = ap_make_content_type(r, r-content_type); +if (ctype) { +apr_table_setn(headers_out, Content-Type, ctype); +} } if (r-content_encoding
Re: svn commit: r1627749 - in /httpd/httpd/branches/2.4.x: ./ CHANGES STATUS modules/cache/cache_util.c
In 2.2 code, this problem is actually in two places. It is also in the store_headers function in modules/cache/mod_mem_cache.c. On 10/14/2014 8:40 AM, Mike Rumph wrote: Hello Jim and Jan, I am considering a proposal of backporting this fix to the 2.2 branch. At first look, this fix doesn't apply to 2.2 code. But I noticed that the pertinent code has been refactored between 2.2 and 2.4. The same problem exists in 2.2, but just in a different location. In 2.2, the problem is in the store_headers function in modules/cache/mod_disk_cache.c. Are either of you interested in working a patch for this? Otherwise, I will look at it myself in a few days. Thanks, Mike Rumph On 9/26/2014 4:00 AM, j...@apache.org wrote: Author: jim Date: Fri Sep 26 11:00:14 2014 New Revision: 1627749 URL: http://svn.apache.org/r1627749 Log: Merge r1624234 from trunk: SECURITY (CVE-2014-3581): Fix a mod_cache NULL pointer deference in Content-Type handling. mod_cache: Avoid a crash when Content-Type has an empty value. PR56924. Submitted By: Mark Montague mark catseye.org Reviewed By: Jan Kaluza Submitted by: jkaluza Reviewed/backported by: jim Modified: httpd/httpd/branches/2.4.x/ (props changed) httpd/httpd/branches/2.4.x/CHANGES httpd/httpd/branches/2.4.x/STATUS httpd/httpd/branches/2.4.x/modules/cache/cache_util.c Propchange: httpd/httpd/branches/2.4.x/ -- Merged /httpd/httpd/trunk:r1624234 Modified: httpd/httpd/branches/2.4.x/CHANGES URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original) +++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Fri Sep 26 11:00:14 2014 @@ -2,6 +2,10 @@ Changes with Apache 2.4.11 + *) SECURITY: CVE-2014-3581 (cve.mitre.org) + mod_cache: Avoid a crash when Content-Type has an empty value. + PR 56924. [Mark Montague mark catseye.org, Jan Kaluza] + *) mod_cache: Avoid sending 304 responses during failed revalidations PR56881. [Eric Covener] Modified: httpd/httpd/branches/2.4.x/STATUS URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/STATUS (original) +++ httpd/httpd/branches/2.4.x/STATUS Fri Sep 26 11:00:14 2014 @@ -102,11 +102,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mod_cache: CVE-2014-3581 - Avoid a crash when Content-Type has an empty - value. PR56924. - trunk patch: http://svn.apache.org/r1624234 - 2.4.x patch: trunk works (modulo CHANGES) - +1: jkaluza, jim, ylavic PATCHES PROPOSED TO BACKPORT FROM TRUNK: Modified: httpd/httpd/branches/2.4.x/modules/cache/cache_util.c URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/cache/cache_util.c?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/modules/cache/cache_util.c (original) +++ httpd/httpd/branches/2.4.x/modules/cache/cache_util.c Fri Sep 26 11:00:14 2014 @@ -1258,8 +1258,10 @@ apr_table_t *cache_merge_headers_out(req if (r-content_type !apr_table_get(headers_out, Content-Type)) { -apr_table_setn(headers_out, Content-Type, - ap_make_content_type(r, r-content_type)); +const char *ctype = ap_make_content_type(r, r-content_type); +if (ctype) { +apr_table_setn(headers_out, Content-Type, ctype); +} } if (r-content_encoding
Re: svn commit: r1627749 - in /httpd/httpd/branches/2.4.x: ./ CHANGES STATUS modules/cache/cache_util.c
I thought at the time, the discussion was that ap_make_content_type in those releases never returned NULL. On Tue, Oct 14, 2014 at 1:01 PM, Mike Rumph mike.ru...@oracle.com wrote: In 2.2 code, this problem is actually in two places. It is also in the store_headers function in modules/cache/mod_mem_cache.c. On 10/14/2014 8:40 AM, Mike Rumph wrote: Hello Jim and Jan, I am considering a proposal of backporting this fix to the 2.2 branch. At first look, this fix doesn't apply to 2.2 code. But I noticed that the pertinent code has been refactored between 2.2 and 2.4. The same problem exists in 2.2, but just in a different location. In 2.2, the problem is in the store_headers function in modules/cache/mod_disk_cache.c. Are either of you interested in working a patch for this? Otherwise, I will look at it myself in a few days. Thanks, Mike Rumph On 9/26/2014 4:00 AM, j...@apache.org wrote: Author: jim Date: Fri Sep 26 11:00:14 2014 New Revision: 1627749 URL: http://svn.apache.org/r1627749 Log: Merge r1624234 from trunk: SECURITY (CVE-2014-3581): Fix a mod_cache NULL pointer deference in Content-Type handling. mod_cache: Avoid a crash when Content-Type has an empty value. PR56924. Submitted By: Mark Montague mark catseye.org Reviewed By: Jan Kaluza Submitted by: jkaluza Reviewed/backported by: jim Modified: httpd/httpd/branches/2.4.x/ (props changed) httpd/httpd/branches/2.4.x/CHANGES httpd/httpd/branches/2.4.x/STATUS httpd/httpd/branches/2.4.x/modules/cache/cache_util.c Propchange: httpd/httpd/branches/2.4.x/ -- Merged /httpd/httpd/trunk:r1624234 Modified: httpd/httpd/branches/2.4.x/CHANGES URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/ CHANGES?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original) +++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Fri Sep 26 11:00:14 2014 @@ -2,6 +2,10 @@ Changes with Apache 2.4.11 + *) SECURITY: CVE-2014-3581 (cve.mitre.org) + mod_cache: Avoid a crash when Content-Type has an empty value. + PR 56924. [Mark Montague mark catseye.org, Jan Kaluza] + *) mod_cache: Avoid sending 304 responses during failed revalidations PR56881. [Eric Covener] Modified: httpd/httpd/branches/2.4.x/STATUS URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/ STATUS?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/STATUS (original) +++ httpd/httpd/branches/2.4.x/STATUS Fri Sep 26 11:00:14 2014 @@ -102,11 +102,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mod_cache: CVE-2014-3581 - Avoid a crash when Content-Type has an empty - value. PR56924. - trunk patch: http://svn.apache.org/r1624234 - 2.4.x patch: trunk works (modulo CHANGES) - +1: jkaluza, jim, ylavic PATCHES PROPOSED TO BACKPORT FROM TRUNK: Modified: httpd/httpd/branches/2.4.x/modules/cache/cache_util.c URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/ modules/cache/cache_util.c?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/modules/cache/cache_util.c (original) +++ httpd/httpd/branches/2.4.x/modules/cache/cache_util.c Fri Sep 26 11:00:14 2014 @@ -1258,8 +1258,10 @@ apr_table_t *cache_merge_headers_out(req if (r-content_type !apr_table_get(headers_out, Content-Type)) { -apr_table_setn(headers_out, Content-Type, - ap_make_content_type(r, r-content_type)); +const char *ctype = ap_make_content_type(r, r-content_type); +if (ctype) { +apr_table_setn(headers_out, Content-Type, ctype); +} } if (r-content_encoding -- Eric Covener cove...@gmail.com
Re: svn commit: r1627749 - in /httpd/httpd/branches/2.4.x: ./ CHANGES STATUS modules/cache/cache_util.c
Hello Eric, Okay. Thanks. I must have missed that discussion. I just now compared ap_make_content_type in both 2.2 and 2.4. It looks like you are correct. Some code to return NULL was added in 2.4. So there is no need to check the return from ap_make_content_type for NULL. Sorry for the noise. Take care, Mike On 10/14/2014 10:03 AM, Eric Covener wrote: I thought at the time, the discussion was that ap_make_content_type in those releases never returned NULL. On Tue, Oct 14, 2014 at 1:01 PM, Mike Rumph mike.ru...@oracle.com mailto:mike.ru...@oracle.com wrote: In 2.2 code, this problem is actually in two places. It is also in the store_headers function in modules/cache/mod_mem_cache.c. On 10/14/2014 8:40 AM, Mike Rumph wrote: Hello Jim and Jan, I am considering a proposal of backporting this fix to the 2.2 branch. At first look, this fix doesn't apply to 2.2 code. But I noticed that the pertinent code has been refactored between 2.2 and 2.4. The same problem exists in 2.2, but just in a different location. In 2.2, the problem is in the store_headers function in modules/cache/mod_disk_cache.c. Are either of you interested in working a patch for this? Otherwise, I will look at it myself in a few days. Thanks, Mike Rumph On 9/26/2014 4:00 AM, j...@apache.org mailto:j...@apache.org wrote: Author: jim Date: Fri Sep 26 11:00:14 2014 New Revision: 1627749 URL: http://svn.apache.org/r1627749 Log: Merge r1624234 from trunk: SECURITY (CVE-2014-3581): Fix a mod_cache NULL pointer deference in Content-Type handling. mod_cache: Avoid a crash when Content-Type has an empty value. PR56924. Submitted By: Mark Montague mark catseye.org http://catseye.org Reviewed By: Jan Kaluza Submitted by: jkaluza Reviewed/backported by: jim Modified: httpd/httpd/branches/2.4.x/ (props changed) httpd/httpd/branches/2.4.x/CHANGES httpd/httpd/branches/2.4.x/STATUS httpd/httpd/branches/2.4.x/modules/cache/cache_util.c Propchange: httpd/httpd/branches/2.4.x/ -- Merged /httpd/httpd/trunk:r1624234 Modified: httpd/httpd/branches/2.4.x/CHANGES URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original) +++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Fri Sep 26 11:00:14 2014 @@ -2,6 +2,10 @@ Changes with Apache 2.4.11 + *) SECURITY: CVE-2014-3581 (cve.mitre.org http://cve.mitre.org) + mod_cache: Avoid a crash when Content-Type has an empty value. + PR 56924. [Mark Montague mark catseye.org http://catseye.org, Jan Kaluza] + *) mod_cache: Avoid sending 304 responses during failed revalidations PR56881. [Eric Covener] Modified: httpd/httpd/branches/2.4.x/STATUS URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/STATUS (original) +++ httpd/httpd/branches/2.4.x/STATUS Fri Sep 26 11:00:14 2014 @@ -102,11 +102,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mod_cache: CVE-2014-3581 - Avoid a crash when Content-Type has an empty - value. PR56924. - trunk patch: http://svn.apache.org/r1624234 - 2.4.x patch: trunk works (modulo CHANGES) - +1: jkaluza, jim, ylavic PATCHES PROPOSED TO BACKPORT FROM TRUNK: Modified: httpd/httpd/branches/2.4.x/modules/cache/cache_util.c URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/cache/cache_util.c?rev=1627749r1=1627748r2=1627749view=diff == --- httpd/httpd/branches/2.4.x/modules/cache/cache_util.c (original) +++ httpd/httpd/branches/2.4.x/modules/cache/cache_util.c
Re: svn commit: r1628919 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_substitute.c
Am 14.10.2014 um 14:22 schrieb Christophe JAILLET: Hi, this patch is in the backport proposal for 2.4.x. See my remarks below. The only one that worse it is the one for comparison on new varbuf length either with or with = Best regards, CJ Le 02/10/2014 11:50, rj...@apache.org a écrit : Author: rjung Date: Thu Oct 2 09:50:24 2014 New Revision: 1628919 URL: http://svn.apache.org/r1628919 Log: mod_substitute: Make maximum line length configurable. Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/modules/filters/mod_substitute.c Modified: httpd/httpd/trunk/modules/filters/mod_substitute.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_substitute.c?rev=1628919r1=1628918r2=1628919view=diff == --- httpd/httpd/trunk/modules/filters/mod_substitute.c (original) +++ httpd/httpd/trunk/modules/filters/mod_substitute.c Thu Oct 2 09:50:24 2014 @@ -33,6 +33,13 @@ #define APR_WANT_STRFUNC #include apr_want.h +/* + * We want to limit the memory usage in a way that is predictable. + * Therefore we limit the resulting length of the line. + * This is the default value. + */ +#define AP_SUBST_MAX_LINE_LENGTH (128*MAX_STRING_LEN) Why not use directly 1048576 or (1024*1024) or MBYTE defined below), should MAX_STRING_LEN change one day? I'm totally fine with either of your proposals. The chosen form was what I found in the code and I didn't want to do an unrelated change. but yes, i also had to first find out what the MAX_STRING_LEN is, befor I knew, what the actual value was. So setting it to some fixed value is clearer. +1 to either 1024*1024 or 1048576. #define SEDRMPATBCKT(b, offset, tmp_b, patlen) do { \ apr_bucket_split(b, offset); \ @@ -143,9 +152,9 @@ static apr_status_t do_pattmatch(ap_filt const char *repl; /* * space_left counts how many bytes we have left until the - * line length reaches AP_SUBST_MAX_LINE_LENGTH. + * line length reaches max_line_length. */ -apr_size_t space_left = AP_SUBST_MAX_LINE_LENGTH; +apr_size_t space_left = cfg-max_line_length; apr_size_t repl_len = strlen(script-replacement); while ((repl = apr_strmatch(script-pattern, buff, bytes))) { @@ -161,7 +170,7 @@ static apr_status_t do_pattmatch(ap_filt * are constanting allocing space and copying * strings. */ -if (vb.strlen + len + repl_len AP_SUBST_MAX_LINE_LENGTH) +if (vb.strlen + len + repl_len cfg-max_line_length) why there... return APR_ENOMEM; ap_varbuf_strmemcat(vb, buff, len); ap_varbuf_strmemcat(vb, script-replacement, repl_len); @@ -228,21 +237,21 @@ static apr_status_t do_pattmatch(ap_filt int left = bytes; const char *pos = buff; char *repl; -apr_size_t space_left = AP_SUBST_MAX_LINE_LENGTH; +apr_size_t space_left = cfg-max_line_length; while (!ap_regexec_len(script-regexp, pos, left, AP_MAX_REG_MATCH, regm, 0)) { apr_status_t rv; have_match = 1; if (script-flatten !force_quick) { /* copy bytes before the match */ -if (vb.strlen + regm[0].rm_so = AP_SUBST_MAX_LINE_LENGTH) +if (vb.strlen + regm[0].rm_so = cfg-max_line_length) and = here ? That block is followed by first copying regm[0].rm_so to the end of vb and then rv = ap_varbuf_regsub(vb, script-replacement, pos, AP_MAX_REG_MATCH, regm, cfg-max_line_length - vb.strlen); If we start with vb.strlen + regm[0].rm_so == cfg-max_line_length, then after appending regm[0].rm_so we have vb.strlen == cfg-max_line_length and the last param in ap_varbuf_regsub() gets 0. But a 0 there does not mean at most 0 bytes, but instead unlimited number of bytes :( So yes, we could change the condition to a , but we would then need to skip over the ap_varbuf_regsub() call below. I think we can keep as is but I should add a comment about that special case. OK? return APR_ENOMEM; if (regm[0].rm_so 0) ap_varbuf_strmemcat(vb, pos, regm[0].rm_so); @@ -629,6 +641,44 @@ static const char *set_pattern(cmd_parms return NULL; } +#define KBYTE 1024 +#define MBYTE 1048576 +#define GBYTE
Re: svn commit: r1628919 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_substitute.c
Le 14/10/2014 19:55, Rainer Jung a écrit : Am 14.10.2014 um 14:22 schrieb Christophe JAILLET: Hi, this patch is in the backport proposal for 2.4.x. See my remarks below. The only one that worse it is the one for comparison on new varbuf length either with or with = Best regards, CJ Le 02/10/2014 11:50, rj...@apache.org a écrit : Author: rjung Date: Thu Oct 2 09:50:24 2014 New Revision: 1628919 URL: http://svn.apache.org/r1628919 Log: mod_substitute: Make maximum line length configurable. Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/modules/filters/mod_substitute.c Modified: httpd/httpd/trunk/modules/filters/mod_substitute.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_substitute.c?rev=1628919r1=1628918r2=1628919view=diff == --- httpd/httpd/trunk/modules/filters/mod_substitute.c (original) +++ httpd/httpd/trunk/modules/filters/mod_substitute.c Thu Oct 2 09:50:24 2014 @@ -33,6 +33,13 @@ #define APR_WANT_STRFUNC #include apr_want.h +/* + * We want to limit the memory usage in a way that is predictable. + * Therefore we limit the resulting length of the line. + * This is the default value. + */ +#define AP_SUBST_MAX_LINE_LENGTH (128*MAX_STRING_LEN) Why not use directly 1048576 or (1024*1024) or MBYTE defined below), should MAX_STRING_LEN change one day? I'm totally fine with either of your proposals. The chosen form was what I found in the code and I didn't want to do an unrelated change. but yes, i also had to first find out what the MAX_STRING_LEN is, befor I knew, what the actual value was. So setting it to some fixed value is clearer. +1 to either 1024*1024 or 1048576. Up to you. When I looked at it, I grep'ed source code and the first match was: ./srclib/apr/passwd/apr_getpass.c:80:#define MAX_STRING_LEN 256 Only looking at the #define (and not at the file!) I first thought that doc was not in lone with code. later on, I found the correct one in httpd.h :) That block is followed by first copying regm[0].rm_so to the end of vb and then rv = ap_varbuf_regsub(vb, script-replacement, pos, AP_MAX_REG_MATCH, regm, cfg-max_line_length - vb.strlen); If we start with vb.strlen + regm[0].rm_so == cfg-max_line_length, then after appending regm[0].rm_so we have vb.strlen == cfg-max_line_length and the last param in ap_varbuf_regsub() gets 0. But a 0 there does not mean at most 0 bytes, but instead unlimited number of bytes :( So yes, we could change the condition to a , but we would then need to skip over the ap_varbuf_regsub() call below. I think we can keep as is but I should add a comment about that special case. OK? OK, understood. +1 for a comment if someone, one day, notices it and tries understand if it is a mistake or not. +if (rv != APR_SUCCESS || max 0) +{ +return SubstituteMaxLineLength must be a non-negative integer optionally + suffixed with 'k', 'm' or 'g'.; and 'b' ? You are right, I probably added the 'b'later while working on it and forgot to update the description text. Was just a minor issue.