Re: Weird SSL issue with module
On Mon, Sep 21, 2009 at 23:14, Joe Nardone jnard...@gmail.com wrote: Under 2.2, it appears that no matter what I do, the headers and data are being sent in two separate SSL records in the response. This is true at least for POST requests. How large are these POST requests, Joe? I ask because SSL_read() doesn't support records 16 kB (which may or may not be the maximum size for TLS records - it probably is, but don't take my word for it). http://www.openssl.org/docs/ssl/SSL_read.html
Re: Weird SSL issue with module
On Mon, Sep 21, 2009 at 23:14, Joe Nardone jnard...@gmail.com wrote: Under 2.2, it appears that no matter what I do, the headers and data are being sent in two separate SSL records in the response. This is true at least for POST requests. How large are these POST requests, Joe? I ask because SSL_read() doesn't support records 16 kB (which may or may not be the maximum size for TLS records - it probably is, but don't take my word for it). http://www.openssl.org/docs/ssl/SSL_read.html The request size isn't the problem (and you can have requests larger than 16k, it's just in more than one data record/fragment). The problem is the response -- regardless of response size (even a few bytes), the headers and data are being sent in separate segments despite comments in the source indicating this should not occur. I have a content-length header, so it's not an issue of chunking. I guess I'm going to have to roll my own httpd/mod_ssl with more debugging and try and unwind this. joe
AW: OCSP stapling in mod_ssl - use as OCSP cache for client authentication
After some time off (off this topic, at least), I am now trying to work my way
through the stapling code and the mechanisms of caching in mod_ssl. Phew... I
had expected this to be a little more straight forward even for a non-C-guru
like me. *sigh* ;)
My favourite place to have the OCSP caching done is in ssl_engine_ocsp.c -
verify_ocsp_status(), right where an OCSP request would be created:
static int verify_ocsp_status(...)
{
[...]
/* Query response cache HERE */
ruri = determine_responder_uri(sc, cert, c, pool);
if (!ruri) {
return V_OCSP_CERTSTATUS_UNKNOWN;
}
request = create_request(ctx, cert, certID, s, pool);
[...]
The idea, inspired by your stapling code, Steve, is:
- Use SHA1 hash of cert (X509_digest(cert, EVP_sha1(), idx, NULL)) as key for
the cache.
- Query cache for entry of idx above
-_ only create and dispatch new request, if no or invalid entry
- store/update response after (new) request has been dispatched and response
has been received
- validate response (either received from cache or from connection to
responder) -- this code is present, of course, and has been further customized
already.
Is there a possibility to *not* customize a bunch of files like ssl_private.h,
ssl_scache.c, ssl_engine_init.c and so on, but have all necessary handling
placed here in ssl_engine_ocsp.c (and maybe ssl_util_ocsp.c), without messing
up?
Of course, I see the pros of a generic approach, but this is definitely only
for internal OCSP caching nothing to be communicated outside of this place.
On a more detailed level, it is the caching mechanisms that trouble me. What
needs to be done to execute caching operations?
- Define and initialize a cache - similar to SSL session cache. The actual SSL
session cache functions and structures cannot be used for this, just with a
different storage (file)?
- Define and initialize a mutex for access to this additional cache...
So, I am lacking the right way to start this up, aims being at first a quick
implementation and continued refactoring to a really well done solution
afterwards. :-/
Mit freundlichen Grüßen / Kind regards
Natanael Mignon
Von: Dr Stephen Henson [shen...@oss-institute.org]
Gesendet: Freitag, 11. September 2009 11:45
An: dev@httpd.apache.org
Betreff: Re: OCSP stapling in mod_ssl - use as OCSP cache for client
authentication
What I think you are trying to do is to include a cache for OCSP queries the
proxy itself makes which is IMHO the best solution. So instead of always
consulting the OCSP responder it instead checks the cache to see if there is a
valid OCSP response in there, if it is expired or invalid then and only then
would it renew the response by making an actual query. Doing things that way
doesn't need OCSP stapling support in the server(s).
If that's correct then you could reuse some of the OCSP response query and
caching code in the stapling patch. It implements similar functionality.
Re: mod_fcgid
On Mon, Sep 21, 2009 at 7:16 PM, Ricardo Cantu rica...@smartcsc.com wrote: I'm a long time user of mod_fcgi BTW, I think you mean mod_fastcgi. I haven't found a true mod_fcgi, though some people on the web have used that to refer to mod_fcgid. and would like to start using mod_fcgid. I've been running mod_fcgi with a custom patch of mine. I like to see if it could be included in mod_fcgid or maybe you can give me an other way to accomplish what I need without the patch. What I have is one C program that lives on a linux server. I want to use a web browser as the UI. The C program is a classic one instance to one UI. If 5 people want to run this program I need five separate and persistent copies of the program running, one for each person. When one is done then the program exits. What I've done is sym-linked my program to make unique program names like: program-1 program-2 program-3 etc.. Then each browser asks for a different name. plus the patch that allows setting killInterval to 0, so the process manager won't kill my process ever. Here are the settings: -maxClassProcesses 1 -singleThreshold 1 -killInterval 0. I know I could have done it with FastCGIExternalServer, but I wanted everything to be dynamic. I think this may work for you: DefaultMinClassProcessCount 1 DefaultMaxClassProcessCount 1 IdleTimeout 2147483647 ProcessLifetime 2147483647 Those magic values for IdleTimeout and ProcessLifetime are as close to unlimited as you can get with the current code. Note that these can only be set globally with mod_fcgid. That's probably a much bigger problem than the unlimited hack.
Re: mod_fcgid
On Tuesday 22 September 2009 7:03:36 am Jeff Trawick wrote: On Mon, Sep 21, 2009 at 7:16 PM, Ricardo Cantu rica...@smartcsc.com wrote: I'm a long time user of mod_fcgi BTW, I think you mean mod_fastcgi. I haven't found a true mod_fcgi, though some people on the web have used that to refer to mod_fcgid. Yes, mod_fastcgi, and would like to start using mod_fcgid. I've been running mod_fcgi with a custom patch of mine. I like to see if it could be included in mod_fcgid or maybe you can give me an other way to accomplish what I need without the patch. What I have is one C program that lives on a linux server. I want to use a web browser as the UI. The C program is a classic one instance to one UI. If 5 people want to run this program I need five separate and persistent copies of the program running, one for each person. When one is done then the program exits. What I've done is sym-linked my program to make unique program names like: program-1 program-2 program-3 etc.. Then each browser asks for a different name. plus the patch that allows setting killInterval to 0, so the process manager won't kill my process ever. Here are the settings: -maxClassProcesses 1 -singleThreshold 1 -killInterval 0. I know I could have done it with FastCGIExternalServer, but I wanted everything to be dynamic. I think this may work for you: DefaultMinClassProcessCount 1 DefaultMaxClassProcessCount 1 IdleTimeout 2147483647 ProcessLifetime 2147483647 Those magic values for IdleTimeout and ProcessLifetime are as close to unlimited as you can get with the current code. Note that these can only be set globally with mod_fcgid. That's probably a much bigger problem than the unlimited hack. It's seem very reasonable to have 0 (unlimited) as valid value for killInterval, IdleTimeout, and ProcessLifetime. As I don't think the actual time of zero seconds would make sense in any of those variables. Any objections to such a patch?
Re: mod_fcgid
On Tue, Sep 22, 2009 at 9:34 AM, Ricardo Cantu rica...@smartcsc.com wrote: On Tuesday 22 September 2009 7:03:36 am Jeff Trawick wrote: I think this may work for you: DefaultMinClassProcessCount 1 DefaultMaxClassProcessCount 1 IdleTimeout 2147483647 ProcessLifetime 2147483647 Those magic values for IdleTimeout and ProcessLifetime are as close to unlimited as you can get with the current code. Note that these can only be set globally with mod_fcgid. That's probably a much bigger problem than the unlimited hack. It's seem very reasonable to have 0 (unlimited) as valid value for killInterval, IdleTimeout, and ProcessLifetime. As I don't think the actual time of zero seconds would make sense in any of those variables. Any objections to such a patch? I think it is perfectly fine to allow IdleTimeout and ProcessLifetime to accept 0 and treat it as unlimited.* Did you mean IdleScanInterval instead of killInterval? IMO it isn't so useful to disable the various scans. --/-- *Currently one of the other directives uses -1 as unlimited. We should change that one to also use 0 as unlimited, but allow -1 during a migration period so that existing configurations work. Note also that we've renamed the directives but continue to support the old ones. The new directive names in the current source code and documented at http://httpd.apache.org/mod_fcgid/mod/mod_fcgid.html#upgrade are just a rough draft. They'll be tweaked further before GA.
Re: mod_fcgid
I think this patch will do it. See what you think. On Tuesday 22 September 2009 8:15:12 am Jeff Trawick wrote: On Tue, Sep 22, 2009 at 9:34 AM, Ricardo Cantu rica...@smartcsc.com wrote: On Tuesday 22 September 2009 7:03:36 am Jeff Trawick wrote: I think this may work for you: DefaultMinClassProcessCount 1 DefaultMaxClassProcessCount 1 IdleTimeout 2147483647 ProcessLifetime 2147483647 Those magic values for IdleTimeout and ProcessLifetime are as close to unlimited as you can get with the current code. Note that these can only be set globally with mod_fcgid. That's probably a much bigger problem than the unlimited hack. It's seem very reasonable to have 0 (unlimited) as valid value for killInterval, IdleTimeout, and ProcessLifetime. As I don't think the actual time of zero seconds would make sense in any of those variables. Any objections to such a patch? I think it is perfectly fine to allow IdleTimeout and ProcessLifetime to accept 0 and treat it as unlimited.* Did you mean IdleScanInterval instead of killInterval? IMO it isn't so useful to disable the various scans. --/-- *Currently one of the other directives uses -1 as unlimited. We should change that one to also use 0 as unlimited, but allow -1 during a migration period so that existing configurations work. Note also that we've renamed the directives but continue to support the old ones. The new directive names in the current source code and documented at http://httpd.apache.org/mod_fcgid/mod/mod_fcgid.html#upgrade are just a rough draft. They'll be tweaked further before GA. Index: docs/manual/mod/mod_fcgid.html.en === --- docs/manual/mod/mod_fcgid.html.en (revision 817674) +++ docs/manual/mod/mod_fcgid.html.en (working copy) @@ -25,8 +25,8 @@ /div table class=moduletrtha href=module-dict.html#DescriptionDescription:/a/thtdProvides for execution of FastCGI applications/td/tr trtha href=module-dict.html#StatusStatus:/a/thtdExternal/td/tr -trtha href=module-dict.html#ModuleIdentifierModule Identifier:/a/thtdfcgid_module/td/tr -trtha href=module-dict.html#SourceFileSource File:/a/thtdmod_fcgid.c/td/tr +trtha href=module-dict.html#ModuleIdentifierModule�Identifier:/a/thtdfcgid_module/td/tr +trtha href=module-dict.html#SourceFileSource�File:/a/thtdmod_fcgid.c/td/tr trtha href=module-dict.html#CompatibilityCompatibility:/a/thtdApache 2.0 and higher/td/tr/table h3Summary/h3 @@ -677,7 +677,8 @@ trtha href=directive-dict.html#ModuleModule:/a/thtdmod_fcgid/td/tr /table pApplication processes which have not handled a request - for this period of time will be terminated./p +for this period of time will be terminated. A value of code0/code +disables the check./p /div div class=topa href=#page-headerimg alt=top src=../images/up.gif //a/div @@ -767,13 +768,13 @@ table class=directive trtha href=directive-dict.html#DescriptionDescription:/a/thtdMax requests handled by each FastCGI application/td/tr trtha href=directive-dict.html#SyntaxSyntax:/a/thtdcodeFCGIDMaxRequestsPerProcess emvalue/em/code/td/tr -trtha href=directive-dict.html#DefaultDefault:/a/thtdcodeFCGIDMaxRequestsPerProcess -1/code/td/tr +trtha href=directive-dict.html#DefaultDefault:/a/thtdcodeFCGIDMaxRequestsPerProcess 0/code/td/tr trtha href=directive-dict.html#ContextContext:/a/thtdserver config, virtual host/td/tr trtha href=directive-dict.html#StatusStatus:/a/thtdExternal/td/tr trtha href=directive-dict.html#ModuleModule:/a/thtdmod_fcgid/td/tr /table pFastCGI application processes will be terminated after handling - the specified number of requests. A value of code-1/code + the specified number of requests. A value of code0/code disables the check./p /div @@ -835,7 +836,8 @@ trtha href=directive-dict.html#ModuleModule:/a/thtdmod_fcgid/td/tr /table pIdle application processes which have existed for greater - than this time will be terminated./p +than this time will be terminated. A value of code0/code +disables the check./p /div div class=topa href=#page-headerimg alt=top src=../images/up.gif //a/div Index: docs/manual/mod/mod_fcgid.xml === --- docs/manual/mod/mod_fcgid.xml (revision 817674) +++ docs/manual/mod/mod_fcgid.xml (working copy) @@ -606,7 +606,8 @@ contextlistcontextserver config/context/contextlist usage pApplication processes which have not handled a request - for this period of time will be terminated./p +for this period of time will be terminated. A value of code0/code + disables the check./p /usage /directivesynopsis @@ -686,11 +687,11 @@ nameFCGIDMaxRequestsPerProcess/name descriptionMax requests handled by each FastCGI application/description syntaxFCGIDMaxRequestsPerProcess emvalue/em/syntax -
Re: mod_fcgid
On Tue, Sep 22, 2009 at 12:04 PM, Ricardo Cantu rica...@smartcsc.comwrote: I think this patch will do it. See what you think. Thanks! I think just a couple of tweaks are needed. Values of -1 for max_requests_per_process need to be fixed at config time so that we don't clutter the main logic with migration support. The function that handles parsing that directive is set_max_requests_per_process() in fcgid_conf.c. Also in that file, DEFAULT_MAX_REQUESTS_PER_PROCESS needs to be 0 instead of -1. Then, the check +} else if (sconf-max_requests_per_process 0// negative and zero are unlimited doesn't have to be handled differently than the others. (BTW, it is a tiny bit helpful to omit generated .html.* from patches.)
Re: mod_fcgid
On Tuesday 22 September 2009 10:39:32 am Jeff Trawick wrote:
On Tue, Sep 22, 2009 at 12:04 PM, Ricardo Cantu rica...@smartcsc.comwrote:
I think this patch will do it. See what you think.
Thanks! I think just a couple of tweaks are needed.
Values of -1 for max_requests_per_process need to be fixed at config time
so that we don't clutter the main logic with migration support. The
function that handles parsing that directive is
set_max_requests_per_process() in fcgid_conf.c.
Done.
Also in that file, DEFAULT_MAX_REQUESTS_PER_PROCESS needs to be 0 instead
of -1.
Done.
Then, the check
+} else if (sconf-max_requests_per_process 0// negative and
zero are unlimited
doesn't have to be handled differently than the others.
Done.
(BTW, it is a tiny bit helpful to omit generated .html.* from patches.)
Sorry, will do.
Index: modules/fcgid/fcgid_pm_main.c
===
--- modules/fcgid/fcgid_pm_main.c (revision 817674)
+++ modules/fcgid/fcgid_pm_main.c (working copy)
@@ -59,7 +59,8 @@
/* Should I check the idle list now? */
if (procmgr_must_exit()
|| apr_time_sec(now) - apr_time_sec(lastidlescan) =
-sconf-idle_scan_interval)
+sconf-idle_scan_interval
+|| (!sconf-idle_timeout !sconf-proc_lifetime))
return;
lastidlescan = now;
@@ -74,17 +75,17 @@
next_node = proc_table[current_node-next_index];
last_active_time = current_node-last_active_time;
start_time = current_node-start_time;
-if ((apr_time_sec(now) - apr_time_sec(last_active_time)
- sconf-idle_timeout
- || apr_time_sec(now) - apr_time_sec(start_time)
- sconf-proc_lifetime)
+if (((sconf-idle_timeout (apr_time_sec(now) - apr_time_sec(last_active_time)
+ sconf-idle_timeout))
+ || (sconf-proc_lifetime (apr_time_sec(now) - apr_time_sec(start_time)
+ sconf-proc_lifetime)))
is_kill_allowed(main_server, current_node)) {
/* Set die reason for log */
-if (apr_time_sec(now) - apr_time_sec(last_active_time)
-sconf-idle_timeout)
+if (sconf-idle_timeout (apr_time_sec(now) - apr_time_sec(last_active_time)
+ sconf-idle_timeout))
current_node-diewhy = FCGID_DIE_IDLE_TIMEOUT;
-else if (apr_time_sec(now) - apr_time_sec(start_time)
- sconf-proc_lifetime)
+else if (sconf-proc_lifetime (apr_time_sec(now) - apr_time_sec(start_time)
+ sconf-proc_lifetime))
current_node-diewhy = FCGID_DIE_LIFETIME_EXPIRED;
/* Unlink from idle list */
Index: modules/fcgid/fcgid_conf.c
===
--- modules/fcgid/fcgid_conf.c (revision 817674)
+++ modules/fcgid/fcgid_conf.c (working copy)
@@ -47,7 +47,7 @@
#define DEFAULT_IPC_CONNECT_TIMEOUT 3
#define DEFAULT_IPC_COMM_TIMEOUT 40
#define DEFAULT_OUTPUT_BUFFERSIZE 65536
-#define DEFAULT_MAX_REQUESTS_PER_PROCESS -1
+#define DEFAULT_MAX_REQUESTS_PER_PROCESS 0
#define DEFAULT_MAX_REQUEST_LEN (1024*1024*1024)/* 1G */
#define DEFAULT_MAX_MEM_REQUEST_LEN (1024*64) /* 64k */
#define DEFAULT_WRAPPER_KEY ALL
@@ -472,7 +472,9 @@
server_rec *s = cmd-server;
fcgid_server_conf *config =
ap_get_module_config(s-module_config, fcgid_module);
-config-max_requests_per_process = atol(arg);
+if ((config-max_requests_per_process = atol(arg)) == -1) {
+ config-max_requests_per_process = 0;
+ }
config-max_requests_per_process_set = 1;
return NULL;
}
Index: modules/fcgid/fcgid_bridge.c
===
--- modules/fcgid/fcgid_bridge.c (revision 817674)
+++ modules/fcgid/fcgid_bridge.c (working copy)
@@ -193,7 +193,7 @@
ctx-procnode-diewhy = FCGID_DIE_COMM_ERROR;
return_procnode(s, ctx-procnode,
1 /* communication error */ );
-} else if (sconf-max_requests_per_process != -1
+} else if (sconf-max_requests_per_process
++ctx-procnode-requests_handled =
sconf-max_requests_per_process) {
ctx-procnode-diewhy = FCGID_DIE_LIFETIME_EXPIRED;
Re: mod_fcgid: Problem serving binary content with Apache 2.2.13 - PHP 5.2.9
Ok, i am puzzled Jeff Trawick traw...@gmail.com schrieb im Newsbeitrag news:cc67648e0909211455i5cb6c7c3ub4fdcc25cb9cc...@mail.gmail.com... Please try translating the CGI config directly to FastCGI as noted above, instead of using the Plesk-generated config, and let us know what happens. Replace this bit Files ~ (\.php) SetHandler fcgid-script FCGIWrapper /usr/bin/php-cgi5 .php Options +ExecCGI Allow from all /Files with Alias /phppath/ /usr/bin/ Location /phppath/ SetHandler fcgid-script Options +ExecCGI /Location I could not get it to work in my httpd.include file as Apache was always throwing an error (i.e. Alias not allowed here, Location not allowed here etc). I ended up changing my /etc/apache2/conf.d/mod_fcgid.conf (excerpt) from: --- ## ## PHP via FastCGI ## ## uncomment the following line if you want to handle php via mod_fcgid ## #FilesMatch \.php$ #AddHandler fcgid-script .php #FCGIWrapper /srv/www/cgi-bin/php5 .php #Options +ExecCGI #/FilesMatch ## /IfModule # End of IfModule fcgid_module ## --- to --- ## ## PHP via FastCGI ## ## uncomment the following line if you want to handle php via mod_fcgid ## #FilesMatch \.php$ #AddHandler fcgid-script .php #FCGIWrapper /srv/www/cgi-bin/php5 .php #Options +ExecCGI #/FilesMatch ## Alias /phppath/ /usr/bin/ Location /phppath/ SetHandler fcgid-script Options +ExecCGI /Location /IfModule # End of IfModule fcgid_module ## so basically inserting your replacement code at the bottom inside the IfModule fcgid_module container. Now there are two things: 1) On virtual hosts which use 'FastCGI-Application' in Plesk and which httpd.include looks like this: VirtualHost IP:80 ... Directory /srv/www/vhosts/domain.tld/httpdocs IfModule mod_perl.c Files ~ (\.pl$) SetHandler perl-script PerlHandler ModPerl::Registry Options ExecCGI allow from all PerlSendHeader On /Files /IfModule IfModule mod_python.c Files ~ (\.py$) SetHandler python-program PythonHandler mod_python.cgihandler /Files /IfModule IfModule mod_fcgid.c Files ~ (\.fcgi) SetHandler fcgid-script Options +FollowSymLinks +ExecCGI /Files /IfModule IfModule mod_fcgid.c Files ~ (\.php) SetHandler fcgid-script FCGIWrapper /usr/bin/php-cgi5 .php Options +ExecCGI allow from all /Files /IfModule Options -Includes -ExecCGI /Directory Include /srv/www/vhosts/domain.tld/conf/vhost.conf /VirtualHost a) I sometimes get all files offered to download (not really able to replicate this, it seems it is connected to the use of .htaccess and modify.php in the same directory i sometimes get php files with image/jpeg header for download, sometimes with x-httpd-php header...kind of bizarre...) and b) The watermark image still fails for the same reason as stated before: PHP tries to parse the image. I have proof that this config uses mod_fcgid because phpinfo() gives only _ENV[PATH] under Environment plus i do get a _SERVER[Authorization] and other Auth variables when calling phpinfo() from a .htaccess secured directory (AuthType Basic) which was not the case if it was mod_cgi. 2) Virtual hosts which use 'CGI-Application' in Plesk and which httpd.include therefore looks like this: VirtualHost IP:80 ... Directory /srv/www/vhosts/domain.tld/httpdocs IfModule mod_perl.c Files ~ (\.pl$) SetHandler perl-script PerlHandler ModPerl::Registry Options ExecCGI allow from all PerlSendHeader On /Files /IfModule IfModule mod_python.c Files ~ (\.py$) SetHandler python-program PythonHandler mod_python.cgihandler /Files /IfModule IfModule mod_fcgid.c Files ~ (\.fcgi) SetHandler fcgid-script Options +FollowSymLinks +ExecCGI /Files /IfModule Files ~ (\.php) AddHandler php-script .php Options +ExecCGI allow from all /Files Options -Includes -ExecCGI /Directory Include /srv/www/vhosts/domain.tld/conf/vhost.conf /VirtualHost do get parsed using mod_fcgid as well now. Maybe this makes sense because in my /etc/apache2/sysconfig.d/loadmodule.conf the mod_cgi.so comes before mod_fcgid.so:
Re: mod_fcgid
On Tue, Sep 22, 2009 at 1:50 PM, Ricardo Cantu rica...@smartcsc.com wrote: On Tuesday 22 September 2009 10:39:32 am Jeff Trawick wrote: On Tue, Sep 22, 2009 at 12:04 PM, Ricardo Cantu rica...@smartcsc.com wrote: I think this patch will do it. See what you think. Thanks! I think just a couple of tweaks are needed. Values of -1 for max_requests_per_process need to be fixed at config time so that we don't clutter the main logic with migration support. The function that handles parsing that directive is set_max_requests_per_process() in fcgid_conf.c. Done. Also in that file, DEFAULT_MAX_REQUESTS_PER_PROCESS needs to be 0 instead of -1. Done. Then, the check +} else if (sconf-max_requests_per_process 0// negative and zero are unlimited doesn't have to be handled differently than the others. Done. (BTW, it is a tiny bit helpful to omit generated .html.* from patches.) Sorry, will do. But you omitted the .xml too ;) Anyway, the latest patch and the previous xml have been committed as r817772. Thanks! (I tweaked whitespace ever so slightly and noted that FCGIDMaxRequestsPerProcess/MaxRequestsPerProcess still accepts -1.)
Re: Memory usage, core output filter, and apr_brigade_destroy
On Sunday 13 September 2009, Stefan Fritsch wrote:
On Sunday 13 September 2009, Ruediger Pluem wrote:
On 09/13/2009 01:11 PM, Stefan Fritsch wrote:
http://httpd.apache.org/docs/trunk/developer/output-filters.htm
l recommends to reuse bucket brigades and to not use
apr_brigade_destroy. However, both in 2.2 and in trunk, the
core output filter sometimes calls apr_brigade_destroy on
brigades that it has received down the chain from earlier
output filters. Is this not bound to cause problems since the
brigade's pool cleanup is then removed but the brigade is still
reused later on?
It could be. But the questions is if it is really reused later
on.
Since this is the recommended design for output filters, I am sure
it can happen.
I have attached two patches:
In the first, I change (hopefully) all places to not
apr_brigade_destroy brigades that have been passed down the filter
chain. Especially the core_output_filter change needs some review.
In the second, I have changed all occurences of apr_brigade_split to
apr_brigade_split_ex and I have made some more changes where bucket
brigades can be reused.
The test suite shows no regressions.
Cheers,
Stefan
diff --git a/modules/http/byterange_filter.c b/modules/http/byterange_filter.c
index a79b7f7..92048ab 100644
--- a/modules/http/byterange_filter.c
+++ b/modules/http/byterange_filter.c
@@ -307,7 +307,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_byterange_filter(ap_filter_t *f,
APR_BRIGADE_INSERT_TAIL(bsend, e);
/* we're done with the original content - all of our data is in bsend. */
-apr_brigade_destroy(bb);
+apr_brigade_cleanup(bb);
/* send our multipart output */
return ap_pass_brigade(f-next, bsend);
diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c
index 3e96cb9..01ced24 100644
--- a/modules/http/http_filters.c
+++ b/modules/http/http_filters.c
@@ -1112,7 +1112,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f,
ctx = f-ctx = apr_pcalloc(r-pool, sizeof(header_filter_ctx));
}
else if (ctx-headers_sent) {
-apr_brigade_destroy(b);
+apr_brigade_cleanup(b);
return OK;
}
}
@@ -1283,7 +1283,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f,
ap_pass_brigade(f-next, b2);
if (r-header_only) {
-apr_brigade_destroy(b);
+apr_brigade_cleanup(b);
ctx-headers_sent = 1;
return OK;
}
diff --git a/server/core_filters.c b/server/core_filters.c
index f073765..eb206c1 100644
--- a/server/core_filters.c
+++ b/server/core_filters.c
@@ -316,7 +316,7 @@ int ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
static void setaside_remaining_output(ap_filter_t *f,
core_output_filter_ctx_t *ctx,
apr_bucket_brigade *bb,
- int make_a_copy, conn_rec *c);
+ conn_rec *c);
static apr_status_t send_brigade_nonblocking(apr_socket_t *s,
apr_bucket_brigade *bb,
@@ -392,19 +392,21 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb)
}
}
+if (new_bb != NULL) {
+bb = new_bb;
+}
+
if ((ctx-buffered_bb != NULL)
!APR_BRIGADE_EMPTY(ctx-buffered_bb)) {
-bb = ctx-buffered_bb;
-ctx-buffered_bb = NULL;
if (new_bb != NULL) {
-APR_BRIGADE_CONCAT(bb, new_bb);
+APR_BRIGADE_PREPEND(bb, ctx-buffered_bb);
+}
+else {
+bb = ctx-buffered_bb;
}
c-data_in_output_filters = 0;
}
-else if (new_bb != NULL) {
-bb = new_bb;
-}
-else {
+else if (new_bb == NULL) {
return APR_SUCCESS;
}
@@ -444,7 +446,7 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb)
/* The client has aborted the connection */
c-aborted = 1;
}
-setaside_remaining_output(f, ctx, bb, 0, c);
+setaside_remaining_output(f, ctx, bb, c);
return rv;
}
@@ -508,14 +519,14 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb)
}
}
-setaside_remaining_output(f, ctx, bb, 1, c);
+setaside_remaining_output(f, ctx, bb, c);
return APR_SUCCESS;
}
static void setaside_remaining_output(ap_filter_t *f,
core_output_filter_ctx_t *ctx,
apr_bucket_brigade *bb,
- int make_a_copy, conn_rec *c)
+ conn_rec *c)
{
if (bb == NULL) {
return;
@@ -523,20 +534,14 @@ static void setaside_remaining_output(ap_filter_t *f,
remove_empty_buckets(bb);
if
Re: svn commit: r817830 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID README-FCGID modules/fcgid/fcgid_conf.c modules/fcgid/fcgid_conf.h modules/fcgid/mod_fcgid.c
traw...@apache.org wrote: Author: trawick Date: Tue Sep 22 20:37:44 2009 New Revision: 817830 URL: http://svn.apache.org/viewvc?rev=817830view=rev Log: provide a function to merge per-dir configs so that directives are inherited in the expected manner before: if you then add any fcgid per-dir directive to that container, all other values are reset to defaults instead of continuing to be inherited (not okay) Brilliant, thanks Jeff. What state do you feel the module is in? When should we cut a beta to put the improvements-to-date into users hands?
Re: svn commit: r813376 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml server/core.c
j...@apache.org wrote: URL: http://svn.apache.org/viewvc?rev=813376view=rev Log: veto-ed A belated thanks, and sorry I didn't note this earlier. Is there anything on trunk that precludes cutting an alpha before Jim, Paul or myself snag a tarball for review?
Re: mod_fcgid
On Tuesday 22 September 2009 1:05:24 pm Jeff Trawick wrote: On Tue, Sep 22, 2009 at 1:50 PM, Ricardo Cantu rica...@smartcsc.com wrote: On Tuesday 22 September 2009 10:39:32 am Jeff Trawick wrote: On Tue, Sep 22, 2009 at 12:04 PM, Ricardo Cantu rica...@smartcsc.com wrote: I think this patch will do it. See what you think. Thanks! I think just a couple of tweaks are needed. Values of -1 for max_requests_per_process need to be fixed at config time so that we don't clutter the main logic with migration support. The function that handles parsing that directive is set_max_requests_per_process() in fcgid_conf.c. Done. Also in that file, DEFAULT_MAX_REQUESTS_PER_PROCESS needs to be 0 instead of -1. Done. Then, the check +} else if (sconf-max_requests_per_process 0// negative and zero are unlimited doesn't have to be handled differently than the others. Done. (BTW, it is a tiny bit helpful to omit generated .html.* from patches.) Sorry, will do. But you omitted the .xml too ;) Anyway, the latest patch and the previous xml have been committed as r817772. Thanks! No, Thank you! (I tweaked whitespace ever so slightly and noted that FCGIDMaxRequestsPerProcess/MaxRequestsPerProcess still accepts -1.) Okay, got it all compiled and working. I've stressed here and there and seems that it is running great! I see a little performance boost over the mod_fastcgi module as well. So far the only issue I've come across is when my C program is exited it leaves a zombie process for a couple of seconds then get's cleaned up. Log confirms this as well; [Tue Sep 22 14:40:57 2009] [notice] mod_fcgid: process /var/www/html/engine/bin/engine_10(12453) exit(normal exit), terminated by calling exit(), return code: 0 [Tue Sep 22 14:40:57 2009] [warn] mod_fcgid: cleanup zombie process 12453 Is this normal? or is there a directive that would help it exit cleaner?
Re: mod_fcgid
On Tue, Sep 22, 2009 at 4:50 PM, Ricardo Cantu rica...@smartcsc.com wrote: Okay, got it all compiled and working. I've stressed here and there and seems that it is running great! I see a little performance boost over the mod_fastcgi module as well. So far the only issue I've come across is when my C program is exited it leaves a zombie process for a couple of seconds then get's cleaned up. Log confirms this as well; [Tue Sep 22 14:40:57 2009] [notice] mod_fcgid: process /var/www/html/engine/bin/engine_10(12453) exit(normal exit), terminated by calling exit(), return code: 0 [Tue Sep 22 14:40:57 2009] [warn] mod_fcgid: cleanup zombie process 12453 Is this normal? or is there a directive that would help it exit cleaner? Set FCGIDZombieScanInterval, which defaults to 3 seconds, down to 1. But note that it may not scan for zombies every second because of this = comparison: /* Should I check zombie processes in idle list now? */ if (procmgr_must_exit() || apr_time_sec(now) - apr_time_sec(lastzombiescan) = sconf-zombie_scan_interval) return; Maybe set it to zero to mean 1 second ;) It doesn't currently complain about a setting of 0. (It would be nice not to have to scan for zombies at a set interval. The process manager could be awakened when a subprocess exits, at least on Unix.)
Re: svn commit: r817830 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID README-FCGID modules/fcgid/fcgid_conf.c modules/fcgid/fcgid_conf.h modules/fcgid/mod_fcgid.c
On Tue, Sep 22, 2009 at 4:44 PM, William A. Rowe, Jr. wr...@rowe-clan.netwrote: What state do you feel the module is in? When should we cut a beta to put the improvements-to-date into users hands? I think it is in a good state. A new tag tomorrow would be great. I'll test more between now and then. (If it gets released, we should warn users that the new directive names haven't settled until developers have had time to further refine them.)
Re: svn commit: r817830 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID README-FCGID modules/fcgid/fcgid_conf.c modules/fcgid/fcgid_conf.h modules/fcgid/mod_fcgid.c
Jeff Trawick wrote: On Tue, Sep 22, 2009 at 4:44 PM, William A. Rowe, Jr. wr...@rowe-clan.net mailto:wr...@rowe-clan.net wrote: What state do you feel the module is in? When should we cut a beta to put the improvements-to-date into users hands? I think it is in a good state. A new tag tomorrow would be great. I'll test more between now and then. (If it gets released, we should warn users that the new directive names haven't settled until developers have had time to further refine them.) +++1... I'd not even offer GA as an option quite yet. Soon.
Re: mod_fcgid
On Tuesday 22 September 2009 3:04:46 pm Jeff Trawick wrote:
On Tue, Sep 22, 2009 at 4:50 PM, Ricardo Cantu rica...@smartcsc.com wrote:
Okay, got it all compiled and working. I've stressed here and there and
seems
that it is running great! I see a little performance boost over the
mod_fastcgi module as well.
So far the only issue I've come across is when my C program is exited it
leaves a zombie process for a couple of seconds then get's cleaned up.
Log confirms this as well;
[Tue Sep 22 14:40:57 2009] [notice] mod_fcgid: process
/var/www/html/engine/bin/engine_10(12453) exit(normal exit), terminated
by calling exit(), return code: 0
[Tue Sep 22 14:40:57 2009] [warn] mod_fcgid: cleanup zombie process 12453
Is this normal? or is there a directive that would help it exit cleaner?
Set FCGIDZombieScanInterval, which defaults to 3 seconds, down to 1. But
note that it may not scan for zombies every second because of this =
comparison:
/* Should I check zombie processes in idle list now? */
if (procmgr_must_exit()
|| apr_time_sec(now) - apr_time_sec(lastzombiescan) =
sconf-zombie_scan_interval)
return;
Maybe set it to zero to mean 1 second ;) It doesn't currently complain
about a setting of 0.
(It would be nice not to have to scan for zombies at a set interval. The
process manager could be awakened when a subprocess exits, at least on
Unix.)
Okay, so this looks like the place to register some type of wake up callback
on exit.
fcgid_proc_unix.c
apr_status_t
proc_spawn_process(char *lpszwapper, fcgid_proc_info * procinfo,
fcgid_procnode * procnode)
.
.
.
/* Unlink it when process exit */
if (ap_unixd_config.suexec_enabled) {
apr_pool_cleanup_register(procnode-proc_pool,
procnode, socket_file_cleanup,
exec_setuid_cleanup);
} else {
apr_pool_cleanup_register(procnode-proc_pool,
procnode, socket_file_cleanup,
apr_pool_cleanup_null);
}
.
.
.
And this is what we would want to wake up:
fcgid_pm_main.c
apr_status_t pm_main(server_rec * main_server, apr_pool_t * configpool)
{
.
.
/* Wait for command */
if (procmgr_peek_cmd(command, main_server) == APR_SUCCESS) {
if (is_spawn_allowed(main_server, command))
fastcgi_spawn(command, main_server, configpool);
procmgr_finish_notify(main_server);
}
.
.
.
What you think?
Re: [VOTE] release httpd mod_ftp-0.9.5 beta?
Bill, William A. Rowe, Jr. schrieb: Guenter Knauf wrote: ftp_commands.c: In function ‘common_list’: ftp_commands.c:694: warning: suggest parentheses around within || As we worked out, the distinction made no effective difference, but... ftp_protocol.c: In function ‘ftp_read_line’: ftp_protocol.c:244: warning: comparison is always false due to limited range of data type ftp_protocol.c:246: warning: comparison is always false due to limited range of data type ftp_protocol.c:246: warning: comparison is always true due to limited range of data type ftp_protocol.c:255: warning: comparison is always false due to limited range of data type ftp_protocol.c:258: warning: comparison is always false due to limited range of data type ftp_protocol.c:258: warning: comparison is always true due to limited range of data type we should fix these first, and re-roll ... that sounds right; this code was faulty on platforms where '\xff' != 0xff. found another issue while testing with FileZilla: Befehl: FEAT Antwort:211-Extensions supported Antwort: TVFS Antwort: UTF8 Antwort: REST STREAM Antwort: MDTM Antwort: EPRT Antwort: SIZE Antwort: PBSZ Antwort: PROT Antwort: AUTH TLS Antwort: EPSV Antwort:211 End Befehl: OPTS UTF8 ON Antwort:504 Error (no message) so clearly mod_ftp now claims to support UTF8, but when FZ tries to set it then it returns an error Gün.
Re: [VOTE] release httpd mod_ftp-0.9.5 beta?
Guenter Knauf wrote: Befehl: OPTS UTF8 ON Antwort: 504 Error (no message) so clearly mod_ftp now claims to support UTF8, but when FZ tries to set it then it returns an error Good catch. Investigating, but I believe returning success is sufficient for most unix and windows platforms. But OS2? Do we need to evaluate that APR_FILEPATH_ENCODING_UTF8 flag? Thoughts?
Re: [VOTE] release httpd mod_ftp-0.9.5 beta?
William A. Rowe, Jr. wrote: Guenter Knauf wrote: Befehl: OPTS UTF8 ON Antwort: 504 Error (no message) so clearly mod_ftp now claims to support UTF8, but when FZ tries to set it then it returns an error Good catch. Investigating, but I believe returning success is sufficient for most unix and windows platforms. But OS2? Do we need to evaluate that APR_FILEPATH_ENCODING_UTF8 flag? With respect to FZ, I plan no change for client authors who deliberately ignore the relevant RFC's. See specifically; http://www.rfc-editor.org/rfc/rfc3659.txt http://www.rfc-editor.org/rfc/rfc2640.txt http://www.rfc-editor.org/rfc/rfc2389.txt and understanding these, review the author's commentary... http://www.smartftp.com/forums/index.php?/topic/12655-bug-in-utf8-support/ http://www.smartftp.com/forums/index.php?/topic/5789-feat-and-opts-utf8/ ... who clearly don't know how to read a spec, never mind how to write a client. There is no RFC specified command OPTS UTF8, and we should not persist this myth.
Re: [VOTE] release httpd mod_ftp-0.9.5 beta?
Hi, William A. Rowe, Jr. schrieb: William A. Rowe, Jr. wrote: Guenter Knauf wrote: Befehl: OPTS UTF8 ON Antwort:504 Error (no message) so clearly mod_ftp now claims to support UTF8, but when FZ tries to set it then it returns an error Good catch. Investigating, but I believe returning success is sufficient for most unix and windows platforms. But OS2? Do we need to evaluate that APR_FILEPATH_ENCODING_UTF8 flag? With respect to FZ, I plan no change for client authors who deliberately ignore the relevant RFC's. See specifically; http://www.rfc-editor.org/rfc/rfc3659.txt http://www.rfc-editor.org/rfc/rfc2640.txt http://www.rfc-editor.org/rfc/rfc2389.txt and understanding these, review the author's commentary... http://www.smartftp.com/forums/index.php?/topic/12655-bug-in-utf8-support/ http://www.smartftp.com/forums/index.php?/topic/5789-feat-and-opts-utf8/ ... who clearly don't know how to read a spec, never mind how to write a client. There is no RFC specified command OPTS UTF8, and we should not persist this myth. from my thoughts I agree if its not in RFC, and BTW. FZ ignores the 504; but FYI IE7 sends this same OPTS UTF8 command too - seen there first. Some more results: FZ, IE6, IE7, Opera 10.0, Mozilla 1.7.13 work fine against Linux + NetWare; BUT: FF 2.0.0.20, 3.0.13, 3.5.3, SeaMonkey 1.1.16 _ALL_ DONT display the root directory - though they _ALL_ do display /incoming ? Can anybody re-create this with the default config? Gün,
Re: [VOTE] release httpd mod_ftp-0.9.5 beta?
Hi, William A. Rowe, Jr. schrieb: William A. Rowe, Jr. wrote: Guenter Knauf wrote: Befehl: OPTS UTF8 ON Antwort:504 Error (no message) so clearly mod_ftp now claims to support UTF8, but when FZ tries to set it then it returns an error Good catch. Investigating, but I believe returning success is sufficient for most unix and windows platforms. But OS2? Do we need to evaluate that APR_FILEPATH_ENCODING_UTF8 flag? With respect to FZ, I plan no change for client authors who deliberately ignore the relevant RFC's. See specifically; http://www.rfc-editor.org/rfc/rfc3659.txt http://www.rfc-editor.org/rfc/rfc2640.txt http://www.rfc-editor.org/rfc/rfc2389.txt and understanding these, review the author's commentary... http://www.smartftp.com/forums/index.php?/topic/12655-bug-in-utf8-support/ http://www.smartftp.com/forums/index.php?/topic/5789-feat-and-opts-utf8/ ... who clearly don't know how to read a spec, never mind how to write a client. There is no RFC specified command OPTS UTF8, and we should not persist this myth. well, thats fine, but at least FZ only uses it because mod_ftp claims to support it - so why do we so if we anyway dont take action on it but only fail? Lets remove it from FEAT reply unless we really support it, and take action on the command. G.
Re: [VOTE] release httpd mod_ftp-0.9.5 beta?
Guenter Knauf wrote: well, thats fine, but at least FZ only uses it because mod_ftp claims to support it - so why do we so if we anyway dont take action on it but only fail? Lets remove it from FEAT reply unless we really support it, and take action on the command. Reread the spec. FEAT - UTF8 tells the client that utf8 is preferred. On unix systems today, that is largely true. On Windows, it's absolutely true from the httpd perspective. And I'm conceding it might be wrong for OS2 or Netware and need your feedback. FEAT - UTF8 does not say UTF8 is the only format to expect, only that it's the format that should be presumed. FEAT UTF8 has no associated OPTS, please read the spec. OPTS takes no arg ON, that's for certain ;-) Bill
Re: [VOTE] release httpd mod_ftp-0.9.5 beta?
Hi, William A. Rowe, Jr. schrieb: Guenter Knauf wrote: well, thats fine, but at least FZ only uses it because mod_ftp claims to support it - so why do we so if we anyway dont take action on it but only fail? Lets remove it from FEAT reply unless we really support it, and take action on the command. Reread the spec. did... FEAT - UTF8 tells the client that utf8 is preferred. On unix systems today, that is largely true. On Windows, it's absolutely true from the httpd perspective. And I'm conceding it might be wrong for OS2 or Netware and need your feedback. FEAT - UTF8 does not say UTF8 is the only format to expect, only that it's the format that should be presumed. not 100% sure, but ATM would say NW does not support UTF8 with the filenames without conversion ... (we had problems in the past with filenames on httpd autoindexed pages, and need mod_chatset_lite to workaround this) FEAT UTF8 has no associated OPTS, please read the spec. OPTS takes no arg ON, that's for certain ;-) k, I dont care much about the error since its ignored by FZ and IE as it seems Gün.
Re: [VOTE] release httpd mod_ftp-0.9.5 beta?
Guenter Knauf wrote: FEAT - UTF8 tells the client that utf8 is preferred. On unix systems today, that is largely true. On Windows, it's absolutely true from the httpd perspective. And I'm conceding it might be wrong for OS2 or Netware and need your feedback. FEAT - UTF8 does not say UTF8 is the only format to expect, only that it's the format that should be presumed. not 100% sure, but ATM would say NW does not support UTF8 with the filenames without conversion ... (we had problems in the past with filenames on httpd autoindexed pages, and need mod_chatset_lite to workaround this) Then I'll roll tomorrow afternoon along with mod_fcgid. We have two ways to handle this; FTPOptions NoUTF8Feature, or something like; --- mod_ftp.c (revision 816386) +++ mod_ftp.c (working copy) @@ -86,12 +86,14 @@ ap_add_version_component(p, FTP_SERVER_STRING); +#if !defined(NETWARE) !defined(OS2) /* * Unless disabled, advertise that UTF8 filenames are preferred/permitted * RFC2640 never -requires- UTF8 names */ if (!(basefsc-options FTP_OPT_NO_UTF8_FEAT)) ftp_feat_advert(UTF8); +#endif /* Finalize ftp_cmd_help and ftp_cmd_feat messages */ ftp_cmd_finalize(p, ptemp); thoughts?
