Re: New Module: mod_auth_mysql_digest

[Justin Erenkrantz]

--On Friday, November 01, 2002 16:02:45 -0800 Rob Emanuele <[EMAIL PROTECTED]> 
wrote:

So I took the mod_auth_digest code and munged it to use mysql for
authentication.  Swell.  It works great and we're heavily using it.


You might be interested in the new auth provider API which allows precisely 
these sort of things to be handled gracefully without any code duplication. 
Take a look at the current aaa code in httpd-2.0.

If you'd be willing to write this as an auth provider, I'm sure we might be 
interested in integrating it in our tree.  -- justin

New Module: mod_auth_mysql_digest

[Rob Emanuele]

So I took the mod_auth_digest code and munged it to use mysql for
authentication.  Swell.  It works great and we're heavily using it.

I'd like to give it back to the open souce community.  Right now its
just a patch file for mod_auth_digest.c in Apache 2.0.40.  I was
wondering what was the best way to keep the mod_auth_digest code and
mod_auth_mysql_digest code together and up to date with each other?

Things I'm debating...  A home on sourceforge for it if needed.  Its own
site so people can find it.  Ways to get more people using it t find my
bugs.

Thanks for any help and suggestions,

Rob


Robert J. Emanuele
Network Programmer
Cyan Worlds, Inc.
[EMAIL PROTECTED]

Re: workaround for encoded slashes (%2f)

[Roy T. Fielding]

Your patch will simply let the %2F through, but then a later section
of code will translate them to / and we've opened a security hole
in the main server.  I'd rather move the rejection code to the
place where a decision has to be made (like the directory walk),
but I have no time to do it myself.  I think it is reasonable to
allow %2F under some circumstances, but only in content handlers
and only as part of path-info and not within the real directory
structure.


is this a veto?


No, I'm usually very clear when I intend to veto things.


  because i'd like to understand how this
'opens a security hole' available to client-side exploitation
without server-side deficiencies (such as a poorly-coded cgi
script).  if there is none, i don't see why this cannot go
in as a starting point.


I don't know if it does or not, but to be sure you have to check
both the URI parsing code (to be sure it isn't ignoring changes to
the URI that would not normally appear with %2F..) and all of the
modules that do URI access control and rewriting.  A better idea
would be that any URI with %2F be externally redirected to the slash
replacement, because you are changing the meaning of the URI
in a way that must be observed by the client.

Roy

Re: workaround for encoded slashes (%2f)

[Rodent of Unusual Size]

Rodent of Unusual Size wrote:
> 
> based on some offline discussion, i am going to table this
> for now and try suitably modified versions of the %5c attack
> against the patched server.

without a demonstrable technical justification, i still consider
it an invalid veto, but the concerns and considerations raised
by otherbill and roy *are* valid reasons to stop and check this
much more carefully, so i'm withdrawing the patch.

Re: workaround for encoded slashes (%2f)

[Rodent of Unusual Size]

based on some offline discussion, i am going to table this
for now and try suitably modified versions of the %5c attack
against the patched server.

Re: workaround for encoded slashes (%2f)

[Rodent of Unusual Size]

"William A. Rowe, Jr." wrote:
> 
> Yes, it's a veto to introduce a security hole as a 'starting point' that
> someone might get around to cleaning up later.

demonstrate that it is a security hole in the server.
if you cannot demonstrate that this opens the server to
client-side attack, i do not regard the above as a valid
technical justification, and do not recognise the veto.
vetos require technical justification, not opinion.

show me that this opens the server to attack, and i'm there.

Re: cvs commit: httpd-2.0/modules/loggers mod_logio.c

[Greg Stein]

On Fri, Nov 01, 2002 at 06:07:53PM -, [EMAIL PROTECTED] wrote:
>...
>   +++ BaseAddr.ref1 Nov 2002 18:07:52 -   1.22
>   @@ -60,3 +60,4 @@
>mod_authz_groupfile  0x6FB10x0001
>mod_authz_host   0x6FB00x0001
>mod_authz_user   0x6FAF0x0001
>   +mod_logio0x6FAE0x0001

