Re: svn commit: r1588244 - in /httpd/httpd/branches/2.4.x: ./ CHANGES STATUS docs/manual/mod/mod_headers.xml modules/metadata/mod_headers.c

2014-04-17 Thread Marion Christophe JAILLET

Hi,

Changelog entry is about Header and RequestHeader but doc has only been 
updated for the first one.

Moreover, a compatibility note should be, IMO, added for the updated syntax.

CJ


Le 17/04/2014 15:36, j...@apache.org a écrit :
  
  Changes with Apache 2.4.10
  
+  *) mod_headers: Allow the value parameter of Header and RequestHeader to

+ contain an ap_expr expression if prefixed with expr=. [Eric Covener]
+
*) rotatelogs: Avoid creation of zombie processes when -p is used on
   Unix platforms.  [Joe Orton]
  


Modified: httpd/httpd/branches/2.4.x/docs/manual/mod/mod_headers.xml
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/docs/manual/mod/mod_headers.xml?rev=1588244r1=1588243r2=1588244view=diff
==
--- httpd/httpd/branches/2.4.x/docs/manual/mod/mod_headers.xml (original)
+++ httpd/httpd/branches/2.4.x/docs/manual/mod/mod_headers.xml Thu Apr 17 
13:36:05 2014
@@ -310,7 +310,7 @@ Header merge Cache-Control no-store env=
  nameHeader/name
  descriptionConfigure HTTP response headers/description
  syntaxHeader [varcondition/var] 
add|append|echo|edit|edit*|merge|set|setifempty|unset|note
-varheader/var [varvalue/var] [varreplacement/var]
+varheader/var [var[expr=]value]/var] [varreplacement/var]
  [early|env=[!]varvariable/var]|expr=varexpression/var]
  /syntax
  contextlistcontextserver config/contextcontextvirtual host/context
@@ -437,9 +437,12 @@ SetIfEmpty and note available in 2.4.7 a
  codeadd/code a varvalue/var is specified as the next argument.
  If varvalue/var
  contains spaces, it should be surrounded by double quotes.
-varvalue/var may be a character string, a string containing format
-specifiers or a combination of both. The following format specifiers
-are supported in varvalue/var:/p
+varvalue/var may be a character string, a string containing
+modulemod_headers/module specific format specifiers (and character
+literals), or an a href=../expr.htmlap_expr/a expression prefixed
+with emexpr=/em/p
+
+p The following format specifiers are supported in varvalue/var:/p
  
  table border=1 style=zebra

  columnspeccolumn width=.25/column width=.75//columnspec






Re: svn commit: r1584896 - /httpd/httpd/trunk/modules/filters/mod_proxy_html.c

2014-04-17 Thread Marion Christophe JAILLET

r1588356

Should you share my analysis and should a CHANGE be useful for what I 
think is a corner case, feel free to add something, or I can do it by 
the end of the week.




Does this fix a crash or a parsing error or ...?  (CHANGES)





Re: svn commit: r1584896 - /httpd/httpd/trunk/modules/filters/mod_proxy_html.c

2014-04-15 Thread Marion Christophe JAILLET

Hi,

AFAIK, no crash has ever been reported for that.
I just noted this while looking at PR56287 and found it odd.

A file such as:
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
htmlhead
meta http-equiv=Conten contentheadbody/body/html
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-

will trigger the scan past the end of the buffer behavior.

