Mailing List Archive

> /httpd/patches/for_Apache_0.3/....
>
> B00042-patch.correct_
> B19_spaces_in_content
> B5-patch.UXBITHACKfor
> E24_content_negotiati
> E24b_cn_configuration
> E25_[forE24]custom_re
> E43_G-XBIT.txt
>
> vote * +1
>
> Everything seems to work as advertised.

I'll second this vote with a +1 *.

I have ~10,000 request on 0.3 with these patches added. All is well.
The CN stuff is very cool. Thanks RST.


> robh
> /* OJ owes Kato beer */

Maybe a brewery...

On Wed, 29 Mar 1995, Randy Terbush wrote:
> > /httpd/patches/for_Apache_0.3/....
> >
> > B00042-patch.correct_
> > B19_spaces_in_content
> > B5-patch.UXBITHACKfor
> > E24_content_negotiati
> > E24b_cn_configuration
> > E25_[forE24]custom_re
> > E43_G-XBIT.txt
> >
> > vote * +1
> >
> > Everything seems to work as advertised.
>
> I'll second this vote with a +1 *.
>
> I have ~10,000 request on 0.3 with these patches added. All is well.
> The CN stuff is very cool. Thanks RST.

Compiling now. I'll install this as hyperreal's main server tonight if
all is well. I'll also post my very small patch for adding the
DOCUMENT_ROOT CGI variable.

Oh yeah, the custom_responses patch mentions "HUGE_STRING_LENGTH" in two
places when it should be HUGE_STRING_LEN - just my comment for now. :)

> > robh
> > /* OJ owes Kato beer */
>
> Maybe a brewery...

Actually, if Kato needs a new home I hear the governor's mansion in
Sacramento will be free for the next year or two.

Brian


--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
brian@hotwired.com brian@hyperreal.com http://www.hotwired.com/Staff/brian/

According to the official ballot of our one-man patch election
commission, the candidates are:

B00042-patch.correct_
B19_spaces_in_content
B5-patch.UXBITHACKfor
E24_content_negotiati
E24b_cn_configuration
E25_[forE24]custom_re
E43_G-XBIT.txt

+1 on the lot.

rst

> According to the official ballot of our one-man patch election
> commission, the candidates are:
>
> B00042-patch.correct_
> B19_spaces_in_content
> B5-patch.UXBITHACKfor
> E24_content_negotiati
> E24b_cn_configuration
> E25_[forE24]custom_re
> E43_G-XBIT.txt
>
> +1 on the lot.
>
> rst

Uh yeah, +1, sorry I was distracted. Trying to understand how/why the server
does/doesn't log redirects.

Ay.

> robh
> /* OJ owes Kato beer */

Heyyyyyy, waiiittttt a minute....

........Roy

(who still owes Rob beer) No, not that Rob, the other Rob.

Date: Sun, 2 Apr 95 17:07 BST
From: drtr@ast.cam.ac.uk (David Robinson)

Hmmm... my understanding was that voting periods ended on Friday.
But, comments on the votes (particularly some of the blackballs):

B19: (B19_spaces_in_content_type.txt) -1

This purports to fix a bug in scan_script_headers (due to me); I
do not see a bug. In
out_headers = (char *) realloc(out_headers,
(loh+p+1)*sizeof(char));
'p' already includes the CRLF in the character count, so I don't see why
this should change to loh+p+3.

Ooops... my mistake. If you'd been around to point this out, I'd have
been happy to correct it. However, the change is at least inoccous,
and real easy to back out of; in any case, the patch does work.

B5: (B5-patch.UXBITHACK_improved) -1

Ok, but the patch is conditional on the constant UXBITHACK, whereas the
test in http_get.c is on XBITHACK (sic).
I haven't looked at B5-patch.UXBITHACKforE24.txt; see below.

Ah yes... in my working 0.4 build, I've got the following at the end
of httpd.h:

/* APACHE --- enable some of our own bugfixes (!) */

#define LOGUSER
#ifdef XBITHACK
#define UXBITHACK
#endif

Andrew, I'm not quite sure why you made these conditional --- do you
believe someone might actually want the old behavior (which is pretty
clearly broken)?

P14: (P14.DBMuserauth.txt) -1

Almost there. It needs to prototype get_dbm_pw, and remove the unused
structure 'r' from that routine.

This is all pretty trifling --- there are worse stylistic problems all
over the base code, and even with some of our own stuff. Your code,
for instance, gives me persistent compiler warnings on stream.c and
util.c because you don't cast the result of malloc() --- at least as
serious as a missing prototype. We clearly ought to clean this all up
at some point, but does it really merit a blackball *now*?

rst

> Hmmm... my understanding was that voting periods ended on Friday.