For efficiency purposes, those numbers should be as close together as
possible. There is some cost to the gaps between a module and the next one
(and/or the overall mapped space). If in the future, a module grows and
causes an overlap, then it merely causes a relocation at load time. So there
isn't a worry about that.

I don't recall how to find the size of the module; only that the gaps are to
be avoided. You could also set yourself a periodic six month reminder to
reexamine the sizes.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

RE: book about apache 2?

[Werner Schalk]

What about listing this book at
http://httpd.apache.org/info/apache_books.html,
it seems to be the one which
covers only Apache2 (here in Germany)?

In my opinion books published by
addison&wesley are quite good, or?
But anyway, there a lot more books
available about the apache here
in Germany and they are neither
listed.

Bye,
Werner.

RE: book about apache 2?

[Lars Eilebrecht]

According to Werner Schalk:

> for the german speaking people: According
> to the website addison-wesley.de there
> will be a german (sorry!) book at the
> end of this month.

Other books are listed at
http://httpd.apache.org/info/apache_books.html


ciao...
-- 
Lars Eilebrecht  - "No maintenance": Impossible to fix.
[EMAIL PROTECTED]

Re: workaround for encoded slashes (%2f)

[William A. Rowe, Jr.]

At 11:59 AM 11/1/2002, Rodent of Unusual Size wrote:
>"Roy T. Fielding" wrote:
>> 
>> Your patch will simply let the %2F through, but then a later section
>> of code will translate them to / and we've opened a security hole
>> in the main server.  I'd rather move the rejection code to the
>> place where a decision has to be made (like the directory walk),
>> but I have no time to do it myself.  I think it is reasonable to
>> allow %2F under some circumstances, but only in content handlers
>> and only as part of path-info and not within the real directory
>> structure.
>
>is this a veto?  because i'd like to understand how this
>'opens a security hole' available to client-side exploitation
>without server-side deficiencies (such as a poorly-coded cgi
>script).  if there is none, i don't see why this cannot go
>in as a starting point.

Yes, it's a veto to introduce a security hole as a 'starting point' that
someone might get around to cleaning up later.

If you want to do something this radical, you are going to need to
float it into 2.1-dev.  Then we can at least insist that 2.2 module
authors do the 'right thing' for security, whatever that is.

Anyone looking at unparsed_uri is subject to falling into this hole.
That would be a good place to start looking for newly introduced
vulnerabilities with your patch.

Bill

Re: Splitting out ssl_engine_io.c?

[William A. Rowe, Jr.]

At 04:27 AM 11/1/2002, Justin Erenkrantz wrote:
>I have a distinct feeling that it might ease our sanity if we split the SSL input and 
>output filter code in ssl_engine_io.c into separate files.

Between the input and output, or between the decoded text filter logic
and the bio network filter logic?  There seem to be four ways to slice it.

>Or, am I just nuts?  Perhaps a rename to ssl_engine_filter.c could also be goodness.  
>(If OtherBill is going to revamp the output section, perhaps now is a good time to 
>split it?)  -- justin

I'll keep them together till we are done, at least.  We don't need more
cruft in mod_ssl.h, in fact I was preparing to refactor *that* file into ssl_engine.h
and mod_ssl.h, to the point where mod_ssl.h only contains public interfaces
to our ssl module for folks to 'plug into' ssl.  The other messy guts shouldn't
be propagated into apache2/include/.

Bill

Re: workaround for encoded slashes (%2f)

[Rodent of Unusual Size]

"Roy T. Fielding" wrote:
> 
> Your patch will simply let the %2F through, but then a later section
> of code will translate them to / and we've opened a security hole
> in the main server.  I'd rather move the rejection code to the
> place where a decision has to be made (like the directory walk),
> but I have no time to do it myself.  I think it is reasonable to
> allow %2F under some circumstances, but only in content handlers
> and only as part of path-info and not within the real directory
> structure.

is this a veto?  because i'd like to understand how this
'opens a security hole' available to client-side exploitation
without server-side deficiencies (such as a poorly-coded cgi
script).  if there is none, i don't see why this cannot go
in as a starting point.

Re: Link Not Found

[André Malo]

