Re: Dealing with Regressions

2006-05-09 Thread Joe Orton
On Mon, May 08, 2006 at 10:12:58AM -0400, Jim Jagielski wrote:
> 
> On May 8, 2006, at 7:36 AM, Nick Kew wrote:
> 
> >OK, we all know we get some embarrassing regressions in our new
> >releases.  PR#39490 in 1.3.35.
> 
> That is an unexpected and unwelcome regression. If I had
> known about it I would have vetoed the patch. I'd be willing
> to actually release a 1.3.36 simply to address that.

I don't think any of this should be unexpected.  All of the recent 
1.3.x/2.0.x regressions I can think of have been caused by adding new 
features.  New features come at an increased risk of regressions - live 
with it, or stop doing it! :)

joe




Re: 1.3.35 status: Was: Re: Dealing with Regressions

2006-05-08 Thread William A. Rowe, Jr.

Jim Jagielski wrote:


In any case, this is notice that I will be RM for 1.3.36


Thanks for voulenteering :)

My point in the earlier thread is that I'll make a 1.3.36 binary for
windows, leave it on the mirror for three days.  But will then pull
down that binary, and notes about 1.3, leaving users to look
>>> over there  if they want to download this flavor for windows in
particular.  I think folks know I'm not an anti-1.3 bigot, but the
bugs remaining in httpd 1.3 windows are either beyond solving, or
will never get the attention, and we have a supported solution.

Bill


1.3.35 status: Was: Re: Dealing with Regressions

2006-05-08 Thread Jim Jagielski

There are several bug reports due to the updated Include
code (eg: 39490, 39513 and 39516). Looks like we got bitten
by what we usually get bitten by: last minute updates :(

My plan is that we release 1.3.36 very soon to address this.
I'd prefer a fix that (1) doesn't replicate lots of code
and (2) causes no more regressions, but if that takes
too much time, I will simply release 1.3.36 with the reverse
applied.

I will see what I can come up with.

In any case, this is notice that I will be RM for 1.3.36



Re: Dealing with Regressions

2006-05-08 Thread Jim Jagielski

What I did is reversed #396294... this keeps HEAD
in a non-regressed state and gives us time to
come up with a full fix.


Re: Dealing with Regressions

2006-05-08 Thread William A. Rowe, Jr.

+1 here as well, but could you attach this to both bugs and get confirmation
that the double-free and wildcards again act as expected for our reporters
after applying the patch?

Bill

Jim Jagielski wrote:

I'd prefer just fixing the regression and keeping
both behaviors :)

+1

On May 8, 2006, at 10:45 AM, Colm MacCarthaigh wrote:


On Mon, May 08, 2006 at 10:12:58AM -0400, Jim Jagielski wrote:


That is an unexpected and unwelcome regression.



Yep, my bad, I never had such a block in my testing largely because I
didn't even know 1.3.x had that feature, *sigh*, it's not even
documented and I can't see it in a changelog and it didn't have that
functionality when I first wrote that patch.


If I had known about it I would have vetoed the patch. I'd be willing
to actually release a 1.3.36 simply to address that.



Patch attached, courtesy of Matthijs van der Klip. Or simply revert  the
whole change.

--
Colm MacCárthaighPublic Key: colm [EMAIL PROTECTED]





.





Re: Dealing with Regressions

2006-05-08 Thread Jim Jagielski

I'd prefer just fixing the regression and keeping
both behaviors :)

+1

On May 8, 2006, at 10:45 AM, Colm MacCarthaigh wrote:


On Mon, May 08, 2006 at 10:12:58AM -0400, Jim Jagielski wrote:

That is an unexpected and unwelcome regression.


Yep, my bad, I never had such a block in my testing largely because I
didn't even know 1.3.x had that feature, *sigh*, it's not even
documented and I can't see it in a changelog and it didn't have that
functionality when I first wrote that patch.


If I had known about it I would have vetoed the patch. I'd be willing
to actually release a 1.3.36 simply to address that.


Patch attached, courtesy of Matthijs van der Klip. Or simply revert  
the

whole change.

--
Colm MacCárthaighPublic Key: colm 
[EMAIL PROTECTED]






Re: Dealing with Regressions

2006-05-08 Thread Graham Leggett
On Mon, May 8, 2006 1:36 pm, Nick Kew wrote:

> We should do better than leaving the users to rediscover and deal with
> regressions themselves, once we know there's a problem.  Can I suggest
> an Errata page, to list *all* known regressions in current/recent
> versions,
> linked from the main page alongside "Download/New Features/Changelog"?

