handler pass to php

2014-10-14 Thread Jeremy Thompson
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

2014-10-14 Thread Joe Lewis
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

2014-10-14 Thread 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?


  
  #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

2014-10-14 Thread Mike Rumph

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

2014-10-14 Thread Mike Rumph

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

2014-10-14 Thread Eric Covener
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

2014-10-14 Thread Mike Rumph

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

2014-10-14 Thread Rainer Jung

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

2014-10-14 Thread Marion Christophe JAILLET


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.