* André Malo wrote:

> * Rodent of Unusual Size wrote:
> 
>> http://www.apache.org/dist/httpd/patches/apply_to_2.0.43/
>> that appears on
>> http://www.apache.org/dist/httpd/
>> doesn't work = (
> 
> right. But the link appears on http://httpd.apache.org/download.cgi
 ^ *also*

nd
-- 
die (eval q-qq:Just Another Perl Hacker
:-)

# André Malo,  #

Re: UDP Support

[Randall Stewart]

Aymerick:

I am wondering why you want to support UDP in apache... I see you
mention discussions in Aug/Sept.. that i missed.. I will hvae
to go dig in the archive..

My take on UDP is it is very very dangerous to enable http over
UDP.. if UDP were every had a wide scale deployment the internet
itself could easily get to a state of congestion collapse. Especially
considering that most traffic IS http (around 70% last I heard).

Is there some special purpose for this that you want to use it?

Thanks

R

Aymerick Jéhanne wrote:

I want to add UDP Support to Apache, and i can see that I'm not the only one
(there were already discussions about it in August and September on this
list).

Ok, but before starting to code, I want be sure to understand the "Protocol
Modules" logic.

For example, if I want HTTP, Echo and POP3, here is an example of
httpd.conf:

Listen 7
Listen 80
Listen 110


POP3protocol on
...



ProtocolEcho on
...


But now, if I want to serve HTTP, HTTPU, Echo, UDP Echo, and POP3,
I need something like that (with a new directive ListenUDP to setup UDP
Listening Sockets) :

Listen 7
ListenUDP 7
Listen 80
ListenUDP 80
Listen 110


POP3protocol on
...



ProtocolEcho on
...



ProtocolEcho on
...


Ok, now i need your opinion: "Is this logic ok ?".
And if so, we will need to improve  syntax to specify the port
type.

If you are ok with this, I'll start coding this stuff...

Aymerick JEHANNE
[EMAIL PROTECTED]

__
Do You Yahoo!?
Sign up for SBC Yahoo! Dial - First Month Free
http://sbc.yahoo.com






--
Randall R. Stewart
[EMAIL PROTECTED] 815-342-5222 (cell phone)

RE: cvs commit: httpd-2.0/modules/ssl ssl_engine_io.c

[MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1)]

Wow.. lots of good changes. I think the whole ssl-engine_io.c looks a lot
cleaner now (and thanks for the info regarding how the SSL filter works).
I'm still looking through the changes..

As regards splitting ssl_engine_io.c for input/output filter stuff, I'm +1
for it.

-Madhu