In theory, making the entries in Changelog in an easier-to-find location
(for example an Errata page as you suggest), coupled with some
release-early-release-often should help prevent this sort of thing.

Regards,
Graham
--




Re: Dealing with Regressions

2006-05-08 Thread Colm MacCarthaigh
On Mon, May 08, 2006 at 03:45:21PM +0100, Colm MacCarthaigh wrote:
> Yep, my bad, I never had such a block in my testing largely because I
> didn't even know 1.3.x had that feature, *sigh*, it's not even
> documented and I can't see it in a changelog and it didn't have that
> functionality when I first wrote that patch. 

Turns out it is documented. I should have caught it from inspection
anyway.

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]


Re: Dealing with Regressions

2006-05-08 Thread Colm MacCarthaigh
On Mon, May 08, 2006 at 10:12:58AM -0400, Jim Jagielski wrote:
> That is an unexpected and unwelcome regression. 

Yep, my bad, I never had such a block in my testing largely because I
didn't even know 1.3.x had that feature, *sigh*, it's not even
documented and I can't see it in a changelog and it didn't have that
functionality when I first wrote that patch. 

> If I had known about it I would have vetoed the patch. I'd be willing
> to actually release a 1.3.36 simply to address that.

Patch attached, courtesy of Matthijs van der Klip. Or simply revert the
whole change.

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
diff -Bru apache_1.3.35/src/main/http_config.c 
apache_1.3.35-include/src/main/http_config.c
--- apache_1.3.35/src/main/http_config.c2006-05-05 12:00:14.0 
+0200
+++ apache_1.3.35-include/src/main/http_config.c2006-05-05 
12:00:39.0 +0200
@@ -1214,16 +1214,19 @@
 return strcmp(f1->fname,f2->fname);
 }
 
