2.2.x release on the horizon

[Eric Covener]

Hi Steffen, we're about to kick off a 2.2.x release. Can you give
Windows a sniff-test?

Thanks,

-- 
Eric Covener
cove...@gmail.com

Re: 2.2 needs a reviewer for http strict backport ...

[Eric Covener]

Thanks Yann!

2.2 running clean under test suite for me on Linux.

On Wed, Jan 4, 2017 at 2:53 PM, Eric Covener  wrote:
> Halp?
>
> --
> Eric Covener
> cove...@gmail.com



-- 
Eric Covener
cove...@gmail.com

Re: svn commit: r1777390 - /httpd/httpd/branches/2.2.x/STATUS

[Eric Covener]

> + wrowe asks: covener, would you apply? I'd like to have at least a second
> + pair of hands and eyes on merging this to branches/2.2.x and
> + am happy to compare/verify against my working copy.
> +

getting a start on it now

Re: A new release process?

[William A Rowe Jr]

On Tue, Jan 3, 2017 at 1:32 PM, Jacob Champion  wrote:
> On 12/29/2016 08:16 PM, David Zuelke wrote:
>>
>> The tl;dr of this approach is that
>>
>> - any x.y.z release only introduces bugfixes. These releases are done
>> every four weeks, like clockwork. If a fix doesn't make the cut for a
>> release, it'll end up in the next one; - x.y.0 releases, on the other
>> hand, may introduce new features, fixes, and deprecations, but no
>> breaking changes; - x.0.0 releases are the big ones (think PHP 7.0.0
>> in late 2015). where backward compatibility may be changed, etc.
>
> My favorite pieces here are
> - the introduction of a release cadence
> - the separation of bugfix releases from feature releases
>
> Both make life easier -- for us, for users, and for intermediate maintainers
> -- for the reasons you've mentioned.

Consider also our consumers, module authors who rely on us to provide an
API they can design to. All of our subsidiary components, be it apr, or expat,
or pcre, or openssl, or libxml2, or lua, or nghttp2... and most of the
expansions
to httpd, be it python or perl or whatnot, all follow essentially this model.

That isn't how httpd evolved; if you look far far back into the pre-1.0 history
and even through the evolution up to 1.3.12, there wasn't a particular rhyme
or reason other than what the major patch submitters wanted to have happen.
So when pools are introduced, minor version bump - a subversion bump was
obviously not going to cut it. But there was not necessarily any compatibility
at all with previously compiled modules.

This all changed radically with 1.3.14 and 2.0 later on. But the aversion from
back in the 1.2 and 1.3 days to declaring the version minors are precious
resources has not changed.

In the meantime, nghttp2 just published the 18th version minor by year end
last year. And remains solidly compatible due to good design from the get go,
it is still major version 1. Will be interesting to watch and see when enough
API issues have accumulated to ditch major v1 for v2.

We are certainly the enigma, and if there is one reason for developers to
disabused themselves from your API, that is likely making shit up as you
go along. I know a lot of folks frustrated back in the PHP 3-4 days, and it
seems like this solution has worked out very well for the PHP crew. And
there was an equal amount of chaos around Perl's versioning and lifecycle
evolution, but that also seems to be working out pretty well.


>> [...]
>>
>> There are a bunch of technicalities that would need adjusting to fit
>> HTTPD, such as release intervals, release management (for PHP, every
>> x.y.* series has two managers who jointly coordinate releases), etc,
>> but overall the idea is, IMO, worth considering.
>
> Our current process is also fairly labor-intensive, so moving to a quick
> release cadence without simultaneously fixing that is not likely to end up
> well, IMO. There are other discussions on the list for improving our
> automated QA, which I think is probably step 1.

Releases themselves are not terribly challenging. The investment, outside
of the reviews and testing we hope many committers are engaged in,
boils down to less than an hour of actual mechanics, and some time to
prepare specific announcement details or merge back in some sequestered
security patches if those have to be applied.


>> As a, more or less, "outside observer", I happen to think that the
>> current method of voting on finals, instead of a practice of rolling
>> out RCs (that are then left up for testing for at least a week), is
>> fundamentally broken. The 2.4 changelog in particular is littered
>> with releases that were never officially published. For users, that's
>> really confusing.
>
> To a certain extent I think this is something that people get used to
> easily... but for security patches in *particular*, I agree. It's
> disconcerting to hear that CVE 2099-BLAH was fixed in 2.4.Z and then find
> out that 2.4.Z was never released.

We say this, that subversion numbers are cheap, and the lack of 2.4.24
really isn't confusing. But I have to answer questions from at least 15
individuals over the course of the following month when that happens :)
So apparently it might not confuse us, but it does confuse the rest of
the world more than we might expect.


>> For maintainers, it's painful to start over the
>> process each time, and it sometimes leads to months and months
>> without a release that contains certain fixes.
>
> The pain of discarding and restarting your manual tests because someone else
> found a regression on another platform two hours into the release vote comes
> to mind, yes.

