Re: Dealing with Regressions
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
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
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
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
+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
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
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
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
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
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
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
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
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).