-CORE_EXPORT(void) ap_process_include_config(server_rec *s, char *fname, pool 
*p, pool *ptemp, 
+CORE_EXPORT(void) ap_process_include_config(server_rec *s, char *fname, pool 
*p, pool *ptemp,
cmd_parms *parms)
 {
 const char *errmsg;
 struct stat finfo;
-
+int ispatt;
 fname = ap_server_root_relative(p, fname);
 
-if (stat(fname, &finfo) == -1)
+if (!(strcmp(fname, ap_server_root_relative(p, RESOURCE_CONFIG_FILE))) ||
+   !(strcmp(fname, ap_server_root_relative(p, ACCESS_CONFIG_FILE {
+   if (stat(fname, &finfo) == -1)
return;
+}
 
 /* 
  * here we want to check if the candidate file is really a
@@ -1231,12 +1234,36 @@
  * horrible loops).  If so, let's recurse and toss it back into
  * the function.
  */
-if (ap_is_rdirectory(fname)) {
+ispatt = ap_is_fnmatch(fname);
+if (ispatt || ap_is_rdirectory(fname)) {
DIR *dirp;
struct DIR_TYPE *dir_entry;
int current;
array_header *candidates = NULL;
fnames *fnew;
+   char *path = ap_pstrdup(p,fname);
+   char *pattern = NULL;
+
+if(ispatt && (pattern = strrchr(path, '/')) != NULL) {
+*pattern++ = '\0';
+if (ap_is_fnmatch(path)) {
+fprintf(stderr, "%s: wildcard patterns not allowed in Include "
+"%s\n", ap_server_argv0, fname);
+exit(1);
+}
+
+if (!ap_is_rdirectory(path)){ 
+fprintf(stderr, "%s: Include directory '%s' not found",
+ap_server_argv0, path);
+exit(1);
+}
+if (!ap_is_fnmatch(pattern)) {
+fprintf(stderr, "%s: must include a wildcard pattern "
+"for Include %s\n", ap_server_argv0, fname);
+exit(1);
+}
+}
+
 
/*
 * first course of business is to grok all the directory
@@ -1244,11 +1271,15 @@
 * for this.
 */
fprintf(stderr, "Processing config directory: %s\n", fname);
+#ifdef NETWARE
dirp = ap_popendir(p, fname);
+#else
+   dirp = ap_popendir(p, path);
+#endif
if (dirp == NULL) {
perror("fopen");
fprintf(stderr, "%s: could not open config directory %s\n",
-   ap_server_argv0, fname);
+   ap_server_argv0, path);
 #ifdef NETWARE
clean_parent_exit(1);
 #else
@@ -1259,9 +1290,11 @@
while ((dir_entry = readdir(dirp)) != NULL) {
/* strip out '.' and '..' */
if (strcmp(dir_entry->d_name, ".") &&
-   strcmp(dir_entry->d_name, "..")) {
+   strcmp(dir_entry->d_name, "..") &&
+(!ispatt ||
+ !ap_fnmatch(pattern,dir_entry->d_name, FNM_PERIOD)) ) {
fnew = (fnames *) ap_push_array(candidates);
-   fnew->fname = ap_make_full_path(p, fname, dir_entry->d_name);
+   fnew->fname = ap_make_full_path(p, path, dir_entry->d_name);
}
}
ap_pclosedir(p, dirp);


Re: Dealing with Regressions

2006-05-08 Thread Nick Kew
On Monday 08 May 2006 14:56, Niklas Edmundsson wrote:
> On Mon, 8 May 2006, Jeff Trawick wrote:
> >> We should do better than leaving the users to rediscover and deal with
> >> regressions themselves, once we know there's a problem.  Can I suggest
> >> an Errata page, to list *all* known regressions in current/recent
> >> versions, linked from the main page alongside "Download/New
> >> Features/Changelog"?
> >
> > +1
> >
> > [REGRESSION] in the changelog might be good too for permanent tracking.
> >
> > Most useful would be to ship a new version within a small amount of
> > time that corrects only regressions and has no other changes (if at
> > all practical; desire is to limit testing concern to speed up
> > delivery).
>
> The same goes for security issues. For apache 2.2.0 there was only a
> note that "the following issues are fixed in the development version"
> or something similar, not exactly what you want as an end user when
> you're affected by that particilar issue.

Agreed.  An Errata page could have up to three subheadings:

* Security [known issues in current or near-current versions]
* Regressions [known issues where we broke something that worked before]
* Critical bugs [the most serious unpatched bugs from bugzilla,
as determined by us rather than some random bod who may or
may not have a clue].

> Either provide a patch, or provide a new version. Neither should be
> too difficult if you limit the scope to only fix the specific issue.

PR#37145 had a patch fairly quickly.  But it wasn't adequately publicised,
so every other poor sod proxying with 2.0.55 had to struggle with
exactly the same thing.  That's what we really should avoid.

Everything on the errata page needs a fix and/or a workaround.
So in the case of PR#37145 we'd have had "Apply the patch"
and "Downgrade to 2.0.54" as options.

-- 
Nick Kew


Re: Dealing with Regressions

2006-05-08 Thread Jim Jagielski


On May 8, 2006, at 7:36 AM, Nick Kew wrote:


OK, we all know we get some embarrassing regressions in our new
releases.  PR#39490 in 1.3.35.


That is an unexpected and unwelcome regression. If I had
known about it I would have vetoed the patch. I'd be willing
to actually release a 1.3.36 simply to address that.


Re: Dealing with Regressions

2006-05-08 Thread Niklas Edmundsson

On Mon, 8 May 2006, Jeff Trawick wrote:


We should do better than leaving the users to rediscover and deal with
regressions themselves, once we know there's a problem.  Can I suggest
an Errata page, to list *all* known regressions in current/recent versions,
linked from the main page alongside "Download/New Features/Changelog"?


+1

[REGRESSION] in the changelog might be good too for permanent tracking.

Most useful would be to ship a new version within a small amount of
time that corrects only regressions and has no other changes (if at
all practical; desire is to limit testing concern to speed up
delivery).


The same goes for security issues. For apache 2.2.0 there was only a 
note that "the following issues are fixed in the development version" 
or something similar, not exactly what you want as an end user when 
you're affected by that particilar issue.


Either provide a patch, or provide a new version. Neither should be 
too difficult if you limit the scope to only fix the specific issue.


/Nikke
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se  | [EMAIL PROTECTED]
---
 I didn't do it nobody saw me you can't prove anything
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=


Re: Dealing with Regressions

2006-05-08 Thread Jeff Trawick

On 5/8/06, Nick Kew <[EMAIL PROTECTED]> wrote:

OK, we all know we get some embarrassing regressions in our new
releases.  PR#39490 in 1.3.35.  Or 2.0.55 being effectively unusable
in a proxy due to PR#37145.  Look at the number of duplicates of
37145 - that's a lot of people with the confidence to report it,
and who didn't find it 'cos it's marked as fixed (despite no fixed
version being released until last week).

We should do better than leaving the users to rediscover and deal with
regressions themselves, once we know there's a problem.  Can I suggest
an Errata page, to list *all* known regressions in current/recent versions,
linked from the main page alongside "Download/New Features/Changelog"?


+1

[REGRESSION] in the changelog might be good too for permanent tracking.

Most useful would be to ship a new version within a small amount of
time that corrects only regressions and has no other changes (if at
all practical; desire is to limit testing concern to speed up
delivery).