On that, adding features to a security release 4 days before tag and roll is
a sure fire path to a bad tag. But features-features-features, gotta collect
them all :)

Looking at a long term solution to the specific quirk that occurred in 2.4.24
and 2.4.25, but the general truth of throwing in one more late last feature

Re: [proposed] 2.4 Maintenance SIG

[Graham Leggett]

On 04 Jan 2017, at 8:37 PM, Jacob Champion  wrote:

>> Can you give us an example of this dead code?
> 
> In modules/ alone (I haven't looked at server/ yet, and don't plan to today), 
> after ignoring build-related files and stripping the svn-diff context, there 
> are twenty *thousand* lines of diff between 2.4 and trunk.
> 
> Now, here's the problem. I can't prove which parts of these are dead and 
> which aren't; I can't prove that *any* of it is dead, really. I only know 
> that there are twenty thousand lines of unreviewed code changes in modules/. 
> That's significant, IMO.

I need to stop you there - none of this code is unreviewed.

“CTR” means “commit then review”, it basically means “write the code, make sure 
it works and integrates properly, then commit it - an email will be sent to the 
project, and people review that diff". Either that code is uncontroversial and 
accepted as is, or it will be challenged.

This review part isn’t a joke. People like Rüdiger Plüm will pick apart and 
judge every line, and you have to defend your changes. Some changes can take a 
long time to get right. This could mean polishing the code as provided, or 
removing it entirely and trying again.

Then, if you want to get to v2.4, you have to pass RTC “review then commit”. 
The change is then reviewed again a second time, this time asking the question 
“will this break existing users in any way”. We don’t change ABI on any stable 
branch (ie v2.4 or below).

> Here are three cherry-picked modules which, to me, seem to be in limbo. 
> Whether we consider this code "dead" or "sleeping" or "on temporary hiatus" 
> or "untouched because it's perfect already" is probably heavily subjective.
> 
> 1) mod_policy
> 
> 1300 lines, added in 2011, last meaningful code change in 2012, no tests that 
> I can find. (However, it does have public documentation.)

That waits for v2.6.

mod_policy is itself a set of tests.

> 2) mod_serf
> 
> 1100 lines, added in 2007, last meaningful code change in 2011, no tests, no 
> public documentation.
> 
> 3) mod_apreq2
> 
> 1000 lines, added in 2011, no meaningful code changes since addition, no 
> tests, no documented public release of libapreq2 since 2010. (It does have 
> public documentation. And it seems like there's history here I don't 
> understand. Maybe it lives somewhere else and was being folded into httpd?)

This is historical.

When v2.4 got released, the changes in mod_serf and mod_apreq2 were deemed to 
be not ready enough for v2.4. When v2.4 got created mod_serf and mod_apreq2 
were not brought forward. If they’re not ready now, the same thing can happen.

> So, there's 3k of the 20k. And remember, my point was that we can fix what I 
> call "dead code" with good old fashioned legwork. I don't advocate trashing 
> trunk, and I don't think having "dead code" is a disaster or a stain on 
> anyone here. I just don't think it's appropriate to spin up an RC from trunk 
> as-is.

Look for the discussion that occurred around November 2011 when v2.4 was 
released:

http://smtp.grokbase.com/t/apache/dev/11bb6wswq2/branched-httpd-2-4-x

We came to the same conclusion then.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

2.2 needs a reviewer for http strict backport ...

[Eric Covener]

Halp?

-- 
Eric Covener
cove...@gmail.com

Re: [proposed] 2.4 Maintenance SIG

[William A Rowe Jr]

To your questions of history;

On Wed, Jan 4, 2017 at 12:37 PM, Jacob Champion  wrote:
>
> 3) mod_apreq2
>
> 1000 lines, added in 2011, no meaningful code changes since addition, no
> tests, no documented public release of libapreq2 since 2010. (It does have
> public documentation. And it seems like there's history here I don't
> understand. Maybe it lives somewhere else and was being folded into httpd?)

Yes, this comes from the Apache Perl (mod_perl) project (also
Apache::Test which they still maintain and release.) As does the Win32
flavor of apxs.

> So, there's 3k of the 20k. And remember, my point was that we can fix what I
> call "dead code" with good old fashioned legwork. I don't advocate trashing
> trunk, and I don't think having "dead code" is a disaster or a stain on
> anyone here. I just don't think it's appropriate to spin up an RC from trunk
> as-is.

Historically, e.g. between 2.0 and 2.2, we have had a series of public
alphas followed by betas labeled 2.1.x-alpha (and -beta), until it was
clean enough to go GA as 2.2.0.

I doubt anyone is thinking of going straight to any 2.6 or 3.0 release
without a series of 2.5-alpha/beta releases. We especially considered
third party module authors when we moved to that schema, so that they
would have the opportunity to adjust for any API or behavior changes
long before a GA announcement, including a feedback loop to ask us to
improve that API for their consumption. Few seem to have taken us up
on that opportunity, but we can always hope for more participation the
next cycle.