> -Original Message-
> From: [EMAIL PROTECTED] [mailto:jerenkrantz@;apache.org]
> Sent: Friday, November 01, 2002 2:37 AM
> To: [EMAIL PROTECTED]
> Subject: cvs commit: httpd-2.0/modules/ssl ssl_engine_io.c
> 
> 
> jerenkrantz2002/11/01 02:37:06
> 
>   Modified:modules/ssl ssl_engine_io.c
>   Log:
>   Add some waypoints to understanding this code
>   
>   Revision  ChangesPath
>   1.90  +8 -0  httpd-2.0/modules/ssl/ssl_engine_io.c
>   
>   Index: ssl_engine_io.c
>   ===
>   RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_io.c,v
>   retrieving revision 1.89
>   retrieving revision 1.90
>   diff -u -u -r1.89 -r1.90
>   --- ssl_engine_io.c 1 Nov 2002 10:19:56 -   1.89
>   +++ ssl_engine_io.c 1 Nov 2002 10:37:06 -   1.90
>   @@ -378,6 +378,9 @@
>return inl;
>}
>
>   +/* This function will read from a brigade and discard the 
> read buckets as it
>   + * proceeds.  It will read at most *len bytes.
>   + */
>static apr_status_t brigade_consume(apr_bucket_brigade *bb,
>apr_read_type_e block,
>char *c, apr_size_t *len)
>   @@ -694,6 +697,7 @@
>
>*len = 0;
>
>   +/* If we have something leftover from last time, try 
> that first. */
>if ((bytes = char_buffer_read(&inctx->cbuf, buf, wanted))) {
>*len = bytes;
>if (inctx->mode == AP_MODE_SPECULATIVE) {
>   @@ -724,6 +728,7 @@
>if (rc > 0) {
>*len += rc;
>if (inctx->mode == AP_MODE_SPECULATIVE) {
>   +/* We want to rollback this read. */
>char_buffer_write(&inctx->cbuf, buf, rc);
>}
>return inctx->rc;
>   @@ -735,6 +740,7 @@
>if (APR_STATUS_IS_EAGAIN(inctx->rc)
>|| APR_STATUS_IS_EINTR(inctx->rc)) {
>if (inctx->block == APR_NONBLOCK_READ) {
>   +/* Already read something, return 
> APR_SUCCESS instead. */
>if (*len > 0) {
>inctx->rc = APR_SUCCESS;
>}
>   @@ -761,6 +767,7 @@
>inctx->rc = APR_EAGAIN;
>
>if (inctx->block == APR_NONBLOCK_READ) {
>   +/* Already read something, return 
> APR_SUCCESS instead. */
>if (*len > 0) {
>inctx->rc = APR_SUCCESS;
>}
>   @@ -962,6 +969,7 @@
>return ssl_io_filter_error(f, bb, status);
>}
>
>   +/* Create a transient bucket out of the decrypted data. */
>if (len > 0) {
>apr_bucket *bucket =
>apr_bucket_transient_create(inctx->buffer, 
> len, f->c->bucket_alloc);
>   
>   
>   
> 

Patch for listen.c

[Randall Stewart]

Hi all:

Attached is a patch to add support for SCTP to apache for those
O/S's that have it.

Now a note about this patch..

The idea behind this is that SCTP is automatically enabled if
available... i.e. there is no flag to turn it on/off.. if you say
listen 80

and have sctp.. you get a listener on both TCP and SCTP port 80.

Now the reason I picked this method is:

a) A port/socket listening is a small amount of overhead...
b) TCP and SCTP are both congestion controlled protocols so
   there should be no threat to the stability of the Big I due
   to the use of SCTP and http (unlike using http with UDP).
c) It seemed like a logical thing to do :-)

Now I suppose the alternative is to expand the config file with
a

listen,tcp: 80
listen,sctp: 80

etc...

But this seemed a bit overkill to avoid listening on one additional
socket...

Comments would be most welcome..

R
--
Randall R. Stewart
[EMAIL PROTECTED] 815-342-5222 (cell phone)

Index: server/listen.c
===
RCS file: /home/cvspublic/httpd-2.0/server/listen.c,v
retrieving revision 1.82
diff -u -r1.82 listen.c
--- server/listen.c 31 Jul 2002 12:44:55 -  1.82
+++ server/listen.c 1 Nov 2002 16:51:37 -
@@ -229,7 +229,8 @@
 apr_socket_t *tmp_sock;
 apr_sockaddr_t *sa;
 
-if ((sock_rv = apr_socket_create(&tmp_sock, APR_INET6, SOCK_STREAM, p)) 
+if ((sock_rv = apr_socket_create_ex(&tmp_sock, APR_INET6, SOCK_STREAM, 
+APR_PROTO_TCP , p)) 
 == APR_SUCCESS &&
 apr_sockaddr_info_get(&sa, NULL, APR_INET6, 0, 0, p) == APR_SUCCESS &&
 apr_bind(tmp_sock, sa) == APR_SUCCESS) { 
@@ -253,6 +254,9 @@
 apr_status_t status;
 apr_port_t oldport;
 apr_sockaddr_t *sa;
+#if APR_HAVE_SCTP
+ap_listen_rec *new2;
+#endif
 
 if (!addr) { /* don't bind to specific interface */
 find_default_family(process->pool);
@@ -280,9 +284,27 @@
 apr_sockaddr_port_get(&oldport, sa);
 if (!strcmp(sa->hostname, addr) && port == oldport) {
 /* re-use existing record */
+#if APR_HAVE_SCTP
+int protocol;
+new = *walk;
+if(new->next) {
+apr_socket_protocol_get(new->next->sd,&protocol);
+}
+if(new->next && (protocol == IPPROTO_SCTP)) {
+/* Next one is a clone of this one so
+ * take it too.
+ */
+*walk = new->next->next;
+new->next->next = ap_listeners;
+} else {
+*walk = new->next;
+new->next = ap_listeners;
+}
+#else
 new = *walk;
 *walk = new->next;
 new->next = ap_listeners;
+#endif
 ap_listeners = new;
 return NULL;
 }
@@ -300,15 +322,44 @@
   addr);
 return "Listen setup failed";
 }
-if ((status = apr_socket_create(&new->sd,
-new->bind_addr->family,
-SOCK_STREAM, process->pool))
+if ((status = apr_socket_create_ex(&new->sd,
+   new->bind_addr->family,
+   SOCK_STREAM, 
+   IPPROTO_TCP , 
+   process->pool))
 != APR_SUCCESS) {
 ap_log_perror(APLOG_MARK, APLOG_CRIT, status, process->pool,
   "alloc_listener: failed to get a socket for %s", addr);
 return "Listen setup failed";
 }
+#if APR_HAVE_SCTP
+new2 = apr_palloc(process->pool, sizeof(ap_listen_rec));
+new2->active = 0;
+if ((status = apr_sockaddr_info_get(&new2->bind_addr, addr, APR_UNSPEC,
+port, 0, process->pool))
+!= APR_SUCCESS) {
+ap_log_perror(APLOG_MARK, APLOG_INFO, status, process->pool,
+  "alloc_listener: SCTP socket not available for listen on %s", 
+addr);
+new->next = ap_listeners;
+ap_listeners = new;
+return NULL;
+}
+if ((status = apr_socket_create_ex(&new2->sd,
+   new2->bind_addr->family,
+   SOCK_STREAM, 
+   IPPROTO_SCTP , 
+   process->pool))
+!= APR_SUCCESS) {
+ap_log_perror(APLOG_MARK, APLOG_INFO, status, process->pool,
+  "alloc_listener: SCTP socket not available for listen on %s", 
+addr);
+new->next = ap_listeners;
+ap_listeners = new;
+return NULL;
 
+}
+new2->next = ap_listeners;
+ap_listeners = new2;
+#endif
 new->next = ap_listeners;

Re: Link Not Found

[André Malo]

* Rodent of Unusual Size wrote:

> http://www.apache.org/dist/httpd/patches/apply_to_2.0.43/
> that appears on
> http://www.apache.org/dist/httpd/
> doesn't work = (

right. But the link appears on http://httpd.apache.org/download.cgi

* Apache 2.0.43 is the best available version 
[...]
  For details see the Official Announcement. Check _here_ to see if any
  patches or [...]

nd
-- 
print "Just Another Perl Hacker";

# André Malo,  #

Re: [PATCH] checking for failures encountered by core_output_filter

[Jeff Trawick]

Justin Erenkrantz <[EMAIL PROTECTED]> writes:

> I think your commits to check c->aborted in various filters should be
> replaced by getting core_input_filter to return APR_ECONNABORTED.

The only filter I changed was the content-length filter.

But yes I would agree that in a commit which fixes our handlers as you
mentioned we also fix the content-length filter to not worry about
c->aborted.

Note that I changed the mod_cgi handler to look at c->aborted as well
as rv after ap_pass_brigade().  In all honesty I don't know who gets
to set c->aborted and whether rv absolutely must be non-APR_SUCCESS if
aborted is set.  But I suspect that mod_cgi wouldn't have to check
both rv and c->aborted to see if an error occurred talking to the
client.

> Namely the following lines in core_output_filter (server/core.c:4002)
> are wrong:
> 
> /* No need to check for SUCCESS, we did that above. */
> if (!APR_STATUS_IS_EAGAIN(rv)) {
> c->aborted = 1;
> }
> 
> /* The client has aborted, but the request was successful. We
>  * will report success, and leave it to the access and error
>  * logs to note that the connection was aborted.
>  */
> return APR_SUCCESS;
> 
> I don't buy that logic at all.  Why should we be returning APR_SUCCESS
> here?  We want to signal an error, so that the filters stop what they
> are doing and exit.

Talk to Ryan :)

> 
> Hmm.  By what I just said for default_handler, if a client aborts,
> that would mean that the handler would be returning with a 500. Hmmm.
> Perhaps we could catch the c->aborted case in the default handler and
> just 'lie' and say that it would have been a 200.  Hmm. Not sure about
> that one.  Thoughts?

I'm kinda stuck thinking that the philosophy should be that we log
whatever was written to the client in the status line, but there are
some implementation details with that one.

> Random thought: define a new HTTP status code which means 'Client got
> the hell out of here, so we stopped early' (207??).  Remember that
> this status code is only going to be presented via the access logs, so
> we really don't need 'support' from HTTP clients.  That could solve
> our problem...
> 
> > Code playing with HTTP status codes should be analyzed to ensure
> > that what is logged is what was written to the client (I guess?)
> > (i.e., some subsequent error doesn't overwrite r->status or
> > whatever is passed to the logger).
> 
> Do we really want to be logging if a filter has an error?

What I meant by "what is logged" is "which HTTP status code ends up in
the access log".  I didn't mean error log.  We'll always write to the
access log (i.e., we'll always run that logging hook) for any request
that we actually read, right?

>   Here's
> another random thought: introduce a logging filter at the top of the
> filter chain which will write a note to the error log if the filters
> return something other than APR_SUCCESS.  This way we don't have to
> explicitly handle that in all of the handlers.

One issue with that is that the filter that encounters the error
really should do the logging so that the most specific information is
available.  If some other code also logs less-specific info, then it
is all for naught.

I'll try not to think about this until Monday :)

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

Re: Link Not Found

[Rodent of Unusual Size]

not acked
-- 
#kenP-)}

Ken Coar, Sanagendamgagwedweinini  http://Golux.Com/coar/
Author, developer, opinionist  http://Apache-Server.Com/

"Millennium hand and shrimp!"
--- Begin Message ---

Hi!

The link:
http://www.apache.org/dist/httpd/patches/apply_to_2.0.43/
that appears on
http://www.apache.org/dist/httpd/
doesn't work = (

Thks, ByE!


--
Cordialmente:

Mario Alberto Cruz Gartner
[EMAIL PROTECTED]

Administracion de Servidores
Red Farallones - Universidad Del Valle

"Yo no sufro de locura... la disfruto a cada minuto!"


--- End Message ---

RE: book about apache 2?

[Werner Schalk]

Hello,

for the german speaking people: According
to the website addison-wesley.de there
will be a german (sorry!) book at the
end of this month. The book seems to be
quite interesting, does somebody know
something about it?

Bye and thanks,
Werner.

Re: cvs commit: httpd-2.0/server request.c

[Jeff Trawick]

Greg Stein <[EMAIL PROTECTED]> writes:

> On Fri, Nov 01, 2002 at 03:27:20AM -, [EMAIL PROTECTED] wrote:
> >...
> >   +++ request.c 1 Nov 2002 03:27:20 -   1.118
> >   @@ -924,6 +924,8 @@
> >/* That temporary trailing slash was useful, now drop it.
> > */
> >if (temp_slash) {
> >   +temp_slash = 0;
> >   +AP_ASSERT(r->filename[filename_len-1] == '/');
> 
> Don't you want
> 
> *temp_slash = '\0';
> 
> ??

As it is, temp_slash is a boolean, not the address of the last char.

But yes it would be better to let temp_slash be NULL or the address of
the last char.

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

Re: book about apache 2?

[Stipe Tolj]

> > i have been asked to recommend a book about apache version 2
> > that would give useful background and reference information
> > for people providing support to customers using it.  so, probably
> > along the lines of 'apache server unleashed' rather than an
> > internals reference.
> >
> > any suggestions?
> 
> Daniel Lopez
> Teach yourself Apache 2 in 24 hours

yep, add me to the fans here :)

Stipe

[EMAIL PROTECTED]
---
Wapme Systems AG

Vogelsanger Weg 80
40470 Düsseldorf

Tel: +49-211-74845-0
Fax: +49-211-74845-299

E-Mail: [EMAIL PROTECTED]
Internet: http://www.wapme-systems.de
---
wapme.net - wherever you are

Re: cvs commit: httpd-2.0/server request.c

[Jeff Trawick]

"William A. Rowe, Jr." <[EMAIL PROTECTED]> writes:

> Folks, this looks wrong after consideration.  If someone is familiar
> with the Linux gcc optimizer, please see my last comments in 
> 
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14147
> 
> I'm starting to feel like the optimizer bit us.

That was the first thing I thought.  However, since that loop does not
fit on one screen it took a bit more concentration to see what was
happening.  Those gotos make it a little more interesting :)

do {
int temp_slash=0;

if (condition met) {
temp_slash=1;
}

...

minimerge:

...

minimerge2:

if (temp_slash) {
r->filename[--filename_len] = '\0';
}



if (matches) {
 if (last_walk->matched == sec_ent[sec_idx]) {
 
 goto minimerge; X
 }
}

} while (thisinfo.filetype == APR_DIR);

The line marked  is one place where we go back to a point
before zapping the last char of r->filename without clearing
temp_slash.  And since filename_len is decremented, we will zap a
different char the next time.

I didn't check if there are other places where can can go back earlier
in the loop without temp_slash being cleared.  But since there are two
goto destinations that seems possible.

It does boggle my mind that we developers apparently never hit this.
I tried to duplicate the mod_dav problem multiple times with no luck.

"William A. Rowe, Jr." <[EMAIL PROTECTED]> writes:

> Folks, this looks wrong after consideration.  If someone is familiar
> with the Linux gcc optimizer, please see my last comments in 
> 
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14147
> 
> I'm starting to feel like the optimizer bit us.
> 
> Bill

> >  Index: request.c
> >  ===
> >  RCS file: /home/cvs/httpd-2.0/server/request.c,v
> >  retrieving revision 1.117
> >  retrieving revision 1.118
> >  diff -u -r1.117 -r1.118
> >  --- request.c 25 Oct 2002 16:38:11 -  1.117
> >  +++ request.c 1 Nov 2002 03:27:20 -   1.118
> >  @@ -924,6 +924,8 @@
> >   /* That temporary trailing slash was useful, now drop it.
> >*/
> >   if (temp_slash) {
> >  +temp_slash = 0;
> >  +AP_ASSERT(r->filename[filename_len-1] == '/');

By the way...  I would like to trust this code enough to make it
AP_DEBUG_ASSERT(), but I guess if you're scared I should be scared too
:)

> >   r->filename[--filename_len] = '\0';

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

Re: dav_new_error

[Greg Stein]

On Fri, Nov 01, 2002 at 10:42:09AM +0100, Sander Striker wrote:
> Thought this might be something for you...
> 
> [08:56]  hey folks, dav_new_error question for the svn'ers
> [08:57]  anyone noticed the int save_errno = errno; bogosity within 
>dav_new_error?
> [08:57]  and has anyone suggested a good way to grab apr_status_t values into 
>that dav_error rec?

It was a shortcut to avoid having to pass the darned thing to every call of
dav_new_error. The "right" answer is to add an apr_status_t to the param
list of dav_new_error.

Note, though, that adding a param to that function is technically an MMN
bump. However, there are only four Apache 2.0 mod_dav backends that I know
about (mod_dav_fs, mod_dav_svn, mod_dav_repos (Catacomb), and mod_dav_psql).
We can easily keep those up to date.

It would be nice to have a finer-grained MMN. In particular, one just for
the DAV API.

I've also been thinking that Justin's new ap_provider stuff should take an
ABI version number in there somewhere. A provider can register an API with a
particular ABI version. A user can ask for providers that match a specific
ABI. Or, (better) a user gets a provider and its ABI. The user can then
adjust its processing based on whatever ABI the provider was designed for.

Doing that to ap_provider would allow us to upgrade the provider-based APIs
without an MMN bump. With sufficient work, the user of a provider could call
the provider in N different ways, based on different incarnations of the
binary interface.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: httpd-2.0/server request.c

[Greg Stein]

On Fri, Nov 01, 2002 at 03:27:20AM -, [EMAIL PROTECTED] wrote:
>...
>   +++ request.c   1 Nov 2002 03:27:20 -   1.118
>   @@ -924,6 +924,8 @@
>/* That temporary trailing slash was useful, now drop it.
> */
>if (temp_slash) {
>   +temp_slash = 0;
>   +AP_ASSERT(r->filename[filename_len-1] == '/');

Don't you want

*temp_slash = '\0';

??

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Splitting out ssl_engine_io.c?

[Justin Erenkrantz]

I have a distinct feeling that it might ease our sanity if we split 
the SSL input and output filter code in ssl_engine_io.c into separate 
files.

Or, am I just nuts?  Perhaps a rename to ssl_engine_filter.c could 
also be goodness.  (If OtherBill is going to revamp the output 
section, perhaps now is a good time to split it?)  -- justin

Re: [PATCH] checking for failures encountered bycore_output_filter

[Justin Erenkrantz]

--On Thursday, October 31, 2002 4:58 PM -0500 Jeff Trawick 
<[EMAIL PROTECTED]> wrote:

default_handler(), which should be returning HTTP status code,
returns whatever ap_pass_brigade() returns.  default_handler()
would have to change too.  Any other handlers as well (In the
thread I pointed to, Ryan claimed that 99% of handlers return
whatever ap_pass_brigade() returned.  So I suspect that some other
handlers need to be changed as well.)


Right, it's been a long-standing problem that this API is completely 
horked (yet another reason I'm not overjoyed with the thought of 
forcing the 2.0.43 module API on all developers until we release a 
2.4/3.0).  OtherBill just suggested to me a solution like changing 
the default_handler line from:

return ap_pass_brigade(r->output_filters, bb);

to:

rv = ap_pass_brigade(r->output_filters, bb);
if (rv != APR_SUCCESS && r->status == HTTP_OK) {
   return HTTP_INTERNAL_SERVER_ERROR;
}
else {
   return r->status;
}

That way, if a filter does set r->status for some reason, we return 
that value, otherwise we return 500.

Handlers should be returning HTTP status codes, while filters should 
be returning APR status codes.  I find myself believe that is the 
right model, because in theory (gah!), the filters could be more than 
just HTTP.  Too bad we can't use enums for one of those status codes 
and wash our hands of the whole deal by relying on type safety.  Oh, 
well.

I think your commits to check c->aborted in various filters should be 
replaced by getting core_input_filter to return APR_ECONNABORTED.

Namely the following lines in core_output_filter (server/core.c:4002) 
are wrong:

   /* No need to check for SUCCESS, we did that above. */
   if (!APR_STATUS_IS_EAGAIN(rv)) {
   c->aborted = 1;
   }

   /* The client has aborted, but the request was successful. We
* will report success, and leave it to the access and error
* logs to note that the connection was aborted.
*/
   return APR_SUCCESS;

I don't buy that logic at all.  Why should we be returning 
APR_SUCCESS here?  We want to signal an error, so that the filters 
stop what they are doing and exit.

Hmm.  By what I just said for default_handler, if a client aborts, 
that would mean that the handler would be returning with a 500. 
Hmmm.  Perhaps we could catch the c->aborted case in the default 
handler and just 'lie' and say that it would have been a 200.  Hmm. 
Not sure about that one.  Thoughts?

Random thought: define a new HTTP status code which means 'Client got 
the hell out of here, so we stopped early' (207??).  Remember that 
this status code is only going to be presented via the access logs, 
so we really don't need 'support' from HTTP clients.  That could 
solve our problem...

Code playing with HTTP status codes should be analyzed to ensure
that what is logged is what was written to the client (I guess?)
(i.e., some subsequent error doesn't overwrite r->status or
whatever is passed to the logger).


Do we really want to be logging if a filter has an error?  Here's 
another random thought: introduce a logging filter at the top of the 
filter chain which will write a note to the error log if the filters 
return something other than APR_SUCCESS.  This way we don't have to 
explicitly handle that in all of the handlers.

clear as mud?


Oh, sure.  -- justin