You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by André Malo <nd...@perlig.de> on 2003/01/13 01:13:00 UTC

move RFC 1413 out of core?

hmm, I'd like to move the ident code out of core into a separate module, 
say "mod_ident", since it's a feature, which is hardly used in more than 1% 
of the cases, where the apache httpd will be used. But before doing any 
effort: are there any objections on this?

implementation planned that way: mod_ident registers an optional function 
in core, which will be called from the ap_get_remote_logname if available.
(that api function should probably stay in core, if other modules rely on 
it, shouldn't it?)

It also should handle the %...l logformat in mod_log_config, so that this 
specific logformat function will also moved to mod_ident.

Opinions?

nd
-- 
print "Just Another Perl Hacker";

# André Malo, <http://www.perlig.de/> #

Re: Review desired - mod_ident

Posted by André Malo <nd...@perlig.de>.
* Justin Erenkrantz wrote:

> --On Thursday, January 16, 2003 5:09 PM +0100 André Malo
> <nd...@perlig.de> wrote:
> 
>> Another question: I guess, the change does require an MMN bump,
>> doesn't it?
> 
> Well, I'm not real sure if we consider core_dir_config a part of our
> public structures.  I really don't see why a module would be playing
> around in there.

Oh, I didn't think about the core_dir_config. I thought about the (then) 
missing ap_rfc1413 function (and the global timeout variable, anyway).

nd
-- 
print "Just Another Perl Hacker";

# André Malo, <http://www.perlig.de/> #

Re: Review desired - mod_ident

Posted by Greg Ames <gr...@apache.org>.
Justin Erenkrantz wrote:
> --On Thursday, January 16, 2003 5:09 PM +0100 André Malo <nd...@perlig.de> 
> wrote:
> 
>> Another question: I guess, the change does require an MMN bump,
>> doesn't it?
> 
> 
> Well, I'm not real sure if we consider core_dir_config a part of our 
> public structures.  I really don't see why a module would be playing 
> around in there.
> 
> Anyone?  -- justin

I agree with Justin.  IMO any core_*_config is private to the core.  I recall us 
putting stuff in the core_request_config to keep it out of the request_rec for 
example.  Dunno if any modules are being bad.

Greg


Re: Review desired - mod_ident

Posted by Justin Erenkrantz <je...@apache.org>.
--On Thursday, January 16, 2003 5:09 PM +0100 André Malo 
<nd...@perlig.de> wrote:

> Another question: I guess, the change does require an MMN bump,
> doesn't it?

Well, I'm not real sure if we consider core_dir_config a part of our 
public structures.  I really don't see why a module would be playing 
around in there.

Anyone?  -- justin

Re: Review desired - mod_ident

Posted by André Malo <nd...@perlig.de>.
* Justin Erenkrantz wrote:

> --On Wednesday, January 15, 2003 2:32 AM +0100 André Malo
> <nd...@perlig.de> wrote:
> 
>> I'd like to let someone review before committing, because it's at
>> least a  bunch of core changes.
> 
> Looks good upon cursory review.  I think we could optimize some of
> the socket calls (short read/write cases), but that was there before.
> We could always clean that up later.  (sscanf - ugh!)
> 
> My only real complaint is the tabs/formatting.  =)  There seem to be
> lots of whitespace issues.  (Again, there before, but we ought to
> clean it up when we place it in the new location.)

ah well, c&p from rfc1413.c ;-)
I'll run a tab killer and whitespace cleanup before committing. For better 
reproducing of the changes, I think, the code cleanup is better to apply 
later.
Another question: I guess, the change does require an MMN bump, doesn't it?

nd
-- 
Da fällt mir ein, wieso gibt es eigentlich in Unicode kein
"i" mit einem Herzchen als Tüpfelchen? Das wär sooo süüss!
 
                                 -- Björn Höhrmann in darw

Re: Review desired - mod_ident (was: move RFC 1413 out of core?)

Posted by Justin Erenkrantz <je...@apache.org>.
--On Wednesday, January 15, 2003 2:32 AM +0100 André Malo 
<nd...@perlig.de> wrote:

> I'd like to let someone review before committing, because it's at
> least a  bunch of core changes.

Looks good upon cursory review.  I think we could optimize some of 
the socket calls (short read/write cases), but that was there before. 
We could always clean that up later.  (sscanf - ugh!)

My only real complaint is the tabs/formatting.  =)  There seem to be 
lots of whitespace issues.  (Again, there before, but we ought to 
clean it up when we place it in the new location.)  -- justin

Review desired - mod_ident (was: move RFC 1413 out of core?)

Posted by André Malo <nd...@perlig.de>.
* Justin Erenkrantz wrote:

> --On Monday, January 13, 2003 1:13 AM +0100 André Malo <nd...@perlig.de>
> wrote:
> 
>> hmm, I'd like to move the ident code out of core into a separate
>> module,  say "mod_ident", since it's a feature, which is hardly
>> used in more than 1%  of the cases, where the apache httpd will be
>> used. But before doing any  effort: are there any objections on
>> this?
> 
> Sounds decent.  I wouldn't even turn it on by default.
> 
> Of course, this is 2.1+ only.  -- justin