Re: [proposed] 2.4 Maintenance SIG

[Jacob Champion]

On 01/04/2017 12:13 AM, Graham Leggett wrote:

On 03 Jan 2017, at 10:47 PM, Jacob Champion  wrote:


I don't feel that trunk is a dead branch, but I do think there is dead code in 
trunk.


Can you give us an example of this dead code?


In modules/ alone (I haven't looked at server/ yet, and don't plan to 
today), after ignoring build-related files and stripping the svn-diff 
context, there are twenty *thousand* lines of diff between 2.4 and trunk.


Now, here's the problem. I can't prove which parts of these are dead and 
which aren't; I can't prove that *any* of it is dead, really. I only 
know that there are twenty thousand lines of unreviewed code changes in 
modules/. That's significant, IMO.


Here are three cherry-picked modules which, to me, seem to be in limbo. 
Whether we consider this code "dead" or "sleeping" or "on temporary 
hiatus" or "untouched because it's perfect already" is probably heavily 
subjective.


1) mod_policy

1300 lines, added in 2011, last meaningful code change in 2012, no tests 
that I can find. (However, it does have public documentation.)


2) mod_serf

1100 lines, added in 2007, last meaningful code change in 2011, no 
tests, no public documentation.


3) mod_apreq2

1000 lines, added in 2011, no meaningful code changes since addition, no 
tests, no documented public release of libapreq2 since 2010. (It does 
have public documentation. And it seems like there's history here I 
don't understand. Maybe it lives somewhere else and was being folded 
into httpd?)


So, there's 3k of the 20k. And remember, my point was that we can fix 
what I call "dead code" with good old fashioned legwork. I don't 
advocate trashing trunk, and I don't think having "dead code" is a 
disaster or a stain on anyone here. I just don't think it's appropriate 
to spin up an RC from trunk as-is.


--Jacob

Re: HTTP/2 frame prioritization not honored

[Kyriakos Zarifis]

Hi Stefan,

Yes, this is making a big, obvservable difference!

Specifically, in all 3 repeats, the high priority stream is now served
100ms after it was received, writing ~100 frames (~1.6MB) of currently
served, lower-priority stream.   (was: 500ms, 500frames(~7.5MB))

In more detail, after the high-prio request is received, 20 more low-prio
frames are served before the h2_task for it logs that it opens the output
for the new stream. Then, another 80 low-frames are served before the
high-prio reply is written. (relevant logs below)

This already has an observable impact on the transition to the next page
the moment I click on the link (goes from 1.5sec to less than 500ms), which
I think is great because this tweak is relevant not just to this scenario,
but to any higher level stream that begins while lower ones are served,
even within a single page.

I'm wondering if the change you made can be pushed harder to make the
switch to the new stream even faster, e.g. avoiding even those 100 frames?


Thanks,
Kyriakos



[Wed Jan 04 10:14:48.577687 2017] [http2:debug] [pid 24864]
h2_stream.c(213): [client] AH03082: h2_stream(0-19): opened

[Wed Jan 04 10:14:48.577758 2017] [http2:debug] [pid 24864]
h2_session.c(452): [client] AH03066: h2_session(0): recv
FRAME[HEADERS[length=39, hend=1, stream=19, eos=1]], frames=13/1486 (r/s)

20 x lower-prio frames:

[Wed Jan 04 10:14:48.577864 2017] [http2:debug] [pid 24864]
h2_session.c(685): [client] AH03068: h2_session(0): sent
FRAME[DATA[length=16275, flags=0, stream=5, padlen=0]], frames=16/1486 (r/s)

[Wed Jan 04 10:14:48.578775 2017] [http2:debug] [pid 24864] h2_task.c(106):
[client] AH03348: h2_task(0-19): open output to GET 204.57.7.200
/preposition/nextnav.html
80 x lower-prio frames:

[Wed Jan 04 10:14:48.578790 2017] [http2:debug] [pid 24864]
h2_session.c(685): [client] AH03068: h2_session(0): sent
FRAME[DATA[length=16275, flags=0, stream=5, padlen=0]], frames=16/1504 (r/s)

[Wed Jan 04 10:14:48.682168 2017] [http2:debug] [pid 24864]
h2_session.c(685): [client] AH03068: h2_session(0): sent
FRAME[HEADERS[length=87, hend=1, stream=19, eos=0]], frames=16/1587 (r/s)

[Wed Jan 04 10:14:48.682186 2017] [http2:debug] [pid 24864]
h2_session.c(685): [client] AH03068: h2_session(0): sent
FRAME[DATA[length=456, flags=1, stream=19, padlen=0]], frames=16/1588 (r/s)

On Wed, Jan 4, 2017 at 9:28 AM, Stefan Eissing  wrote:

> Hi Kyriakos,
>
> sorry for not replying earlier. I could find the issue you ran into,
> namely that mod_http2 is obsessed with the streams it already has and does
> not submit ready responses - until the existing streams are done or pause.
>
> I hope that the new release works much more nicely for you. You find it at
> https://github.com/icing/mod_h2/releases/tag/v1.8.7
>
> Thanks,
>
> Stefan
>
> > Am 02.01.2017 um 23:33 schrieb Kyriakos Zarifis :
> >
> > Thanks Stefan!
> >
> > I just tried the tweaked version. I think I am seeing similar behavior,
> i.e. the higher-prio HTML reply is sent ~500ms after its request is
> received, writing ~500 lower-prio DATA frames (~7.5MB) in the meantime.
> >
> > Before any conclusions, I wanted to make sure I compiled/used the
> tweaked mod properly with my existing Apache/2.4.25 on Ubuntu, since I
> haven't done the process before: I couldn't find details on the right way
> to swap in/out module versions, so I ended up compiling v.1.8.6 and
> pointing to the created mod_http2.so in 
> "/etc/apache2/mods-enabled/http2.load",
> but I'm really not sure that's the right way. The only way I verified it
> was seeing this in /var/log/apache2/error.log:
> >
> > "[http2:info] [pid 24935] AH03090: mod_http2 (v1.8.6-git,
> feats=CHPRIO+SHA256+INVHD, nghttp2 1.17.0), initializing..."
> >
> >
> > Assuming this is an acceptable way to use the tweaked version of the
> module (please let me know if not), where should I share two apache log
> files (one trace for each module version) so you could verify what I see?
> >
> >
> >
> >
> > A few relevant lines from the v1.8.6 run (similar to the stable module,
> AFAICT):
> >
> > [Mon Jan 02 13:59:59.636519 2017] [http2:debug] [pid 26718]
> h2_session.c(439): [client ] AH03066: h2_session(0): recv
> FRAME[HEADERS[length=39, hend=1, stream=19, eos=1]], frames=13/1721 (r/s)
> > [Mon Jan 02 13:59:59.637099 2017] [http2:debug] [pid 26718]
> h2_task.c(106): [client ] AH03348: h2_task(0-19): open output to GET
> /preposition/nextnav.html
> >
> > [ ... continue sending ~500 DATA frames for streams 7-11 ...]
> >
> > [Mon Jan 02 14:00:00.177350 2017] [http2:debug] [pid 26718]
> h2_session.c(661): [client ] AH03068: h2_session(0): sent
> FRAME[HEADERS[length=87, hend=1, stream=19, eos=0]], frames=16/2209 (r/s)
> > [Mon Jan 02 14:00:00.177366 2017] [http2:debug] [pid 26718]
> h2_session.c(661): [client ] AH03068: h2_session(0): sent
> FRAME[DATA[length=456, flags=1, stream=19, padlen=0]], frames=16/2210
> (r/s)8.6
> >
> > [ ... continue sending 

Re: Could/Shouldn't check_header() allow folding?

[Yann Ylavic]

On Wed, Jan 4, 2017 at 6:22 PM, William A Rowe Jr  wrote:
> On Wed, Jan 4, 2017 at 11:12 AM, Yann Ylavic  wrote:
>>
>> This would work for me (on the proxy side), too.
>> The patch (attached) is a bit longer, but still reasonable IMHO.
>> WDYT?
>
> Not understanding if (!header->key) { continue; } - why success if there is
> a dead ': UnnamedValue' entry in the output headers table?

That's quite due to apr_table internals, it seems that
apr_table_entry_t->key can be set to NULL there to remove an entry.

Every apr_table_elts() iterator I've seen or worked with does this
check, and so does apr_table_[v]do BTW.

> Unclear why
> apr_table_do needed to be re-implemented.

Because we can't change the value in an apr_table_do callback.
(Note this patch won't change the value in place, but will make it
point to another buffer).

>
> Does it make sense to smash surrounding whitespace here? A single SP
> is going to have the same semantic value as multiple SP/TAB characters.

Makes sense, will do it on commit ("end += 3 + strspn(end + 3, "\t
");") if no one is against the whole change.

Re: HTTP/2 frame prioritization not honored

[Stefan Eissing]

Hi Kyriakos,

sorry for not replying earlier. I could find the issue you ran into, namely 
that mod_http2 is obsessed with the streams it already has and does not submit 
ready responses - until the existing streams are done or pause.

I hope that the new release works much more nicely for you. You find it at 
https://github.com/icing/mod_h2/releases/tag/v1.8.7

Thanks,

Stefan

> Am 02.01.2017 um 23:33 schrieb Kyriakos Zarifis :
> 
> Thanks Stefan!
> 
> I just tried the tweaked version. I think I am seeing similar behavior, i.e. 
> the higher-prio HTML reply is sent ~500ms after its request is received, 
> writing ~500 lower-prio DATA frames (~7.5MB) in the meantime.
> 
> Before any conclusions, I wanted to make sure I compiled/used the tweaked mod 
> properly with my existing Apache/2.4.25 on Ubuntu, since I haven't done the 
> process before: I couldn't find details on the right way to swap in/out 
> module versions, so I ended up compiling v.1.8.6 and pointing to the created 
> mod_http2.so in "/etc/apache2/mods-enabled/http2.load", but I'm really not 
> sure that's the right way. The only way I verified it was seeing this in 
> /var/log/apache2/error.log:
> 
> "[http2:info] [pid 24935] AH03090: mod_http2 (v1.8.6-git, 
> feats=CHPRIO+SHA256+INVHD, nghttp2 1.17.0), initializing..." 
> 
> 
> Assuming this is an acceptable way to use the tweaked version of the module 
> (please let me know if not), where should I share two apache log files (one 
> trace for each module version) so you could verify what I see?
> 
> 
> 
> 
> A few relevant lines from the v1.8.6 run (similar to the stable module, 
> AFAICT):
> 
> [Mon Jan 02 13:59:59.636519 2017] [http2:debug] [pid 26718] 
> h2_session.c(439): [client ] AH03066: h2_session(0): recv 
> FRAME[HEADERS[length=39, hend=1, stream=19, eos=1]], frames=13/1721 (r/s)
> [Mon Jan 02 13:59:59.637099 2017] [http2:debug] [pid 26718] h2_task.c(106): 
> [client ] AH03348: h2_task(0-19): open output to GET  
> /preposition/nextnav.html
> 
> [ ... continue sending ~500 DATA frames for streams 7-11 ...]
> 
> [Mon Jan 02 14:00:00.177350 2017] [http2:debug] [pid 26718] 
> h2_session.c(661): [client ] AH03068: h2_session(0): sent 
> FRAME[HEADERS[length=87, hend=1, stream=19, eos=0]], frames=16/2209 (r/s)
> [Mon Jan 02 14:00:00.177366 2017] [http2:debug] [pid 26718] 
> h2_session.c(661): [client ] AH03068: h2_session(0): sent 
> FRAME[DATA[length=456, flags=1, stream=19, padlen=0]], frames=16/2210 (r/s)8.6
> 
> [ ... continue sending streams 11 onwards ...]
> 
> Thanks!
> 
> On Sat, Dec 31, 2016 at 5:43 AM, Stefan Eissing 
>  wrote:
> Hi Kyriakos,
> 
> have a look at https://github.com/icing/mod_h2/releases/tag/v1.8.6
> 
> That version flushes when at least 2 TLS records are ready to send. Also, 
> frame sizes are now aligned to TLS record sizes. So they are influenced by 
> the H2TLSWarmUpSize and H2TLSCoolDownSecs settings.
> 
> Additionally, and highly experimental, I added H2TLSFlushCount to configure 
> the number of records to flush. You may play around with it (default is 2) in 
> your scenarios.
> 
> I hope that this reduces buffering and makes the server more (another word 
> for agile, pls) to stream changes. Please let me know if that had any effect 
> on your tests.
> 
> Thanks,
> 
> Stefan
> 
> > Am 29.12.2016 um 12:40 schrieb Kyriakos Zarifis :
> >
> > That means the images should get a minim of ~30% of the available bandwidth 
> > as long as they have data. My reading.
> >
> > Right. Makes sense.
> 
> Stefan Eissing
> 
> bytes GmbH
> Hafenstrasse 16
> 48155 Münster
> www.greenbytes.de
> 
> 

