httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marc Slemko <ma...@znep.com>
Subject Re: cvs commit: apache-1.3/src/support checkgid.c Makefile.tmpl
Date Mon, 29 Oct 2001 16:29:42 GMT
On Mon, 29 Oct 2001, Rodent of Unusual Size wrote:

> Marc Slemko wrote:
> > 
> > It is completely bogus to start adding support
> > programs to check every possible error condition
> 
> Hardly what is happening here.

It is the start of a very ugly trend.  How long will it be before there
is the next bug report "we need a checkxxx because error condition x 
isn't properly reported".

You can do a hell of a lot of damage to the design of a piece of software
by throwing random hacks at it saying "oh, this is only a little bit of
ugly stuff, and it is ok because I don't want to do it the right
way".  Please don't do that to apache any more than already happens.

> 
> > It results in all the error checking code being
> > duplicated in two places
> 
> Since there is NO 'error checking code' for this
> case in the server proper, there is no duplication.

Of course there is!  Apache tries, if it fails it logs an error.  

What exactly do you think:

        if (setgid(ap_group_id) == -1) {
            ap_log_error(APLOG_MARK, APLOG_ALERT, server_conf,
                        "setgid: unable to set group id to Group %u",
                        (unsigned)ap_group_id);
            clean_child_exit(APEXIT_CHILDFATAL);

does?

> > The proper solution is to either have Apache do
> > more checking itself before it daemonizes or change
> > apachectl to handle errors that happen after it
> > daemonizes.
> 
> Since AFAICS this condition can only reliably be
> checked by actually calling setgid(), the first would
> seem to potentially require some major work.  And
> the latter introduces timing issues that are worse
> than the proposed solution.

???

If it is so impossible to have Apache do all config checking before
daemonizing, then what is so horrible about having apachectl start
Apache, then wait to make sure it started?  apachectl could even watch
the logfile for a server-has-started indication that we make sure to
only log after all initialization has been done.

> 
> I disagree with your remarks in any event, considering
> all the noise on the list in the past about 'the way
> for non-httpd apps to do things with/to the config is
> through preprocessing' -- this is just preprocessing
> that doesn't store the config in LDAP, or XML, or
> whatever, but does a band-aid check the server doesn't.

Umh... this is a COMPLETELY different ballpark.  Just how do you 
expect apachectl to parse Apache configuration files anyway?  I guess
the next commit will be code to reparse all apache config files.  

> 
> > It is not to duplicate all the error checking code
> > in external programs.
> 
> Again, that ain't what's happening here, so a little less
> hyperbole, eh?
> 
> I'll look into what's involved in doing the setgid()
> before daemonising, but if it's a hassle on 1.3 I'm
> going to stick with this band-aid solution because it
> *does* address an existing problem.  2.0 can be done better.

Well, sorry to interrupt you in doing what you Will Do no matter what.

This is wrong because:

1. it is non-trivial to parse apache config files from apachectl  How do
you deal with includes?  How do you deal with the fact that there can 
be Group directives in vhosts which may be before or after the main
Group directive?
2. there are a huge array of other problems that people complain about with
similar causes that are not fixed by this and that, if we continue down
this path, will require duplicating a hell of a lot of code for no reason.
Why not just do it right once and fix it for _EVERYTHING_?
3. a lot of other similar problems can not be detected so trivially (eg.
problems in third party module init functions), but
actually require the full server and all it's code to check for.
4. people are perfectly capable of figuring out "hey, the server didn't
work, what's in the error log?", and if they can't then they will have a
lot of other problems using Apache.  This particular problem is one
that generally _ONLY_ happens the very first time someone is trying to 
start Apache, so the "I added some configuration then tried to restart 
Apache, and it said it worked so I went and got drunk, but it actually
failed to restart" complaint is in a completely different ballpark.
This is a very bad route to be going down just to change the behaviour of 
some default installs the first time someone tries to run the server so
that apachectl spits out an error instead of them finding it in the 
error log.

Nearly every post I see complaining about "can't setgid to xxxx"
includes the error message from the logs, the user just has no clue
what to do with it.  If this is a problem, then we need to fix the problem,
not go off on some wild journey...


Mime
View raw message