line 658: ap_regexec(meta[^]*(http-equiv)[^]*) OK
line 665: strncasecmp(header, Content-  OK, string not 
found

line 667: apr_strmatch(content  OK
then looping with +7 every time until reaching a \0 because no '=' is in 
the buffer


In the example above, we go past then end of 'buf' because of the 
unconditional +7 and no check that we are about to scan past the end of 
'buf'


So, the ocde with a 'continue' can:
- eat time until we find a \0 somewhere
- do a 'content = apr_pstrndup' with unexpected data if we finally 
reach a = (however, the strdup will not copy data past the end of buf)

- crash if we try to read memory that does not belong to the process ?

So I imagine that someone who can write a file on the server behind the 
proxy could set up what looks like a DoS, expecting to crash a process 
from time to time.

All this is true if 'ProxyHTMLMeta' is on, which is not the default.


This is all theoretical and with my tests, I never managed to crash my 
server.
Anyway, the logic that was in place was broken. My change is not perfect 
(we should try to find another 'content', if any, to keep the spirit of 
the code), but is, IMO, safer because avoid a possible crash.



Should you share my analysis and should a CHANGE be useful for what I 
think is a corner case, feel free to add something, or I can do it by 
the end of the week.



Best regards,
CJ




Le 15/04/2014 21:16, Jeff Trawick a écrit :
On Fri, Apr 4, 2014 at 4:30 PM, jaillet...@apache.org 
mailto:jaillet...@apache.org wrote:


Author: jailletc36
Date: Fri Apr  4 20:30:38 2014
New Revision: 1584896

URL: http://svn.apache.org/r1584896
Log:
Do not perform a p+= 7 that could go past the end of the buffer in
case we find a 'content' without a corresponding '='.


Does this fix a crash or a parsing error or ...?  (CHANGES)


Should we need to deal with this case, a new search should be
performed to find the real starting position of another potential
'content=' pattern.

Modified:
httpd/httpd/trunk/modules/filters/mod_proxy_html.c

Modified: httpd/httpd/trunk/modules/filters/mod_proxy_html.c
URL:

http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_proxy_html.c?rev=1584896r1=1584895r2=1584896view=diff

==
--- httpd/httpd/trunk/modules/filters/mod_proxy_html.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_proxy_html.c Fri Apr  4
20:30:38 2014
@@ -672,8 +672,9 @@ static meta *metafix(request_rec *r, con
 p += 7;
 while (apr_isspace(*p))
 ++p;
+/* XXX Should we search for another content=
pattern? */
 if (*p != '=')
-continue;
+break;
 while (*p  apr_isspace(*++p));
 if ((*p == '\'') || (*p == '')) {
 delim = *p++;





--
Born in Roswell... married an alien...
http://emptyhammock.com/
http://edjective.org/





Re: svn commit: r1584941 - /httpd/httpd/branches/2.4.x/STATUS

2014-04-05 Thread Marion Christophe JAILLET

Hi,

I've gone quickly thru the module and I have a few remarks:
- What is the use of FN_LOG_MARK on line 87? Couldn't we use 
APLOG_MARK instead?
  'connect_to_peer' is called only in one place with 
module_index=APLOG_MODULE_INDEX


   - Do things like AP_MODULE_MAGIC_AT_LEAST(20130702,2) on line 186 
and 228 are meaningful?
 I mean, is there a link between the value of 
MODULE_MAGIC_NUMBER_MAJOR on trunk and on stable branch?

 Should patches related to 'ap_log_rdata' be merged first?

   - Maybe temp_pool in 'req_rsp' could be kept till the end of the 
function and table 'vars' (line 784) allocated from there ?


CJ


Le 05/04/2014 01:44, traw...@apache.org a écrit :

Author: trawick
Date: Fri Apr  4 23:44:32 2014
New Revision: 1584941

URL: http://svn.apache.org/r1584941
Log:
mod_authnz_fcgi...

Modified:
 httpd/httpd/branches/2.4.x/STATUS

Modified: httpd/httpd/branches/2.4.x/STATUS
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1584941r1=1584940r2=1584941view=diff
==
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Fri Apr  4 23:44:32 2014
@@ -239,6 +239,11 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   2.4.x patch: trunk works (modulo CHANGES)
   +1: ylavic
  
+   * Merge mod_authnz_fcgi from trunk.

+ 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authnz_fcgi.c
+ 
http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_authnz_fcgi.xml
+ +1: trawick
+
  OTHER PROPOSALS
  
 * A list of further possible backports can be found at:








Re: svn commit: r1584582 - /httpd/httpd/branches/2.4.x/STATUS

2014-04-04 Thread Marion Christophe JAILLET


Le 04/04/2014 13:59, Yann Ylavic a écrit :

On Fri, Apr 4, 2014 at 1:35 PM, Eric Covener cove...@gmail.com wrote:

FYI not necessary to propose docs-only changes in STATUS, they are CTR.

Oh, I see, thanks for the information.
Should I (or one) backport it if no one else screams for a while then?



Just in case, removal of compatibility notes against 2.3.x has been 
discussed a few months ago.

See http://marc.info/?t=13861912831r=1w=2

No real concensus about it.
I'm still +1 for removing these references.


I'm not sure that the compatibility notes are really consistent in the 
current tree. I am quite sure that some configuration options have been 
added or enhanced without stating in which version it happened.
I have in my TODO list to check, for each 2.4.x releases, which options 
have been added/modified and if the corresponding compatibility notes 
have been added in the doc. I've not taken the time yet to go thru all that.


The only example I have in mind right now is r1523242 where the 
'change=no' parameter has been added to 2.4.7.

Doc has been updated in r1523325.


Best regards,
CJ


Re: AW: APR_FOPEN_BUFFERED and small files

2014-02-14 Thread Marion Christophe JAILLET

lol, sure


Le 14/02/2014 21:44, Plüm, Rüdiger, Vodafone Group a écrit :



-Ursprüngliche Nachricht-
Von: Christophe JAILLET  Gesendet: Freitag, 14. Februar 2014 21:39
An: dev@httpd.apache.org
Betreff: APR_FOPEN_BUFFERED and small files

Hi,

when a file is opened using apr_file_open with the flag
APR_FOPEN_BUFFERED, a 4096 bytes buffer is allocated in the pool.
4096 is the half of a pool block, so it often leads to the allocation
of a new 8k block.

When opening .htaccess files, the APR_FOPEN_BUFFERED flag is set but in
most cases, I think that this file is much smaller than 4096 bytes.
This lead to potentially allocating much more memory than useful in the
request pool.


Do you think it would be interesting to teach apr_file_open to allocate
max(size of the file, 4096) when opening small files with

You mean min(size of the file, 4096) ?


APR_FOPEN_BUFFERED set ?
I expect .htaccess file to be a few hundreds of byte. So this would save
~ 3 ko in the request pool.

CJ


Regards

Rüdiger





Re: [VOTE] obscuring (or not) commit logs/CHANGES for fixes to vulnerabilities

2014-01-10 Thread Marion Christophe JAILLET


Le 10/01/2014 14:38, Jeff Trawick a écrit :
[ ] It is an accepted practice (but not required) to obscure or omit 
the vulnerability impact in CHANGES or commit log information when 
committing fixes for vulnerabilities to any branch.


[X] It is mandatory to provide best available description and any 
available tracking information when committing fixes for 
vulnerabilities to any branch, delaying committing of the fix if the 
information shouldn't be provided yet.


[ ] ___ (fill in the blank)

---/---



Could be also interesting to be able to deliver quick fix.

For example, 2.4.7 is the latest stable version. 2.4.8 has things 
back-ported from trunk little by little and should be TR one day (in 
feb ?).


Should an important vulnerability be found, then releasing:
   - a 2.4.7.1or
   - 2.4.7 SP1or
   - 2.4.8 and delaying everything already accepted in backport for a 
later 2.4.9or

   - whatever else
with *only fixes* for this issue, could be interesting.

Doing so would avoid time for TR and avoid releasing something in a hurry.

Best regards,
CJ


Re: svn commit: r1556914 - /httpd/httpd/trunk/modules/dav/lock/locks.c

2014-01-09 Thread Marion Christophe JAILLET

Sure,

but I personally prefer to keep only one exit point in functions.
Just a matter of taste.

CJ


Le 09/01/2014 20:17, Rainer Jung a écrit :

On 09.01.2014 19:48, jaillet...@apache.org wrote:

Author: jailletc36
Date: Thu Jan  9 18:48:11 2014
New Revision: 1556914

URL: http://svn.apache.org/r1556914
Log:
Add missing break in 'dav_generic_do_refresh' to avoid useless computation.

Modified:
 httpd/httpd/trunk/modules/dav/lock/locks.c

Modified: httpd/httpd/trunk/modules/dav/lock/locks.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/lock/locks.c?rev=1556914r1=1556913r2=1556914view=diff
==
--- httpd/httpd/trunk/modules/dav/lock/locks.c (original)
+++ httpd/httpd/trunk/modules/dav/lock/locks.c Thu Jan  9 18:48:11 2014
@@ -1093,6 +1093,7 @@ static int dav_generic_do_refresh(dav_lo
  {
  dp-f.timeout = new_time;
  dirty = 1;
+break;
  }
  }

Here and in r1556911: you could also drop the variable dirty, return 1
instead of break and return 0 after the loop.

Regards,

Rainer






Re: Some redundant code and comment typos in mod_remoteip

2013-12-13 Thread Marion Christophe JAILLET

Not correct,

this is just a french man who didn't take time to check in a 
dictionary...  :)


I update...
Thx

CJ


Le 13/12/2013 19:57, Mike Rumph a écrit :

equivalant versus equivalent
Perhaps this is a difference in British versus American spelling, 
correct?


Anyway, thanks for the commits.

Mike Rumph

On 12/12/2013 10:12 PM, Christophe JAILLET wrote:

Trunk
=
r1550650 for comments upodate
r1550651 for redundant check


2.4.x
=
r1550652 for comments upodate
The other one will be proposed for backport with other easy patches 
to synch 2.4 and trunk in the coming days.



BTW, for someone who has write access to APR tree, 
s/equivilant/equivalant/ in incluce/arch/netware/apr_private.h


Thx,
CJ









Re: svn commit: r1490290 - /httpd/httpd/trunk/modules/lua/lua_passwd.c

2013-06-06 Thread Marion Christophe JAILLET

Correct.

This was introduced in r1465115.
Fixed in trunk in r1490507.


Le 06/06/2013 17:54, Guenter Knauf a écrit :

ua/lua_passwd.c T




Re: svn commit: r1476694 - in /httpd/httpd/branches/2.4.x: CHANGES STATUS docs/manual/mod/mod_authnz_ldap.xml include/ap_mmn.h include/httpd.h modules/aaa/mod_authnz_ldap.c server/util.c

2013-04-27 Thread Marion Christophe JAILLET


Le 28/04/2013 01:14, minf...@apache.org a écrit :

Author: minfrin
Date: Sat Apr 27 23:14:11 2013
New Revision: 1476694

URL: http://svn.apache.org/r1476694
Log:
mod_authnz_ldap: Allow using exec: callouts like SSLPassphraseDialog
for AuthLDAPBindPassword.

trunk patch: http://svn.apache.org/viewvc?view=revisionrevision=1433478
  http://svn.apache.org/viewvc?view=revisionrevision=1467523
  http://svn.apache.org/viewvc?view=revisionrevision=1467792
2.4.x patch: 
http://people.apache.org/~druggeri/patches/AuthLDAPBindPasswordExec-2.4.patch
  (20130119 - updated to include minor mmn bump)
  (20130412 - updated to not use static var - thx, wrowe)



Modified: httpd/httpd/branches/2.4.x/include/ap_mmn.h
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/ap_mmn.h?rev=1476694r1=1476693r2=1476694view=diff
==
--- httpd/httpd/branches/2.4.x/include/ap_mmn.h (original)
+++ httpd/httpd/branches/2.4.x/include/ap_mmn.h Sat Apr 27 23:14:11 2013
@@ -401,7 +401,8 @@
   * 20120211.9 (2.4.4-dev)  Add fgrab() to ap_slotmem_provider_t.
   * 20120211.10 (2.4.4-dev) Add in bal_persist field to proxy_server_conf
   * 20120211.11 (2.4.4-dev) Add ap_bin2hex()
- * 20120211.12 (2.4.5-dev)  Add ap_remove_input|output_filter_byhandle()
+ * 20120211.12 (2.4.4-dev)  Add ap_remove_input|output_filter_byhandle()
+ * 20120211.13 (2.4.4-dev)  Add ap_get_exec_line
   */


Shouldn't we keep 2.4.5-dev here ?

CJ


Re: svn commit: r1463045 - /httpd/httpd/trunk/modules/aaa/mod_auth_digest.c

2013-03-31 Thread Marion Christophe JAILLET

Hi,

they are 3 similar constructions in server/log.c

CJ


Le 31/03/2013 22:13, s...@apache.org a écrit :

Author: sf
Date: Sun Mar 31 20:13:48 2013
New Revision: 1463045

URL: http://svn.apache.org/r1463045
Log:
ap_log_error already logs the error string, no need to log it twice

Modified:
 httpd/httpd/trunk/modules/aaa/mod_auth_digest.c

Modified: httpd/httpd/trunk/modules/aaa/mod_auth_digest.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_auth_digest.c?rev=1463045r1=1463044r2=1463045view=diff
==
--- httpd/httpd/trunk/modules/aaa/mod_auth_digest.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_auth_digest.c Sun Mar 31 20:13:48 2013
@@ -240,10 +240,8 @@ static apr_status_t initialize_secret(se
  #endif
  
  if (status != APR_SUCCESS) {

-char buf[120];
  ap_log_error(APLOG_MARK, APLOG_CRIT, status, s, APLOGNO(01758)
- error generating secret: %s,
- apr_strerror(status, buf, sizeof(buf)));
+ error generating secret);
  return status;
  }
  








Re: svn commit: r1463049 - /httpd/httpd/trunk/modules/aaa/mod_auth_digest.c

2013-03-31 Thread Marion Christophe JAILLET

Hi,

doc also has to be clean the same way.

CJ

Le 31/03/2013 22:38, s...@apache.org a écrit :

Author: sf
Date: Sun Mar 31 20:38:17 2013
New Revision: 1463049

URL: http://svn.apache.org/r1463049
Log:
Remove partial non-working implementation of MD5-sess and qop=auth-int.

If anyone wants to finish the code, it can be retrieved from svn history.
Remove some obsolete references to the truerand library.

Modified:
 httpd/httpd/trunk/modules/aaa/mod_auth_digest.c


Re: svn commit: r1451478 - /httpd/httpd/trunk/server/util_script.c

2013-03-22 Thread Marion Christophe JAILLET

Yes, i work and test on trunk.

I'll give it a try.

Thx

CJ
Le 22/03/2013 22:14, Stefan Fritsch a écrit :

On Tuesday 19 March 2013, Marion  Christophe JAILLET wrote:

Le 18/03/2013 22:43, Stefan Fritsch a écrit :

On Thursday 14 March 2013, you wrote:

BTW, I tried to activate pool debug with using

|-enable-pool-debug=all  but the server crashes while starting
|on

my test machine.
Do you know if it is supposed to work (and I do something wrong)
or no  one uses it with httpd ?

I am sure that I have used at least parts of the pool debugging
with httpd in the past. I will try it again when I have some
time.

Have you tried using prefork? IIRC, there were some threading
issues that were caught by full pool debugging.

Cheers,
Stefan

I tried only with event and worker.
in both cases, I tried to avoid multithreading issue by setting:
  ThreadsPerChild 1
  MaxRequestWorkers 1

I'll do more testing this evening.

Were you using trunk? If yes, maybe this helps:

http://svn.apache.org/r1459992





Re: svn commit: r1451478 - /httpd/httpd/trunk/server/util_script.c

2013-03-19 Thread Marion Christophe JAILLET


Le 18/03/2013 22:43, Stefan Fritsch a écrit :

On Thursday 14 March 2013, you wrote:

BTW, I tried to activate pool debug with using
|-enable-pool-debug=all  but the server crashes while starting on
my test machine.
Do you know if it is supposed to work (and I do something wrong) or
no  one uses it with httpd ?

I am sure that I have used at least parts of the pool debugging with
httpd in the past. I will try it again when I have some time.

Have you tried using prefork? IIRC, there were some threading issues
that were caught by full pool debugging.

Cheers,
Stefan


I tried only with event and worker.
in both cases, I tried to avoid multithreading issue by setting:
ThreadsPerChild 1
MaxRequestWorkers 1

I'll do more testing this evening.

Best regards,
CJ


Re: svn commit: r1451478 - /httpd/httpd/trunk/server/util_script.c

2013-03-13 Thread Marion Christophe JAILLET
My goal was to check for useless memory allocation when calling logging 
function.
Logging with TRACE is unlikely to output something on a production 
machine. However, function called as parameters of the logging function 
will still be called.


I made a check on the whole source code to check for useless memory 
allocation as a side effect of logging.

I found the one below, in an error path.


It is part of the time I'm spending to analyze memory allocation and use 
done by httpd.
I've modified apr_palloc and so on to give me some feedback and I'm 
looking at it.
With current trunk, with a light configuration and a server configured 
to be single threaded, 11541 calls to apr_palloc, for a total of 4,4 Mo, 
are performed during stat-up. According to my configuration, I find it 
high, but ok, why not, it is just start-up
For processing a single request like http://localhosr/foo, 254 new calls 
are done for a total of 15 ko, mostly in the request pool.
Reducing it to fit in only one 8k, if possible, would be nice. It would 
avoid the pool to allocate more memory.

Here is my goal.


BTW, I tried to activate pool debug with using |-enable-pool-debug=all 
but the server crashes while starting on my test machine.
Do you know if it is supposed to work (and I do something wrong) or no 
one uses it with httpd ?


I haven't saved details about it but it would be easy to reproduce if 
you are interested.



|CJ



Le 13/03/2013 22:26, Stefan Fritsch a écrit :
Note that there is some macro magic in http_log.h that does this 
automatically on C99 compilers. There is nothing wrong with doing the 
check explicitly, and it is definitely a good idea if the saved 
function call is very expensive. But in general other improvements may 
have more impact and therefore be a better use of your time. But of 
course that's your choice ;)


On Fri, 1 Mar 2013, jaillet...@apache.org wrote:


Author: jailletc36
Date: Fri Mar  1 06:33:40 2013
New Revision: 1451478

URL: http://svn.apache.org/r1451478
Log:
Avoid some memory allocation on error path in 'http2env' if TRACE1 
logging is not activated

Avoid a function ca

Modified:
   httpd/httpd/trunk/server/util_script.c

Modified: httpd/httpd/trunk/server/util_script.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_script.c?rev=1451478r1=1451477r2=1451478view=diff
== 


--- httpd/httpd/trunk/server/util_script.c (original)
+++ httpd/httpd/trunk/server/util_script.c Fri Mar  1 06:33:40 2013
@@ -73,9 +73,10 @@ static char *http2env(request_rec *r, co
*cp++ = '_';
}
else {
-ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
-  Not exporting header with invalid name as 
envvar: %s,

-  ap_escape_logitem(r-pool, w));
+if (APLOGrtrace1(r))
+ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
+Not exporting header with invalid name 
as envvar: %s,

+ap_escape_logitem(r-pool, w));
return NULL;
}
}