well, been there, done that ;-)

I've attached the full patch against HEAD. Changes to the rfc1413.c/core 
code:

- added IdentityCheckTimeout directive (defaults to 30)
- the ident lookup is plugged via an optional function
- the function ap_rfc1413 no longer exists (to avoid failures, resp. force 
  compile errors if someone trys)
- -> removed rfc1413.c and rfc1413.h
- put mod_ident.c into modules/metadata (ok? or is there a better place?)

I'd like to let someone review before committing, because it's at least a 
bunch of core changes.

(module documentation follows then.)

nd
-- 
s  s^saaaaaoaaaoaaaaooooaaoaaaomaaaa  a  alataa  aaoat  a  a
a maoaa a laoata  a  oia a o  a m a  o  alaoooat aaool aaoaa
matooololaaatoto  aaa o a  o ms;s;\s;s;g;y;s;:;s;y#mailto: #
 \51/\134\137| http://www.perlig.de #;print;# > nd@perlig.de

Re: move RFC 1413 out of core?

Posted by Justin Erenkrantz <je...@apache.org>.
--On Monday, January 13, 2003 1:13 AM +0100 André Malo <nd...@perlig.de> 
wrote:

> hmm, I'd like to move the ident code out of core into a separate
> module,  say "mod_ident", since it's a feature, which is hardly
> used in more than 1%  of the cases, where the apache httpd will be
> used. But before doing any  effort: are there any objections on
> this?

Sounds decent.  I wouldn't even turn it on by default.

Of course, this is 2.1+ only.  -- justin

Re: move RFC 1413 out of core?

Posted by André Malo <nd...@perlig.de>.
* Mads Toftum wrote:

> On Mon, Jan 13, 2003 at 02:19:26AM +0100, André Malo wrote:
>> oh darn, that's a weird definition. Ok, then I think, I'll leave it in
>> mod_log_config (which uses the api function anyway). Thanks for the hint
>> ;-)
>>
> Couldn't this be fixed by either changing the logformat to have - in that
> place, or by just substituting a - whenever ident is not turned on?
> Having a module for it might be a bit overkill though ;)

the functionality of "determine the ident user" is not in mod_log_config 
(of course) but the definition of the "%l" format itself. My first thought 
was to move the format definition also to mod_ident, which is - as Joshua 
pointed out - not a good choice, since the %l format will be needed by 
default.

So it will stay in mod_log_config, which uses the already existing api call 
to get the value. Then it doesn't matter if mod_ident is present or not, 
because if the api call returns no value it will be automatically set to 
"-" in the log (which is the current behaviour, too). The format definition 
code is quite small, so the little overhead in mod_log_config doesn't 
really count.

nd
-- 
"Die Untergeschosse der Sempergalerie bleiben währenddessen aus
 statistischen Gründen geflutet." -- Spiegel Online

Re: move RFC 1413 out of core?

Posted by Mads Toftum <ma...@toftum.dk>.
On Mon, Jan 13, 2003 at 02:19:26AM +0100, André Malo wrote:
> oh darn, that's a weird definition. Ok, then I think, I'll leave it in 
> mod_log_config (which uses the api function anyway). Thanks for the hint 
> ;-)
> 
Couldn't this be fixed by either changing the logformat to have - in that
place, or by just substituting a - whenever ident is not turned on? 
Having a module for it might be a bit overkill though ;)

vh

Mads Toftum
-- 
`Darn it, who spiked my coffee with water?!' - lwall


Re: move RFC 1413 out of core?

Posted by André Malo <nd...@perlig.de>.
* Joshua Slive wrote:

> On Mon, 13 Jan 2003, André Malo wrote:
>> It also should handle the %...l logformat in mod_log_config, so that this
>> specific logformat function will also moved to mod_ident.
> 
> The only problem is that %l, even though it is almost always empty, is
> included in the common log format.  If we create mod_ident and then don't
> include it by default, the CLF won't work in the default config.

oh darn, that's a weird definition. Ok, then I think, I'll leave it in 
mod_log_config (which uses the api function anyway). Thanks for the hint 
;-)

nd
-- 
>kann mir jemand sagen, was genau @-Domains sind?
Ein Mythos. Ein Werbetrick. Verarsche. Nenn es wie du willst...

                 -- Alexandra Buss und Björn Höhrmann in dciwam

Re: move RFC 1413 out of core?

Posted by Joshua Slive <jo...@slive.ca>.
On Mon, 13 Jan 2003, André Malo wrote:
> It also should handle the %...l logformat in mod_log_config, so that this
> specific logformat function will also moved to mod_ident.

The only problem is that %l, even though it is almost always empty, is
included in the common log format.  If we create mod_ident and then don't
include it by default, the CLF won't work in the default config.

Joshua.