Stefan Eissing

bytes GmbH
Hafenstrasse 16
48155 Münster
www.greenbytes.de

Re: Could/Shouldn't check_header() allow folding?

[William A Rowe Jr]

On Wed, Jan 4, 2017 at 11:12 AM, Yann Ylavic  wrote:
>
> This would work for me (on the proxy side), too.
> The patch (attached) is a bit longer, but still reasonable IMHO.
> WDYT?

Not understanding if (!header->key) { continue; } - why success if there is
a dead ': UnnamedValue' entry in the output headers table? Unclear why
apr_table_do needed to be re-implemented.

Does it make sense to smash surrounding whitespace here? A single SP
is going to have the same semantic value as multiple SP/TAB characters.

Re: Could/Shouldn't check_header() allow folding?

[Yann Ylavic]

On Wed, Jan 4, 2017 at 2:21 PM, William A Rowe Jr  wrote:
> On Wed, Jan 4, 2017 at 6:57 AM, Yann Ylavic  wrote:
>> I'm using a (third-party/closed) module which replaces newlines in
>> header values (like base64 encoded PEMs) with obs-fold.
>> That's probably obsolete, but not forbidden per se...
>
> Actually, it is, c.f. 3.2.4 of RFC 7230
>
>[...] This specification deprecates such
>line folding except within the message/http media type
>(Section 8.3.1).  A sender MUST NOT generate a message that includes
>line folding (i.e., that has any field-value that contains a match to
>the obs-fold rule) unless the message is intended for packaging
>within the message/http media type.

