Re: svn commit: r901578 - in /httpd/httpd/trunk: CHANGES modules/http/http_request.c modules/metadata/mod_headers.c server/protocol.c
On 21.01.2010 08:20, wr...@apache.org wrote: > Author: wrowe > Date: Thu Jan 21 07:19:41 2010 > New Revision: 901578 > > URL: http://svn.apache.org/viewvc?rev=901578&view=rev > Log: > Correctly align the behavior of headers_in to be consistent with the > treatment of headers_out, resolving PR 48359 by keeping subrequest > scope changes out of the main request headers. This ensures that all > requests-without-bodies behave as the requests-with-bodies code has. > > Modified: > httpd/httpd/trunk/CHANGES > httpd/httpd/trunk/modules/http/http_request.c > httpd/httpd/trunk/modules/metadata/mod_headers.c > httpd/httpd/trunk/server/protocol.c > > Modified: httpd/httpd/trunk/modules/http/http_request.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_request.c?rev=901578&r1=901577&r2=901578&view=diff > == > --- httpd/httpd/trunk/modules/http/http_request.c (original) > +++ httpd/httpd/trunk/modules/http/http_request.c Thu Jan 21 07:19:41 2010 > @@ -442,7 +442,7 @@ > new->request_time= r->request_time; > new->main= r->main; > > -new->headers_in = r->headers_in; > +new->headers_in = apr_table_copy(r->pool, r->headers_in); > new->headers_out = apr_table_make(r->pool, 12); > new->err_headers_out = r->err_headers_out; > new->subprocess_env = rename_original_env(r->pool, r->subprocess_env); > @@ -515,6 +515,8 @@ > r->per_dir_config = rr->per_dir_config; > /* copy output headers from subrequest, but leave negotiation headers */ > r->notes = apr_table_overlay(r->pool, rr->notes, r->notes); > +r->headers_in = apr_table_overlay(r->pool, rr->headers_in, > + r->headers_in); I think this is wrong. Think of the case where the subrequest does not change anything on headers_in. In this case the contents of headers_in of the request and the subrequest are the same (the subrequest has a copy of the requests table). Doing an apr_table_overlay would result in a table that has all key value pairs of the original headers_in doubled. Regards Rüdiger
Re: next alpha, this Wednesday?
On 1/20/2010 11:01 PM, Paul Querna wrote: > I think i'll take the 1.4.2 APR tag, and try to use APR-Util 1.3.9 That sounds sensible. I looked at either dropping crypto or backing up to use the first take (as in the 2.3.4-alpha tarball), but really don't want to do either hastily, with no feedback from the author. If this 1.3.9 does the trick, we can take the time to do 1.4 right. Actually working up a map the inform newer committers of the API argument logic, and identify the broken API's for 2.0. Though some might think it should be obvious from the existing functions, there are a number of bad examples now to lead the committer astray. Bill
Re: next alpha, this Wednesday?
I think i'll take the 1.4.2 APR tag, and try to use APR-Util 1.3.9 On Wed, Jan 20, 2010 at 3:17 PM, William A. Rowe Jr. wrote: > On 1/20/2010 4:56 PM, Sander Temme wrote: >> >> On Jan 20, 2010, at 9:42 AM, William A. Rowe Jr. wrote: >> >>> On 1/20/2010 10:01 AM, Sander Temme wrote: And the %lld printf formatter on MacOSX... small, but it does break a test. >>> >>> That's fixed on the branch, or no? About to tag 1.4.2. >> >> I'm about to get on a flight, will test once under way. > > Sorry - missed it. It's tagged, and we'll move along to fix more bugs with > the next release. > > I don't see a whole lot of reason to hold up a release if nobody on the > platform > has cared to patch a known flaw in 3 months :) > > We can sort of count on a 1.4.3 soon, I have a request to fix a win32 edge > case > for subversion that didn't merit destabilizing this release, but would be a > great fix to have for windows users, before subversion 1.7 is released. > >
Re: [mod_fcgid PATCH] catch errors from setuid()/seteuid()
I man seteuid in my Linux box, there are two types of errors: ERRORS The seteuid() function shall fail if: EINVAL The value of the uid argument is invalid and is not supported by the implementation. EPERM The process does not have appropriate privileges and uid does not match the real group ID or the saved set-group- ID. If directly pass 0 in setuid(), EINVAL may not happend If this process is seteuid from root, EPERM may not happend so, I think the check is just a textbook logic check? just call _exit(1) if it fail? -- From: "Jeff Trawick" Sent: Thursday, January 21, 2010 5:38 AM To: Subject: [mod_fcgid PATCH] catch errors from setuid()/seteuid() > During the last hackathon, Paul was kind enough to run the clang/llvm > static analysis on mod_fcgid > (http://zeus.kimaker.com/~chip/fcgid-scan/). That pointed out these > setuid()/seteuid() calls that aren't checked prior to running a child. > > The error checking itself is simple enough, but there's an ugly aspect > of the implementation that results in trying to switch effective/real > uids multiple times that I worked around. (See the FIXME text in the > patch. I'm not aware of a simple solution, especially one simple > enough to get into 2.3.5) The seteuid() call would otherwise fail on > subsequent invocations for the same child. > > IIRC Joe thought that the seteuid() wasn't needed at all, but the > setuid() fails without it on Solaris. > > Concerns? > > Is there some reason that testing on Linux and Solaris wouldn't be sufficient? >
[mod_fcgid PATCH] display elapsed instead of absolute time in server status
to represent when process was started and when it was last used.
Any concerns with this somewhat gratuitous change?
(also changes "Requests handled" to "Accesses")
now:
Pid Active IdleAccessesState
30890 99 0 1479Ready
and after all such tables:
"Active and Idle are time active and time since last request, in seconds."
before:
Pid Start time Last active timeRequest handled State
18135 1258059310 1258059318 126 Ready
17670 1258059285 1258059327 732 Ready
17671 1258059286 1258059327 948 Ready
18136 1258059310 1258059327 294 Ready
17672 1258059287 1258059320 331 Ready
Index: modules/fcgid/mod_fcgid.c
===
--- modules/fcgid/mod_fcgid.c (revision 901434)
+++ modules/fcgid/mod_fcgid.c (working copy)
@@ -295,6 +295,7 @@
gid_t last_gid = 0;
uid_t last_uid = 0;
apr_size_t last_share_grp_id = 0;
+apr_time_t now;
const char *last_virtualhost = NULL;
const char *basename, *tmpbasename;
fcgid_procnode *proc_table = proctable_get_table_array();
@@ -353,6 +354,8 @@
}
proctable_unlock(r);
+now = apr_time_now();
+
/* Sort the array */
if (num_ent != 0)
qsort((void *)ar, num_ent, sizeof(fcgid_procnode *),
@@ -383,8 +386,8 @@
/* Create a new table for this process info */
ap_rputs("\n\n"
- "PidStart timeLast active time"
- "Requests handledState"
+ "PidActiveIdle"
+ "AccessesState"
"\n", r);
last_inode = current_node->inode;
@@ -396,13 +399,18 @@
}
ap_rprintf(r, "%" APR_PID_T_FMT "%" APR_TIME_T_FMT
"%" APR_TIME_T_FMT "%d%s",
- current_node->proc_id.pid,
apr_time_sec(current_node->start_time),
- apr_time_sec(current_node->last_active_time),
+ current_node->proc_id.pid,
+ apr_time_sec(now - current_node->start_time),
+ apr_time_sec(now - current_node->last_active_time),
current_node->requests_handled,
get_state_desc(current_node));
}
-if (num_ent != 0)
+if (num_ent != 0) {
ap_rputs("\n\n", r);
+ap_rputs("\n"
+ "Active and Idle are time active and time
since\n"
+ "last request, in seconds.\n", r);
+}
return OK;
}
Re: next alpha, this Wednesday?
On 1/20/2010 4:56 PM, Sander Temme wrote: > > On Jan 20, 2010, at 9:42 AM, William A. Rowe Jr. wrote: > >> On 1/20/2010 10:01 AM, Sander Temme wrote: >>> >>> And the %lld printf formatter on MacOSX... small, but it does break a test. >> >> That's fixed on the branch, or no? About to tag 1.4.2. > > I'm about to get on a flight, will test once under way. Sorry - missed it. It's tagged, and we'll move along to fix more bugs with the next release. I don't see a whole lot of reason to hold up a release if nobody on the platform has cared to patch a known flaw in 3 months :) We can sort of count on a 1.4.3 soon, I have a request to fix a win32 edge case for subversion that didn't merit destabilizing this release, but would be a great fix to have for windows users, before subversion 1.7 is released.
Re: next alpha, this Wednesday?
On Jan 20, 2010, at 9:42 AM, William A. Rowe Jr. wrote: > On 1/20/2010 10:01 AM, Sander Temme wrote: >> >> And the %lld printf formatter on MacOSX... small, but it does break a test. > > That's fixed on the branch, or no? About to tag 1.4.2. I'm about to get on a flight, will test once under way. S. -- Sander Temme scte...@apache.org PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF smime.p7s Description: S/MIME cryptographic signature
Re: DO NOT REPLY [Bug 48359] Buffer overflow related to setting RequestHeader
On 20 Jan 2010, at 10:47, bugzi...@apache.org wrote: > https://issues.apache.org/bugzilla/show_bug.cgi?id=48359 > > --- Comment #7 from Ruediger Pluem 2010-01-20 03:47:50 > CET --- > (In reply to comment #6) >> It's a RFC - if your comment represents a +1, I'm happy to revert and commit >> a patch based on his proposal - I was looking for a sanity check from the >> other committers who had reviewed the original fix. > > Yes, this a +1. -1 for anything that's a candidate for backport to 2.2. Unless someone can convince me otherwise. This raises an issue of what exactly is a subrequest. It's a mix of the parent request and separate (new) fields, and request headers come from the parent. There are probably modules that rely on this (without risking the bug we're dealing with), and they'll break if we change it. Using r->pool is IMHO the lesser of two evils. -- Nick Kew
[mod_fcgid PATCH] catch errors from setuid()/seteuid()
During the last hackathon, Paul was kind enough to run the clang/llvm
static analysis on mod_fcgid
(http://zeus.kimaker.com/~chip/fcgid-scan/). That pointed out these
setuid()/seteuid() calls that aren't checked prior to running a child.
The error checking itself is simple enough, but there's an ugly aspect
of the implementation that results in trying to switch effective/real
uids multiple times that I worked around. (See the FIXME text in the
patch. I'm not aware of a simple solution, especially one simple
enough to get into 2.3.5) The seteuid() call would otherwise fail on
subsequent invocations for the same child.
IIRC Joe thought that the seteuid() wasn't needed at all, but the
setuid() fails without it on Solaris.
Concerns?
Is there some reason that testing on Linux and Solaris wouldn't be sufficient?
Index: modules/fcgid/fcgid_proc_unix.c
===
--- modules/fcgid/fcgid_proc_unix.c (revision 901320)
+++ modules/fcgid/fcgid_proc_unix.c (working copy)
@@ -160,10 +160,44 @@
return APR_SUCCESS;
}
+static void log_setid_failure(const char *id_type,
+ uid_t user_id)
+{
+char errno_desc[120];
+char errmsg[240];
+
+apr_strerror(errno, errno_desc, sizeof errno_desc);
+apr_snprintf(errmsg, sizeof errmsg,
+ "(%d)%s: mod_fcgid child unable to set %s to %ld\n",
+ errno, errno_desc, id_type, (long)user_id);
+write(STDERR_FILENO, errmsg, strlen(errmsg));
+}
+
+/* FIXME
+ * When using suexec, mod_fcgid registers this child cleanup
+ * with the pool for every FastCGI application as a way of
+ * switching user ids in the child before running suexec.
+ * But such a cleanup will be run for every such pool. IOW,
+ * when creating the third FastCGI application process this
+ * cleanup will run three times before exec-ing.
+ * ap_unixd_config.user_id is invariant, so this does not
+ * cause an operational problem. Note that seteuid(0) only
+ * works the first time, or until the first setuid(), so skip
+ * subsequent calls by checking the uid.
+ */
static apr_status_t exec_setuid_cleanup(void *dummy)
{
-seteuid(0);
-setuid(ap_unixd_config.user_id);
+if (getuid() != ap_unixd_config.user_id) {
+if (seteuid(0) == -1) {
+log_setid_failure("effective uid", 0);
+_exit(1);
+}
+
+if (setuid(ap_unixd_config.user_id) == -1) {
+log_setid_failure("uid", ap_unixd_config.user_id);
+_exit(1);
+}
+}
return APR_SUCCESS;
}
@@ -227,7 +261,10 @@
return errno;
}
-/* Unlink it when process exit */
+/* Register a cleanup to
+ * 1. Unlink the socket when the process exits
+ * 2. (suexec mode only, in the child cleanup) Switch to the configured uid
+ */
if (ap_unixd_config.suexec_enabled) {
apr_pool_cleanup_register(procnode->proc_pool,
procnode, socket_file_cleanup,
Hackathon reminder 1/25 & 1/26
Just a quick reminder that the httpd & incubating Traffic Server hackathon happens at the Google Campus in Mountain View CA this coming Mon and Tues. There's still time to register, just add yourself to the wiki page at; http://wiki.apache.org/httpd/HTTPD%2BTS%2BHackathon and expect the simulcast to occur on the irc.freenode.net #httpd-dev channel, with open questions submitted to this list. See you there!
Re: tagging mod_fcgid-2.3.5 in the next day or so...anything in-progress?
On Wed, Jan 20, 2010 at 12:41 PM, William A. Rowe Jr. wrote: > On 1/20/2010 6:05 AM, Jeff Trawick wrote: >> Let me know if another day or two would help. (I'll look at a couple >> of issues myself to see if they have simple/safe solutions.) > > Actually, if you can tag this by tomorrow afternoon, I should be able > to vote on it before the weekend, otherwise I won't be testing anything > until next week. will-do
Re: next alpha, this Wednesday?
On 1/20/2010 10:01 AM, Sander Temme wrote: > > And the %lld printf formatter on MacOSX... small, but it does break a test. That's fixed on the branch, or no? About to tag 1.4.2.
Re: tagging mod_fcgid-2.3.5 in the next day or so...anything in-progress?
On 1/20/2010 6:05 AM, Jeff Trawick wrote: > Let me know if another day or two would help. (I'll look at a couple > of issues myself to see if they have simple/safe solutions.) Actually, if you can tag this by tomorrow afternoon, I should be able to vote on it before the weekend, otherwise I won't be testing anything until next week. I have nothing to add in 2.3.5, my next ideas are novel enough to be destabilizing, and 2.3.5 should be the bandaid to 2.3.4 regressions.
Re: next alpha, this Wednesday?
On Jan 19, 2010, at 6:14 PM, William A. Rowe Jr. wrote: >> For APR, we can use 1.4.1: >> http://svn.apache.org/repos/asf/apr/apr/tags/1.4.1/ > > Actually 1.4.1 is going not-released, because of a significant hash/table > regression. But I'll make life easy, and tag 1.4.2 [essentially 1.4.1 less > that broken commit] before lunchtime tomorrow. As 1.4.1 saw no -other- > objections than this problem observed at SVN, there's almost no reason > it won't be approved. And the %lld printf formatter on MacOSX... small, but it does break a test. S. -- Sander Temme scte...@apache.org PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF smime.p7s Description: S/MIME cryptographic signature
tagging mod_fcgid-2.3.5 in the next day or so...anything in-progress?
Let me know if another day or two would help. (I'll look at a couple of issues myself to see if they have simple/safe solutions.)
RE: ErrorDocument and ProxyErrorOverride
On Tue, 2010-01-19 at 16:54 -0800, Jeff Tharp wrote: > That does fix this issue, but the browser still gets a 200 instead of a 404. > I know that's caused some confusion for our operation as well. Think about > SEO here -- we have a site behind an Apache-based reverse proxy. We want to > use ProxyErrorOverride and ErrorDocument to make sure we send proper error > pages no matter what the backend application spits out (because often times > its more like a stack trace than a nice human-readable page). Yet, if we > trigger a 404, we send a 200 back, which of course means a search engine > crawler misses the original 404. I need ProxyErrorOverride on to deal with > the 500/503 type errors from the backend. And thus I can't send a nice 404 > from the backend, because the proxy will still override it. So how do I > return a clean 404 in that scenario? Thats annoying - I'd only been looking at the logs since that was what I was worried about, but now you mention it, returning a 404 to the client is just as important :/ Mark. > Jeff Tharp, System Administrator ESRI - Redlands, CA > http://www.esri.com > > -Original Message- > From: Mark Watts [mailto:m.wa...@eris.qinetiq.com] > Sent: Tuesday, January 19, 2010 1:12 AM > To: dev@httpd.apache.org > Subject: Re: ErrorDocument and ProxyErrorOverride > > > > What appears in the log file of the proxy depends on how the access > > log line is configured. > > > > Have a look here > > http://httpd.apache.org/docs/2.2/mod/mod_log_config.html#formats > > > > If you have %s in your CustomLog directive, you'll log the 404. If you > > have %>s you'll log the 200. > > Bingo! > > I do indeed have %>s in my LogFormat, which I'd never noticed before > (ahh, the joys of cut 'n paste) > > Thanks for this. > > Mark. > -- Mark Watts BSc RHCE MBCS Senior Systems Engineer, Managed Services Manpower www.QinetiQ.com QinetiQ - Delivering customer-focused solutions GPG Key: http://www.linux-corner.info/mwatts.gpg signature.asc Description: This is a digitally signed message part
Re: ErrorDocument and ProxyErrorOverride
On Wed, Jan 20, 2010 at 01:54, Jeff Tharp wrote: > That does fix this issue, but the browser still gets a 200 instead of a 404. > I know that's caused some confusion for our operation as well. Think about > SEO here -- we have a site behind an Apache-based reverse proxy. We want to > use ProxyErrorOverride and ErrorDocument to make sure we send proper error > pages no matter what the backend application spits out (because often times > its more like a stack trace than a nice human-readable page). Yet, if we > trigger a 404, we send a 200 back, which of course means a search engine > crawler misses the original 404. I need ProxyErrorOverride on to deal with > the 500/503 type errors from the backend. And thus I can't send a nice 404 > from the backend, because the proxy will still override it. So how do I > return a clean 404 in that scenario? I understand your problem, we had and still have the same problem. I guess, I am not sure though, that you simply cannot get a 404 by configuration only, if the error page is served via an HTTP request. You'll get the 404 in the case in which you have ErrorDocument 404 /local_file_on_the_reverse_proxy. Maybe there could be some ways to still get the 404, but you need code. For example, you could think of hooking proxy_request_status(int *status, request_rec *r). There you could write *status = r->prev->status. (r is the request that fetches the error page and r->prev is the original request that gave you the 404.) Next you will have to prevent apache to think that it got a recursive error (error getting the error document). I don't know how to do that, you'll have to dig into the apache sources. Another line of thought is to hook insert_error_filter and there to set a custom filter that sets the error code of the request that fetches the error document. But I am just brainstorming, I do not know if any of these ideas work. Sorin P.S. I think this discussion is more suited to the modules-dev mailing list.