Yup, the plan was for to have all the patches in place by
Thursday, then vote on Thu/Fri.

While we must put correctness ahead of a speedy development cycle,
I agree with rst's comments that the problems David pointed out
are easily fixed, and do not deserve a late veto.

rob.

Hmm, in my absence you've been voting +1 *... I'll have to do something about
that 8-)

Here are my votes on the patches you've already had some votes on:

B42: (B00042-patch.correct_user_logging.v2.txt) -1 (but might change)

I can't reproduce the problem. I'm not sure that adding another static
array is the way to fix this. And the patch need not be conditional on a
compile-time constant; it is a bug fix.

B19: (B19_spaces_in_content_type.txt) -1

This purports to fix a bug in scan_script_headers (due to me); I do not see
a bug. In
out_headers = (char *) realloc(out_headers,
(loh+p+1)*sizeof(char));
'p' already includes the CRLF in the character count, so I don't see why
this should change to loh+p+3.

B5: (B5-patch.UXBITHACK_improved) -1

Ok, but the patch is conditional on the constant UXBITHACK, whereas the
test in http_get.c is on XBITHACK (sic).
I haven't looked at B5-patch.UXBITHACKforE24.txt; see below.

E24: (E24_content_negotiation.txt and E24b_cn_configuration.txt) 0

I haven't had time to work through this patch yet, or any other which
depends on it.

E25: (E25_custom_responses.txt and E25_[forE24]custom_responses.txt) 0

I haven't had time to work through this (large) patch yet, or any other
which depends on it.

E43: (E43_G-XBIT.txt) 0

Patch couldn't apply this patch. I'll try again with the application
of some user intelligence.


And for the other patches I found in the for_Apache_0.3 directory which I
considered:

E44: (E44.nocolon80.txt) +1

Looks OK to me. Note; I've changed the patch file name on hyperreal;
it was E46.nocolon80.txt.

'O1': (O1.bsd4_4.txt) -1/+1

A +1 if this is given a proper patchid. We should probably consider how
to fix warnings on more architectures, but I see no harm in fixing
it just for bsd4.4 right now.

P14: (P14.DBMuserauth.txt) -1

Almost there. It needs to prototype get_dbm_pw, and remove the unused
structure 'r' from that routine.


Patches B15 and B45 need E24 or E25.

David.

> B42: (B00042-patch.correct_user_logging.v2.txt) -1 (but might change)
>
> I can't reproduce the problem.

I can.

1) /DOCUMENT_ROOT/Secure/index.html:

...
<!--#include file="/Admin/default_author.html" -->
...

2) /DOCUMENT_ROOT/Admin/default_author.html:

...
Some random text
...

3) /DOCUMENT_ROOT/Secure/.htaccess:

AuthUserFile /DOCUMENT_ROOT/.htpasswd
AuthGroupfile /dev/null
AuthName ByPassword
AuthType Basic
<limit GET>
require user www
</limit>

4) GET /Secure/

[enter uid/pwd]

5) Look at the log file.

> I'm not sure that adding another static
> array is the way to fix this.

I'm sure that rewriting the logging model would be better.

> And the patch need not be conditional on a
> compile-time constant; it is a bug fix.

Putting it in an #ifdef means you can switch it on and off to test it yourself
really easily. It is a good thing to get into the habit of doing.

> B5: (B5-patch.UXBITHACK_improved) -1
>
> Ok, but the patch is conditional on the constant UXBITHACK, whereas the
> test in http_get.c is on XBITHACK (sic).
> I haven't looked at B5-patch.UXBITHACKforE24.txt; see below.

That's because its a *patch* to 1.3R's way of doing things. Buh, anyway,
you compile the server -DXBITHACK -DUXBITHACK to get it to work.


> David.
>

Cheers,
Ay.

Andrew Wilson URL: http://www.cm.cf.ac.uk/User/Andrew.Wilson/
Elsevier Science, Oxford Office: +44 0865 843155 Mobile: +44 0589 616144

> Ah yes... in my working 0.4 build, I've got the following at the end
> of httpd.h:
>
> /* APACHE --- enable some of our own bugfixes (!) */
>
> #define LOGUSER
> #ifdef XBITHACK
> #define UXBITHACK
> #endif
>
> Andrew, I'm not quite sure why you made these conditional --- do you
> believe someone might actually want the old behavior (which is pretty
> clearly broken)?

What do you think? ;)

No, the #defs are there so that people can switch them off and run back-to-back
tests between two slightly different servers, which is how I found the LOGUSER
bug in the first place. When we're all convinvced that things are working
properly we can dump the #defs and move on.

>
> rst
>

Ay.

Rst is absolutely correct wrt my giving P14 a -1. I should give it a +1.

David.