OK, pretty clear...

>
>> How about something like:
>
> -1. If we accept obs-fold from CGI, or internally within the
> headers_out, we must
> replace them with a single SP and conform to the spec on the wire.

This would work for me (on the proxy side), too.
The patch (attached) is a bit longer, but still reasonable IMHO.
WDYT?
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c	(revision 1776920)
+++ modules/http/http_filters.c	(working copy)
@@ -690,10 +690,11 @@ struct check_header_ctx {
 };
 
 /* check a single header, to be used with apr_table_do() */
-static int check_header(void *arg, const char *name, const char *val)
+static int check_header(struct check_header_ctx *ctx,
+const char *name, const char **val)
 {
-struct check_header_ctx *ctx = arg;
-const char *test;
+const char *pos, *end;
+char *dst = NULL;
 
 if (name[0] == '\0') {
 ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02428)
@@ -702,12 +703,12 @@ struct check_header_ctx {
 }
 
 if (ctx->strict) { 
-test = ap_scan_http_token(name);
+end = ap_scan_http_token(name);
 }
 else {
-test = ap_scan_vchar_obstext(name);
+end = ap_scan_vchar_obstext(name);
 }
-if (*test) {
+if (*end) {
 ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02429)
   "Response header name '%s' contains invalid "
   "characters, aborting request",
@@ -715,17 +716,55 @@ struct check_header_ctx {
 return 0;
 }
 
-test = ap_scan_http_field_content(val);
-if (*test) {
-ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02430)
-  "Response header '%s' value of '%s' contains invalid "
-  "characters, aborting request",
-  name, val);
-return 0;
+for (pos = *val; *pos; pos = end) {
+end = ap_scan_http_field_content(pos);
+if (*end) {
+if (end[0] != CR || end[1] != LF || (end[2] != ' ' &&
+ end[2] != '\t')) {
+ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02430)
+  "Response header '%s' value of '%s' contains "
+  "invalid characters, aborting request",
+  name, *val);
+return 0;
+}
+if (!dst) {
+*val = dst = apr_palloc(ctx->r->pool, strlen(*val) + 1);
+}
+}
+if (dst) {
+memcpy(dst, pos, end - pos);
+dst += end - pos;
+if (*end) {
+/* turn folding into a single space */
+*dst++ = ' ';
+end += 3;
+}
+}
 }
+if (dst) {
+*dst = '\0';
+}
 return 1;
 }
 