Re: svn commit: r1455225 - in /httpd/httpd/branches/2.4.x: ./ docs/manual/ docs/manual/howto/ docs/manual/mod/ include/ modules/filters/ modules/generators/ modules/slotmem/ os/unix/ server/ support/

2013-03-11 Thread Marion Christophe JAILLET


Le 11/03/2013 20:32, Gregg Smith a écrit :

On 3/11/2013 9:38 AM, j...@apache.org wrote:

Author: jim
Date: Mon Mar 11 16:38:39 2013
New Revision: 1455225

URL: http://svn.apache.org/r1455225
Log:
Merge r1442865, r1442759, r1442326, r1442309, r1448171, r1418556, 
r1448453, r1425771, r1425772, r1425775 from trunk:


...

Add some __attribute__ for automatic format checking.
Correct one catch in sed0.c.

Modified: httpd/httpd/branches/2.4.x/include/util_filter.h
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/util_filter.h?rev=1455225r1=1455224r2=1455225view=diff
== 


--- httpd/httpd/branches/2.4.x/include/util_filter.h (original)
+++ httpd/httpd/branches/2.4.x/include/util_filter.h Mon Mar 11 
16:38:39 2013

@@ -332,8 +332,8 @@ AP_DECLARE(apr_status_t) ap_pass_brigade
  AP_DECLARE(apr_status_t) ap_pass_brigade_fchk(request_rec *r,
apr_bucket_brigade *bucket,
const char *fmt,
-  ...);
-
+  ...)
+ __attribute__((format(printf,3,4)));

  /**
   * This function is used to register an input filter with the system.

Modified: httpd/httpd/branches/2.4.x/modules/filters/regexp.h
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/filters/regexp.h?rev=1455225r1=1455224r2=1455225view=diff
== 


--- httpd/httpd/branches/2.4.x/modules/filters/regexp.h (original)
+++ httpd/httpd/branches/2.4.x/modules/filters/regexp.h Mon Mar 11 
16:38:39 2013

@@ -69,7 +69,8 @@ typedef struct _sed_comp_args {

  extern char *sed_compile(sed_commands_t *commands, sed_comp_args 
*compargs,

   char *ep, char *endbuf, int seof);
-extern void command_errf(sed_commands_t *commands, const char *fmt, 
...);
+extern void command_errf(sed_commands_t *commands, const char *fmt, 
...)

+ __attribute__((format(printf,2,3)));

  #define SEDERR_CGMES command garbled: %s
  #define SEDERR_SMMES Space missing before filename: %s

Modified: httpd/httpd/branches/2.4.x/modules/filters/sed0.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/filters/sed0.c?rev=1455225r1=1455224r2=1455225view=diff
== 


--- httpd/httpd/branches/2.4.x/modules/filters/sed0.c (original)
+++ httpd/httpd/branches/2.4.x/modules/filters/sed0.c Mon Mar 11 
16:38:39 2013

@@ -275,7 +275,7 @@ comploop:
  }

  if(p  commands-respace[RESIZE-1]) {
-command_errf(commands, SEDERR_TMMES);
+command_errf(commands, SEDERR_TMMES, commands-linebuf);
  return -1;
  }



 AFIAK, __attribute__ is gcc specific. What about non-gcc compilers? 
What's might be a consequence of a compiler ignoring it (as MSVC 
does), or will it break any other non-gcc compilers?


Gregg



I proposed it because there was already some __attribute__ elsewhere in 
the source code.
It also helped me trigger some missing parameters, so I thought it would 
be useful to keep and share it.


Should there be an issue with other compiler, IMO we would already be 
aware of it.
It should also be possible to define a macro that expand to nothing if 
the compiler is not gcc.


CJ


Re: svn commit: r1451478 - /httpd/httpd/trunk/server/util_script.c

2013-03-03 Thread Marion Christophe JAILLET

Le 01/03/2013 11:43, Guenter Knauf a écrit :

Hi Christophe,
Am 01.03.2013 08:00, schrieb Christophe JAILLET:

To quick...

you can fix the svn log with:
svn propedit -r 1451478 --revprop svn:log

Gün.



Thanks, done.

CJ



Question about memory allocation in 'substring_conf' (server/util.c)

2013-02-05 Thread Marion Christophe JAILLET

Hi,

This may look useless, but I can't figure out why in function 
'substring_conf' (line 752 in server/util.c) we allocate len+2 bytes ?

This seems to have been like that forever.

IMO, len+1 should be enough.
Changing that would be a huge memory usage improvement :).


Does any one has an idea about it ?

Best regards,
CJ








Re: svn commit: r1422549 - in /httpd/httpd/trunk: include/ap_mmn.h include/httpd.h server/util.c server/util_md5.c

2012-12-17 Thread Marion Christophe JAILLET


Le 17/12/2012 00:20, Stefan Fritsch a écrit :

+AP_DECLARE(void) ap_bin2hex(const void *src, apr_size_t srclen,
char *dest) +{
+const unsigned char *in = src;
+unsigned char *out = (unsigned char *)dest;
+apr_size_t i;
+
+for (i = 0; i  srclen; i++) {
+*out++ = c2x_table[in[i]  4];
+*out++ = c2x_table[in[i]  0xf];
+}
+*out = '\0';
+}
+

The following functions:
SSL_SESSION_id2sz, in ssl_util_ssl.c, line 487
socache_mc_id2key, in mod_socache_memcache.c, line 193
log_xlate_error, in mod_charset_lite.c, line 498
add_password, in htdigest.c, line 157
could also benefit from it.

However, some use uppercase HEX representation.

add_password is not an issue.
socache_mc_id2key should not be an issue, the generated string is only 
used with apr_memcache_[set|getp|delete].
log_xlate_error should not be an issue, the generated string is only 
used for logging.

*But* SSL_SESSION_id2sz could be an issue.

Do you think it could be a win ?
Do you think it would be useful to pass an extra parameter to ap_bin2hex 
in order to tell if the conversion should be done using abcdef or ABCDEF ?


Thanks for your comment.

CJ


Re: svn commit: r1420377 - in /httpd/httpd/trunk: docs/manual/mod/mod_lua.xml modules/lua/lua_apr.c modules/lua/lua_apr.h modules/lua/mod_lua.c

2012-12-12 Thread Marion Christophe JAILLET

Here are a few things triggered by cppcheck.


Le 11/12/2012 21:08, humbed...@apache.org a écrit :

Author: humbedooh
Date: Tue Dec 11 20:08:24 2012
New Revision: 1420377

URL: http://svn.apache.org/viewvc?rev=1420377view=rev
Log:
mod_lua: Add a lot of core httpd/apr functionality to mod_lua
(such as regex matching, expr evaluation, changing/fetching server 
configuration/info - see docs for a complete list).
This also includes a bunch of automatically scraped functions, which may or may 
not be super useful.
Comments appreciated as always, especially on the more hacky bits.


Modified: httpd/httpd/trunk/modules/lua/lua_apr.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/lua/lua_apr.c?rev=1420377r1=1420376r2=1420377view=diff
==
--- httpd/httpd/trunk/modules/lua/lua_apr.c (original)
+++ httpd/httpd/trunk/modules/lua/lua_apr.c Tue Dec 11 20:08:24 2012


I've put the parsed file on:
http://people.apache.org/~jailletc36/lua_apr.html
Check it from there. Things noticed by cppcheck are red-marked.

Except the 'The scope of the variable '...' can be reduced' that can be 
ignored, all the other remarks are relevant.


Especially:
 a useless apr_pcalloc (line 249)
 a out of bound access (line 321) -- I don't know if Rsha1 should 
be [20] or if the loop should be limited to  16
  this seems to be a cut and paste 
error from lua_apr_md5

 a uninitialized variable (line 430)


CJ


Re: svn commit: r1419755 - /httpd/httpd/trunk/modules/http/http_filters.c

2012-12-10 Thread Marion Christophe JAILLET


Le 10/12/2012 21:53, jaillet...@apache.org a écrit :

Author: jailletc36
Date: Mon Dec 10 20:53:24 2012
New Revision: 1419755

URL: http://svn.apache.org/viewvc?rev=1419755view=rev
Log:
Avoid unnecessary %s substitution

Modified:
 httpd/httpd/trunk/modules/http/http_filters.c

Modified: httpd/httpd/trunk/modules/http/http_filters.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=1419755r1=1419754r2=1419755view=diff
==
--- httpd/httpd/trunk/modules/http/http_filters.c (original)
+++ httpd/httpd/trunk/modules/http/http_filters.c Mon Dec 10 20:53:24 2012
@@ -966,10 +966,10 @@ static void basic_http_header(request_re
   * Date and Server are less interesting, use TRACE5 for them while
   * using TRACE4 for the other headers.
   */
-ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r,   %s: %s, Date,
+ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r,   Date: %s,
proxy_date ? proxy_date : date );
  if (server)
-ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r,   %s: %s, Server,
+ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r,   Server: %s,
server);
  }


Just above this, there is the following comment :
/*
 * Date and Server are less interesting, use TRACE5 for them while
 * using TRACE4 for the other headers.
 */
However, I don't see where the other headers are logged.

You can find a little above:
ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, r,
  Response sent with status %d%s,
  r-status,
  APLOGrtrace4(r) ? , headers: : );
but, IMO, it does not log the headers, it only prints headers: .


Apparently, it has been that way, since the beginning in 
http://svn.apache.org/viewvc?view=revisionsortby=daterevision=963057


Should something be added here to match the comment, or remove the comment ?

Best regards,
CJ



Re: svn commit: r1417892 - in /httpd/httpd/branches/2.4.x: ./ docs/manual/ docs/manual/mod/ include/ modules/proxy/

2012-12-06 Thread Marion Christophe JAILLET


Le 06/12/2012 14:59, j...@apache.org a écrit :

Author: jim
Date: Thu Dec  6 13:59:32 2012
New Revision: 1417892

URL: http://svn.apache.org/viewvc?rev=1417892view=rev
Log:
Merge r1404653 from trunk:

Allow for setting of sticky session split char...
Bugz 53893


[...]

--- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.h Thu Dec  6 13:59:32 
2012
@@ -425,6 +425,7 @@ typedef struct {
  unsigned intvhosted:1;
  unsigned intinactive:1;
  unsigned intforcerecovery:1;
+char  sticky_separator;/* separator 
for sessionid/route */
  } proxy_balancer_shared;
  
  #define ALIGNED_PROXY_BALANCER_SHARED_SIZE (APR_ALIGN_DEFAULT(sizeof(proxy_balancer_shared)))



Hi,

I think that the change of position of the sticky_separator struct 
member, should be forward-ported to trunk, shouldn't it ?


Best regards,
CJ


<    1   2