+static int check_headers_table(apr_table_t *t, struct check_header_ctx *ctx)
+{
+const apr_array_header_t *headers = apr_table_elts(t);
+apr_table_entry_t *header;
+int i;
+
+for (i = 0; i < headers->nelts; ++i) {
+header = APR_ARRAY_IDX(headers, i, apr_table_entry_t*);
+if (!header->key) {
+continue;
+}
+if (!check_header(ctx, header->key, (const char **)>val)) {
+return 0;
+}
+}
+return 1;
+}
+
 /**
  * Check headers for HTTP conformance
  * @return 1 if ok, 0 if bad
@@ -738,8 +777,8 @@ static APR_INLINE int check_headers(request_rec *r
 
 ctx.r = r;
 ctx.strict = (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE);
-return apr_table_do(check_header, , r->headers_out, NULL) &&
-   apr_table_do(check_header, , r->err_headers_out, NULL);
+return check_headers_table(r->headers_out, ) &&
+   check_headers_table(r->err_headers_out, );
 }
 
 static int check_headers_recursion(request_rec *r)

Re: [proposed] 2.4 Maintenance SIG

[Jacob Champion]

On 01/04/2017 12:13 AM, Graham Leggett wrote:

On 03 Jan 2017, at 10:47 PM, Jacob Champion  wrote:


I don't feel that trunk is a dead branch, but I do think there is dead code in 
trunk.


Can you give us an example of this dead code?


I'm working to answer this question now. My original assertion is based 
on the fact that a reallyall-build of trunk does not start for the test 
suite without dumping core.


--Jacob

Re: [proposed] 2.4 Maintenance SIG

[Jacob Champion]

On 01/04/2017 08:42 AM, William A Rowe Jr wrote:

On Wed, Jan 4, 2017 at 9:47 AM, Graham Leggett  wrote:

That’s not dead code, that’s just the difference between v2.4 and trunk.


So long as the project chooses not to release it, it sits in a repository DoA.


To a certain extent we'll have to do that, because breaking changes 
can't be backported, and we shouldn't put huge breaking changes on a 
short release cadence. I'm not counting unreleased breaking changes in 
my assertion of "dead code", and I certainly don't think all of the 
2.4->trunk diff is dead and useless.


(I'm currently working on reviewing the diff to answer Graham's 
question; it'll take a little while.)



One of the interesting assertions is that C-T-R -> Trunk, R-T-C Trunk
-> 2.4 is the correct and customary state of contributing to httpd.

At one time this project operated in C-T-R -> Trunk -> Release mode.
It would be productive to return to that model.


I'm not following this jump. If anything, I'm not a fan of CTR to begin 
with for a security-critical code base, *especially* if we're 
considering spinning a release candidate out of a branch that has 
unreviewed patches in it, but that's pulling the conversation in 
directions it doesn't need to go right now. I will bring that back up 
later, in a more appropriate thread.



The goal of R-T-C on 2.4.x is simply to eliminate (unsuccessfully) ABI
breakage and regressions between subversion releases.


The goal of RTC should be to *review* -- to ensure code quality, catch 
problems that aren't caught with tests (ABI, security, etc.), and so on. 
Catching regressions should be the goal of the test suite.


--Jacob

Re: [proposed] 2.4 Maintenance SIG

[William A Rowe Jr]

On Wed, Jan 4, 2017 at 9:47 AM, Graham Leggett  wrote:
> On 04 Jan 2017, at 3:16 PM, William A Rowe Jr  wrote:
>
>>> Can you give us an example of this dead code?
>>
>> svn diff --ignore-properties --no-diff-deleted -x --ignore-all-space
>> https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x/server
>> https://svn.apache.org/repos/asf/httpd/httpd/trunk/server
>> svn diff --ignore-properties --no-diff-deleted -x --ignore-all-space
>> https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x/modules
>> https://svn.apache.org/repos/asf/httpd/httpd/trunk/modules
>
> That’s not dead code, that’s just the difference between v2.4 and trunk.

So long as the project chooses not to release it, it sits in a repository DoA.

One of the interesting assertions is that C-T-R -> Trunk, R-T-C Trunk
-> 2.4 is the correct and customary state of contributing to httpd.

At one time this project operated in C-T-R -> Trunk -> Release mode.
It would be productive to return to that model.

The goal of R-T-C on 2.4.x is simply to eliminate (unsuccessfully) ABI
breakage and regressions between subversion releases.

Re: [proposed] 2.4 Maintenance SIG

[Graham Leggett]

On 04 Jan 2017, at 3:16 PM, William A Rowe Jr  wrote:

>> Can you give us an example of this dead code?
> 
> svn diff --ignore-properties --no-diff-deleted -x --ignore-all-space
> https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x/server
> https://svn.apache.org/repos/asf/httpd/httpd/trunk/server
> svn diff --ignore-properties --no-diff-deleted -x --ignore-all-space
> https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x/modules
> https://svn.apache.org/repos/asf/httpd/httpd/trunk/modules

That’s not dead code, that’s just the difference between v2.4 and trunk.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Could/Shouldn't check_header() allow folding?

[William A Rowe Jr]

On Wed, Jan 4, 2017 at 7:21 AM, William A Rowe Jr  wrote:
> On Wed, Jan 4, 2017 at 6:57 AM, Yann Ylavic  wrote:
>> I'm using a (third-party/closed) module which replaces newlines in
>> header values (like base64 encoded PEMs) with obs-fold.
>
> If we accept obs-fold from CGI, or internally within the
> headers_out, we must
> replace them with a single SP and conform to the spec on the wire.

What about either an optional filter, or simply plug in a module that
only implements a fixup hook, which can be configured on potentially
offending requests to scan and correct the headers_out members?

Such a module could also do something to work around other CTLs,
and invalid header token names, to avoid 500 error conditions and
attempt to still forward some response.

Re: Could/Shouldn't check_header() allow folding?

[William A Rowe Jr]

On Wed, Jan 4, 2017 at 6:57 AM, Yann Ylavic  wrote:
> I'm using a (third-party/closed) module which replaces newlines in
> header values (like base64 encoded PEMs) with obs-fold.
> That's probably obsolete, but not forbidden per se...

Actually, it is, c.f. 3.2.4 of RFC 7230

   [...] This specification deprecates such
   line folding except within the message/http media type
   (Section 8.3.1).  A sender MUST NOT generate a message that includes
   line folding (i.e., that has any field-value that contains a match to
   the obs-fold rule) unless the message is intended for packaging
   within the message/http media type.

> How about something like:
>
> Index: modules/http/http_filters.c
> ===
> --- modules/http/http_filters.c(revision 1776920)
> +++ modules/http/http_filters.c(working copy)
> @@ -701,19 +701,26 @@ static int check_header(void *arg, const char *nam
>  return 0;
>  }
>
> -if (ctx->strict) {
> -test = ap_scan_http_token(name);
> -}
> -else {
> -test = ap_scan_vchar_obstext(name);
> -}
> -if (*test) {
> -ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02429)
> -  "Response header name '%s' contains invalid "
> -  "characters, aborting request",
> -  name);
> -return 0;
> -}
> +test = name;
> +do {
> +if (ctx->strict) {
> +test = ap_scan_http_token(test);
> +}
> +else {
> +test = ap_scan_vchar_obstext(test);
> +}
> +if (*test) {
> +if (test[0] != CR || test[1] != LF || (test[2] != ' ' &&
> +   test[2] != '\t')) {
> +ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, 
> APLOGNO(02429)
> +  "Response header name '%s' contains invalid "
> +  "characters, aborting request",
> +  name);
> +return 0;
> +}
> +test += 3;
> +}
> +} while (*test);
>
>  test = ap_scan_http_field_content(val);
>  if (*test) {
> ?

-1. If we accept obs-fold from CGI, or internally within the
headers_out, we must
replace them with a single SP and conform to the spec on the wire.

Re: [proposed] 2.4 Maintenance SIG

[William A Rowe Jr]

On Wed, Jan 4, 2017 at 2:13 AM, Graham Leggett  wrote:
> On 03 Jan 2017, at 10:47 PM, Jacob Champion  wrote:
>
>> I don't feel that trunk is a dead branch, but I do think there is dead code 
>> in trunk.
>
> Can you give us an example of this dead code?

svn diff --ignore-properties --no-diff-deleted -x --ignore-all-space
https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x/server
https://svn.apache.org/repos/asf/httpd/httpd/trunk/server
svn diff --ignore-properties --no-diff-deleted -x --ignore-all-space
https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x/modules
https://svn.apache.org/repos/asf/httpd/httpd/trunk/modules

Re: [proposed] 2.4 Maintenance SIG

[Jim Jagielski]

> On Jan 4, 2017, at 3:13 AM, Graham Leggett  wrote:
> 
> On 03 Jan 2017, at 10:47 PM, Jacob Champion  wrote:
> 
>> I don't feel that trunk is a dead branch, but I do think there is dead code 
>> in trunk.
> 
> Can you give us an example of this dead code?
> 

I tend to see some of the code related to stuff in httpd-2.4's
STATUS files that are "STALLED" or "BEING WORKED" as not so
much "dead code" but code that needs more eyes on it.

Re: The Version Bump fallacy [Was Re: Post 2.4.25]

[Jim Jagielski]

> On Jan 3, 2017, at 8:04 PM, Noel Butler  wrote:
> 
> On 03/01/2017 23:11, Jim Jagielski wrote:
> 
>> Back in the "old days" we used to provide complimentary builds
>> for some OSs... I'm not saying we go back and do that necessarily,
>> but maybe also providing easily consumable other formats when we
>> do a release, as a "service" to the community might make a lot
>> of sense.
>>  
> 2 years ago it was decided to stop the official -deps (despite they are 
> included in dev still)... now you want to bring it back? (you'd have to if 
> you're going to roll usable binary packages or your "community service" 
> re-built packages are going to be broken)

Nope. Didn't say that. And the inclusion on dev still is known
and even explicitly addressed.

Could/Shouldn't check_header() allow folding?

[Yann Ylavic]

I'm using a (third-party/closed) module which replaces newlines in
header values (like base64 encoded PEMs) with obs-fold.
That's probably obsolete, but not forbidden per se...

How about something like:

Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1776920)
+++ modules/http/http_filters.c(working copy)
@@ -701,19 +701,26 @@ static int check_header(void *arg, const char *nam
 return 0;
 }

-if (ctx->strict) {
-test = ap_scan_http_token(name);
-}
-else {
-test = ap_scan_vchar_obstext(name);
-}
-if (*test) {
-ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02429)
-  "Response header name '%s' contains invalid "
-  "characters, aborting request",
-  name);
-return 0;
-}
+test = name;
+do {
+if (ctx->strict) {
+test = ap_scan_http_token(test);
+}
+else {
+test = ap_scan_vchar_obstext(test);
+}
+if (*test) {
+if (test[0] != CR || test[1] != LF || (test[2] != ' ' &&
+   test[2] != '\t')) {
+ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02429)
+  "Response header name '%s' contains invalid "
+  "characters, aborting request",
+  name);
+return 0;
+}
+test += 3;
+}
+} while (*test);

 test = ap_scan_http_field_content(val);
 if (*test) {
?

Re: Automated tests

[Graham Leggett]

On 31 Dec 2016, at 4:58 AM, William A Rowe Jr  wrote:

> Thinking two things would help.
> 
> Splitting our functional utilities into a libaputil would make it much easier 
> to write the tests that exercise these elements of our code.

Definite +1.

I want to see a C based test suit, the same as apr and apr-util.

> And what I found easiest is a dedicated module to provide diagnostics or 
> tests. When not loaded, they are skipped.

+1.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: [proposed] 2.4 Maintenance SIG

[Graham Leggett]

On 03 Jan 2017, at 10:47 PM, Jacob Champion  wrote:

> I don't feel that trunk is a dead branch, but I do think there is dead code 
> in trunk.

Can you give us an example of this dead code?

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature