Re: HTTP/2 and no-longer "experimental"

[Graham Leggett]

On 31 May 2017, at 2:07 PM, Jim Jagielski  wrote:

> There was discussion some time ago about dropping the "experimental"
> tag from our HTTP/2 implementation. It is causing loads of people
> to not use it, as well as allowing for the perpetuation of FUD that
> httpd really doesn't support HTTP/2.
> 
> I'd like for 2.4.26 to be the release that removes that tag. It
> implies a transition to RTC in 2.4 for it, but I think that that
> is workable and realistic at this point...
> 
> Thoughts? Comments?

+1 to no-longer-experimental.

+1 to RTC.

Further ABI breaking improvements can target httpd v2.6.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Ideas from ApacheCon

[Graham Leggett]

On 23 May 2017, at 3:29 PM, Jim Riggs  wrote:

>>> Does that differ from "CacheQuickHandler OFF"?
> 
> 
> This one is a bit quirky and requires some investigation to see the edge 
> cases. There appear to be some situations where Require directives with 
> `CacheQuickHandler off' do not always take effect. Thus, items that have been 
> cached will be served from the cache even if access restrictions would 
> otherwise disallow them.
> 
> We will just need to do some more testing to see when -- and if -- this is 
> actually occurring. I stumbled across it while doing testing/prep for my 
> talk, but did not investigate it thoroughly. (It is entirely possible that I 
> just messed something up in my config and didn't catch it...or forgot a 
> graceful...or something...)
> 
> In my talk, I just told people to "be careful" with access control and CQF 
> off.

When CacheQuickHandler is off, mod_cache acts as a normal content generator, 
and all normal request handling - including the require directives - applies.

If there are cases where this doesn’t apply we need to investigate them and 
either fix them or properly explain the behaviour.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: SSL Policy Definitions

[Graham Leggett]

On 03 May 2017, at 2:01 PM, Stefan Eissing  wrote:

> We seem to all agree that a definition in code alone will not be good enough. 
> People need to be able to see what is actually in effect.

I think we’re overthinking this.

We only need to document the settings that SSLSecurityLevel has clearly in our 
docs, and make sure that "httpd -L” prints out the exact details so no user 
need ever get confused.

> If we let users define their own classes, it could look like this:

Immediately we’ve jumped into functionality that is beyond Mr/Mrs Normal.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: SSL and Usability and Safety

[Graham Leggett]

On 02 May 2017, at 3:19 PM, Stefan Eissing  wrote:

> How can we help Mr and Ms Normal to stay up to date on these things?
> 
> - We cannot rewrite their config unasked. We need to be backward compatible.
> - Our defaults nowadays are dangerously unsafe, so users MUST do their own 
> settings.
> 
> I advocate that we need (yet another!) SSL directive where administrators can 
> declare their *intent*.
> 
> A. "I want my site safe and usable with modern browsers!"
> B. "I want a safe setting, but people with slightly out-dated clients should 
> be served as well."
> C. "I sadly need compatibility to some very old clients.”

This makes a lot of sense, and there is a lot of precedent for this.

AWS load balancers take an “intent” policy string based on a date, with the 
option of a “default” value:

http://docs.aws.amazon.com/elasticloadbalancing/latest/classic/ssl-config-update.html
--
|  DescribeLoadBalancerPolicies  |
++
|   PolicyName   |
++
|  ELBSecurityPolicy-2016-08 |
|  ELBSecurityPolicy-2015-05 |
|  ELBSecurityPolicy-2015-03 |
|  ELBSecurityPolicy-2015-02 |
|  ELBSecurityPolicy-2014-10 |
|  ELBSecurityPolicy-2014-01 |
|  ELBSecurityPolicy-2011-08 |
|  ELBSample-ELBDefaultCipherPolicy  |
|  ELBSample-OpenSSLDefaultCipherPolicy  |
++
Implementation wise, we could have a directive that is used to select the 
default values of various parameters, for example:

SSLSecurityLevel latest

or

SSLSecurityLevel 2017-05

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: drop experimental from http2 for 2.4.next?

[Graham Leggett]

On 17 Apr 2017, at 10:24 AM, Stefan Eissing  
wrote:

> These modules, they grow up so fast...
> 
> For the project, it would be good to drop that "experimental" and 
> treat HTTP/2 as an integral part of httpd. Not only for political
> posturing (which is important), but also for very technical reasons.
> 
> Looking at https://w3techs.com/technologies/details/ce-http2/all/all
> one can see that HTTP/2 is used by 13% of all sites, which is almost
> double from 1 year ago. Firefox telemetry reports HTTP/2.0 now 
> on 35% of all responses received.
> 
> What needs to be done?

I would say what needs to be done is make it a solid and viable HTTP2 
implementation, declare it non-experimental and let it fly.

> From what I saw in the last two years, these 
> are key areas to improve:
> 
>  1. separation of semantics and serialisation
>  2. connections with >1 requests simultaneously
> 
> mod_http need to spin off a mod_http1 with the parts that read
> and write headers, handle chunked encoding in requests
> and responses. etc.
> 
> mpm needs facilities for processing slave connections and assign
> its resources to slave/master connections in fair and performant
> ways.

These are great to have for httpd v2.6 - let’s develop these above there.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: drop experimental from http2 for 2.4.next?

[Graham Leggett]

On 15 Apr 2017, at 11:02 PM, Eric Covener  wrote:

> Hi everyone, shall we drop experimental from mod_http2 for 2.4.next?

+1.

> We could drop it and keep CTR.

It can’t be not-experimental and CRT at the same time.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: json, rather sooner than later

[Graham Leggett]

On 31 Mar 2017, at 2:51 PM, Stefan Eissing  wrote:

> Hey, I'm currently evaluating some addition to httpd that needs to 
> parse/serialize JSON. 
> I know not everyone is a fan of that format, but some protocols do require it 
> (I try to 
> spare us a discussion about "better" formats :-).
> 
> Does anyone have ideas/preferences about bringing JSON into httpd (and 
> eventually into
> the APR at some later point in time)? I have not found a good lib that is 
> available 
> everywhere. Several have MIT license, but I think we cannot somehow bring 
> those under
> the Apache umbrella, right?
> 
> If anyone has a recommendation or more experience in this then me, I'd be 
> glad for any
> guidance.

I would model this the same way we model XML - we have a compatibility layer in 
APR, and support one (maybe more) json parsers.

Definite +1 to the concept, and given that YAML is now the new hipster cool, we 
might do that at the same/similar time.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Reset out x.minor.z definition of 'minor' at httpd?

[Graham Leggett]

On 20 Jan 2017, at 10:40 PM, Jim Jagielski  wrote:

> In fact, I'd announce 2.5-alpha "immediately" as what's
> in trunk... with a set schedule that 2.6-Beta1 is tagged 2
> months later and a goal that 2.6-GA be announced at
> ApacheCon Miami.
> 
> But this all implies that 2.5/2.6 not be the huge restructure/re-
> factor envisioned by some.

I’m keen to see v2.5-alpha out soon.

There will be a drive to ensure the new goodness on trunk is stable and fixed, 
and this is a good thing.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Reset out x.minor.z definition of 'minor' at httpd?

[Graham Leggett]

On 20 Jan 2017, at 4:29 PM, William A Rowe Jr  wrote:

>>> Right now, there are a number of gratuitous breaking changes on trunk
>>> that change binary ABI. I'm considering a trunk fork to preserve these
>>> changes (branches/3.x-future/?) and reverting every change to trunk/
>>> which was otherwise a no-op. Namespace, decoration, anything that
>>> prevents a user from loading a 2.4.x module in 2.next. And then we
>>> can look at what is a valid reason to take us to 3.0.
>> 
>> -1 (veto).
> 
> To be clear, procedural decisions can't be vetoed. But specific code
> changes can be vetoed.
> 
> I can veto and revert each individual commit on trunk which breaks the
> API or ABI in an unnecessary manner, pointing at that specific
> breakage, and invite the committer to propose the change using the
> existing interfaces.

This would be an abuse of the veto mechanism - the veto is to protect the 
project, not force other people to do work on your behalf, whether “invited” to 
or not.

In a case like this, what you’d be expected to do is write and commit patches 
that fixed any cases you felt was unnecessary yourself, and have those patches 
reviewed by the rest of the project in the normal way.

> In light of your objection, I won't proceed with a preservation
> branch, but take the veto knife directly to trunk.

As per our rules a veto requires a technical justification. Breaking changes 
are allowed on trunk, and so the fact the change is breaking is not in itself 
sufficient justification for a veto.

>> As you are well aware, the jump from v2.0 to v2.2, from v2.2 to v2.4, and 
>> the jump from v2.4 to v2.6 involves breaking changes, and this is a well 
>> established and stable pattern that is well understood by our end users. The 
>> “problem” you’re trying to solve does not exist.
> 
> There is nothing in httpd which is stable, and 2.4.x certainly hasn't
> been well established. Not even 50% of Apache httpd deployments use
> 2.4.x almost four years later, and 2.4.25 looks so considerably
> different than 2.4.1 that it cannot be called a maintenance branch. It
> is impossible to identify from "2.4" what point in its evolution is
> causing a reported issue without knowing the subversion. A handful of
> 2.4.x releases walked out the door without some significant regression
> - not a proud track record. So this problem which I'm trying to solve
> is clearly present.

I disagree, sorry.

> Finally the fact that httpd's last release was Feb '12 indicates to me
> a project at risk.

The last releases occurred in Dec ’16 and Jan ’17 respectively.

If you want to see trunk released, let’s branch and release v2.5.x in 
anticipation of v2.6.x.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Reset out x.minor.z definition of 'minor' at httpd?

[Graham Leggett]

On 20 Jan 2017, at 7:47 PM, David Zuelke  wrote:

> I'd actually like to question the whole practice of porting features back to 
> older branches. I think that's the core reason why trunk is in total 
> disarray, and why no substantial releases have been made. There is just no 
> reason for anyone to push forward the state of 2.6 or even 3.0 if you can 
> just backport whatever you want.

The reason this is bad is because Apache httpd comes with a module ecosystem - 
when you move from httpd v2.0 to v2.2 to v2.4 to v2.6, or rules are that the 
ABI can break, and therefore all modules that depend on that version must be 
recompiled. This includes modules that are closed source and offered by a 
proprietary vendor, or are open source but provided in binary form by a distro.

Right now, you can get new features on the httpd v2.4 branch, but ONLY if that 
feature does not break existing behaviour in any way. This is entirely 
reasonable, convenient, and what we’ve been doing since the 1990s.

> Just define 2.4 as "bug fixes only" the moment 2.6 is released, and start the 
> process of getting 2.6 out ASAP. In fact, why not declare 2.4 that right now? 
> It's already "stable". It doesn't need more features that suddenly change 
> behavior from 2.4.28 to 2.4.29 or whatever (that's the opposite of what users 
> are looking for).
> 
> That's how PHP does it now... new features can go into x.y.0, and once that 
> is released (actually, once it's in beta stage), anything that is not a small 
> fix needs to wait for x.y+1.0 or x+1.0.0. Which is not a big deal because 
> these releases land every year now, like clockwork.

So you have to wait a year for new features to see a release? Definitely not 
keen on that.

> I have said this in the other thread that hasn't gotten much traction ("A new 
> release process?"). The PHP team was in the exact same spot as HTTPD a few 
> years ago. No substantial progress, stale branches, no light at the end of 
> the tunnel, and a lot of fighting.

We’ve had a significant amount of progress, a trunk that is so stable that 
almost all fixes and features can be backported to v2.4 without any fear of 
incompatibility, and the “fighting” is limited to very few individuals.

We’re not broken, we don’t need fixing.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Alternate versioning proposal: patch line releases

[Graham Leggett]

On 20 Jan 2017, at 2:15 AM, Jacob Champion  wrote:

> Ignore the versioning number then; that's not really the core of my proposal. 
> The key points I'm making are
> 
> - introduce the concept of a low-risk release line

We have always had this, the current low-risk release line is called v2.4.x.

> - formally tie said release lines to test suite expansion, in a way that does 
> not impede current contributors to 2.4.x

We have always had this. The test suite is called httpd-test and covers all 
versions of httpd.

> - introduce a stable cadence with as little risk to end users as possible

We have always had this.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Reset out x.minor.z definition of 'minor' at httpd?

[Graham Leggett]

On 19 Jan 2017, at 11:43 PM, William A Rowe Jr  wrote:

> I think one of our disconnects with 2.4 -> 2.6 is that in any other
> framework, there would be no ABI breakage in 2.6. That breakage
> would be deferred to and shipped as 3.0.
> 
> The httpd project choose to call 2.minor releases as breaking
> changes. Due to poor design choices, or frequent refactorings,
> this essentially shifted our release numbering schema down one
> digit. So 2.6.0 is not an enhancement release, but a major release
> in the general understanding of these things. This might be the root
> of Jim's and my failure to come to any consensus.

I don’t see any relationship between poor design choices / frequent 
refactorings and a decision made in the late 1990s on a version numbering 
system.

> Right now, there are a number of gratuitous breaking changes on trunk
> that change binary ABI. I'm considering a trunk fork to preserve these
> changes (branches/3.x-future/?) and reverting every change to trunk/
> which was otherwise a no-op. Namespace, decoration, anything that
> prevents a user from loading a 2.4.x module in 2.next. And then we
> can look at what is a valid reason to take us to 3.0.

-1 (veto).

As you are well aware, the jump from v2.0 to v2.2, from v2.2 to v2.4, and the 
jump from v2.4 to v2.6 involves breaking changes, and this is a well 
established and stable pattern that is well understood by our end users. The 
“problem” you’re trying to solve does not exist.

Arbitrarily reverting the work of others displays a profound lack of respect 
for those members, committers and contributors to the ASF who have contributed 
to our codebase. This behaviour discourages collaboration between the community 
and project, and puts this project at risk.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Async write completion broken in trunk?

[Graham Leggett]

On 18 Jan 2017, at 3:38 PM, Jim Jagielski  wrote:

> That's a good overview... Yeah, it might be getter better, but it does
> seem that the nature of the bugs implies it's also getting more
> fragile. Could be, likely is, just my impression due to some weird
> things I've seen lately on some testing.

Pipelining support as it stands is really ugly.

There is a point that is reached where we need to ask the question “is there 
more data available to read”, and if the answer is yes, we need to follow the 
pipeline path. We answer the question “is there more data available to read” by 
trying to read some data, and then in turn by tying ourselves in ugly knots 
trying to “unread” that data so the next request can work.

I want to fix this properly so we don’t read data, we just get an answer “there 
is data to be read”. This will take a bit of thought, but doesn’t seem 
difficult to do.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Async write completion broken in trunk?

[Graham Leggett]

On 18 Jan 2017, at 3:05 PM, Jim Jagielski  wrote:

> Well, there's this, and it seems like there are issues w/
> the current mutexing as well, causing weird wake-ups.

Only solution really is to find the bugs and fix them.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Async write completion broken in trunk?

[Graham Leggett]

On 18 Jan 2017, at 3:00 PM, Jim Jagielski  wrote:

> Somewhat on-topic but also off-topic as well, but it does seem
> that event on trunk is getting much more fragile instead of more
> stable. It seems to be picking up some kruft which is making
> event on trunk a *worse* option than what's in 2.4, despite a deeper
> async alignment.

Can you give an example?

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Async write completion broken in trunk?

[Graham Leggett]

On 17 Jan 2017, at 7:40 PM, Luca Toscano  wrote:

> Since this email thread seems important, is there any update from anybody 
> working on it? It would be great to open a bugzilla task otherwise, to track 
> everything and make sure that we don't forget about it :)

I looked at this a while back - I found that pipelining support is causing the 
blocking.

When we’re handling pipelined requests and we reach a limit, we block. What we 
should be doing instead is looping round back to WRITE_COMPLETION, waiting for 
permission to write again. This should be reasonably straightforward to fix, 
but my financial year end is the end of this month so can’t look at this till 
February.

I suspect pipelining support has been suppressed in v2.4.x event MPM, and was 
at some point “fixed” on trunk, bringing this problem back.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: RemoteIPProxyProtocol

[Graham Leggett]

On 15 Jan 2017, at 18:35, Daniel Ruggeri  wrote:

>> As we *sure* we want to call it RemoteIPProxyProtocol instead
>> of just "regular" ProxyProtocol ?
>> 
>> The latter just sounds and looks "more right" to me.
> 
> I still like RemoteIPProxyProtocol because I also like the idea of
> "namespacing" modules' directives.

+1.

Anything starting with Proxy* should be in a module called mod_proxy*, 
otherwise confusion will ensue.

Regards,
Graham
--

Re: clang-analyzer?

[Graham Leggett]

On 08 Jan 2017, at 4:45 AM, Leif Hedstrom  wrote:

> I ran clang-analyzer against the HTTPD master branch, and it found 126 
> issues. Many of these are benign, but I was curious if the community has any 
> thoughts on this? With another project, I’ve found that keep static code 
> analysis to zero issues can really help finding new, serious issues 
> (basically, we put the tree in failed state if there’s a new static code 
> analysis issue).
> 
> The issues are all over the source code, in core and mod_’s alike. It’d be 
> pretty tedious to file individual tickets for each of them, so curious if 
> there’s any interest in cleaning this up to start with a clean state? It’d 
> then be easy to add clang-analyzer to the release process for example.

Adding clang-analyzer to a make target (not a default part of the build) would 
be a good step, it would make it easy for anyone to run it if they had it 
available.

The most effective contributions would be patches to fix each one. From 
experience it is difficult to fix these sort of things without the ability to 
rerun the analyser to ensure the issue is gone, and every now and again issues 
uncover things that may take some time to fix. Agreed that getting these things 
to zero would be a good thing to have.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

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

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: 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

Re: [proposed] 2.4 Maintenance SIG

[Graham Leggett]

On 03 Jan 2017, at 4:07 PM, William A Rowe Jr  wrote:

> Can you clarify the problem you’re trying to solve?
> 
> v3.0 and v2.6 are just numbers. For modest changes, we move to v2.6. For a 
> very large architecture change (for example, the addition of filters in v1.x 
> to v2.x), we move to 3.0.
> 
> Is there a very large architecture change planned by anybody?
> 
> In my experience, things that felt initially like big changes have actually 
> turned out to be rather modest changes that are still possible to backport to 
> v2.4 without an issue. For this reason I haven’t seen a reason to push very 
> hard for v2.6, never mind v3.0.
> 
> I do, the very specific problem is that trunk/, and therefore all 
> more-than-modest (or just neglected) contributions are now four years stale 
> and abandoned.
> 
> A certain way to push off contributors is to ignore their patch submissions. 
> The second method is to commit them to a dead fork.

I’m not following this logic.

The rules are patches are committed to trunk first, and therefore trunk is 
never dead by definition. People either backport the change to v2.4, or they 
wait for 2.6. The backport to v2.4 happens immediately, waiting for v2.6 either 
means “I’m happy to wait for v2.6” or it means “I’m not happy to wait, so let’s 
release v2.6”.

This is how it has always been, and I see no problem with this pattern.

> If trunk/ is a dead fork, it may be time for httpd to admit this, trash it 
> and re-fork trunk from 2.4.x branch.

We would obviously never do this, at to do so would treat our contributors with 
contempt.

> Beyond this, see the recent appeal for major.minor breaking change wish list 
> on trunk/STATUS, that is a different thread for you to chime in on.
> 
> Back to topic, who is interested in a stable release chain, since 2.4.x has 
> not been that?

I still don’t understand the problem you’re trying to solve, v2.4.x has 
certainly been a very effective stable release chain. Or more accurately, I do 
not see any problem that cannot be solved by our current process, which is a 
choice between the following:

- backport the stuff to v2.4; otherwise
- release v2.6 and continue.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: [proposed] 2.4 Maintenance SIG

[Graham Leggett]

On 03 Jan 2017, at 2:11 AM, William A Rowe Jr  wrote:

> So far, discussions are polarized on a single axis...
> 
> East: Let's work on 3.0; whatever is going on in 2.4 won't distract me, I 
> won't spend time reviewing enhancements, because 3.0 is the goal.
> 
> West: Let's keep the energy going on 2.4 enhancements, I won't spend time on 
> 3.0 usability because it isn't ready or necessary.
> 
> There is a disconnect, because 'East' folks can't actually put up with the 
> breakage introduced by enhancements they can't review on 2.4, but all feel 
> responsible to keeping 2.4 usable to EOL.
> 
> So I'd like to know, in light of a perpetual chain of (often build and/or 
> run-time breaking regression) enhancements, if there is support for a 
> 2.4.24.x release chain during the 3.0 transition? And support for potentially 
> 3x backports to 2.4.x, 2.4.24.x and 2.2.x, of really serious bug fixes?
> 
> It's clear this project doesn't agree, so the question twists to how we agree 
> to disagree.

Can you clarify the problem you’re trying to solve?

v3.0 and v2.6 are just numbers. For modest changes, we move to v2.6. For a very 
large architecture change (for example, the addition of filters in v1.x to 
v2.x), we move to 3.0.

Is there a very large architecture change planned by anybody?

In my experience, things that felt initially like big changes have actually 
turned out to be rather modest changes that are still possible to backport to 
v2.4 without an issue. For this reason I haven’t seen a reason to push very 
hard for v2.6, never mind v3.0.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Redis and mod_cache/mod_socache

[Graham Leggett]

On 31 Oct 2016, at 5:05 PM, Jim Jagielski  wrote:

> Moving to APR:
> 
> Query: Think it would be worth my time to work on a Redis
> implementation for APR-util? I am working on a minimal Redis
> lib, related to work, which is basically a soft reboot of Credis
> from GoogleCode, which could serve as the core functionality, which is
> what got me thinking about it.

+1.

There is an extension to apr_crypto on apr-util v1.6 that I’d like to get out 
the door, redis for apr_socache can go out at the same time.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Redis and mod_cache/mod_socache

[Graham Leggett]

On 31 Oct 2016, at 4:13 PM, Ruediger Pluem  wrote:

> Which creates the generic question for me: Can't we setup APR in a way that 
> for certain aspects we could compile
> "drivers" against an existing APR whose code exists outside of APR first and 
> gets part of standard APR later easily?
> I guess the redis stuff does not necessarily qualifies for this idea yet, 
> because we have no caching backend API defined
> in APR yet that would allow to just plugin different drivers / providers 
> (memcache does not seem to have a genric API
> defined yet).

We used to - that was what apr-util was for.

You have a point, there is nothing stopping us creating an apr-util-cache-redis 
package with a lifecycle of it’s own.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Redis and mod_cache/mod_socache

[Graham Leggett]

On 31 Oct 2016, at 3:43 PM, Jim Jagielski  wrote:

> It would, but that would mean even more of a APR dependency
> and a wait until the next release of APR and etc, etc, etc...
> Basically, APR moves too slow for httpd.

APR can make a release at any time. It usually doesn’t because it is a stable 
project, but that doesn’t mean it can’t.

APR-util v1.6 has changes on it waiting for release. This would be a good 
addition.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Redis and mod_cache/mod_socache

[Graham Leggett]

On 31 Oct 2016, at 3:30 PM, Jim Jagielski  wrote:

> Query: Think it would be worth my time to work on a
> Redis implementation for mod_cache/mod_socache? I am
> working on a minimal Redis lib, related to work, which
> is basically a soft reboot of Credis from GoogleCode,
> which could serve as the core functionality, which is
> what got me thinking about it.

Would it fit better as an APR-util module? If so, mod_cache_socache could use 
it unmodified.

Regards,
Graham
—



smime.p7s
Description: S/MIME cryptographic signature

Re: Random AH01842 errors in mod_session_crypto

[Graham Leggett]

On 4 Oct 2016, at 15:47, Paul Spangler  wrote:

> From my understanding, apr_crypto_key_t is an opaque struct defined 
> separately by each crypto provider, so mod_session_crypto will not be able to 
> do the sizeof.

That's a sizeof a pointer to apr_crypto_key_t, not the sizeof apr_crypto_key_t 
itself.

Keys are read at server start and reused. Trying to regenerate the key on every 
request has performance implications.

Regards,
Graham
--

Re: Async write completion broken in trunk?

[Graham Leggett]

On 06 Sep 2016, at 12:06 AM, Stefan Fritsch  wrote:

> in trunk, when having a lot of slow long running transfers, most of them seem 
> to hog a thread and only a few of them go into async write completion mode. 
> Compare this to 2.4, where many transfers are doing async write completion 
> and 
> only a small number of threads are busy.
> 
> Is this a known issue?

I’ve seen this before, but couldn’t reproduce it reliably.

If you can catch a long running transfer can you get a stacktrace from it? One 
possible option is that a bucket is insisting on being consumed in one go and 
thus refuses to leave the handler hook and enter write completion, the second 
option is that the bucket ends up in write completion but blocks, and we would 
need to know why.

There are still a number of filters out there that insist on swallowing whole 
bucket brigades in one go. Any of these filters will prevent write completion 
from working. This will show up in the stacktrace.

Regards,
Graham
—

Re: httpd-trunk\modules\filters\mod_crypto

[Graham Leggett]

On 08 Sep 2016, at 12:09 PM, NormW  wrote:

> What versions of apr\apr-util are assumed to be used when compiling 
> httpd-trunk?
> I get the following error when compiling mod_crypto, but can't find where the 
> offending symbol is typedef'd. Presently using apr-1.5 and apr-util-1.5.

Use apr-util v1.6 (as yet unreleased).

Regards,
Graham
—

Re: svn commit: r1753263 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_protocol.c

[Graham Leggett]

On 18 Jul 2016, at 6:32 PM, William A Rowe Jr  wrote:

> Worse, in http 2.4, the first two registered methods collide with BREW and 
> WHEN. That said, the 'fix', if we wanted to resolve it, is to use M_INVALID 
> +3 as the first value.

I’m not seeing the problem here, there is no expectation of binary 
compatibility between httpd v2.4 and trunk. Can you confirm what you’re trying 
to achieve by removing this?

Regards,
Graham
—

Caching PROPFIND

[Graham Leggett]

Hi all,

According to https://tools.ietf.org/html/rfc7234#section-2, it says the 
following:

"However, it is also possible to cache [...] responses to methods other than 
GET if the method's definition allows such caching and defines something 
suitable for use as a cache key."

Would it therefore be reasonable to teach mod_cache how to cache methods other 
than GET in some kind of configurable fashion, with PROPFIND being the obvious 
example?

Regards,
Graham
—

Re: svn commit: r1752099 - in /httpd/httpd/trunk: ./ build/rpm/ docs/log-message-tags/ docs/manual/ docs/manual/mod/ modules/filters/

[Graham Leggett]

On 11 Jul 2016, at 4:39 PM, Ruediger Pluem  wrote:

>> Added: httpd/httpd/trunk/modules/filters/mod_crypto.c
>> URL: 
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_crypto.c?rev=1752099=auto
>> ==
>> --- httpd/httpd/trunk/modules/filters/mod_crypto.c (added)
>> +++ httpd/httpd/trunk/modules/filters/mod_crypto.c Sun Jul 10 17:27:03 2016
> 
>> +static void *merge_crypto_config(apr_pool_t * p, void *basev, void *addv)
>> +{
>> +crypto_conf *new = (crypto_conf *) apr_pcalloc(p, sizeof(crypto_conf));
>> +crypto_conf *add = (crypto_conf *) addv;
>> +crypto_conf *base = (crypto_conf *) basev;
>> +
>> +new->library = (add->library_set == 0) ? base->library : add->library;
>> +new->params = (add->library_set == 0) ? base->params : add->params;
>> +new->library_set = add->library_set || base->library_set;
>> +
>> +new->crypto = base->crypto;
> 
> Shouldn't this be:
> 
> new->crypto = add->crypto;

In this case no, the value of crypto is set globally and needs to be unique 
across the server.

>> +
>> +return (void *) new;
>> +}
>> +
>> +static void *create_crypto_dir_config(apr_pool_t * p, char *dummy)
>> +{
>> +crypto_dir_conf *new =
>> +(crypto_dir_conf *) apr_pcalloc(p, sizeof(crypto_dir_conf));
>> +
>> +new->size_set = 0;  /* unset */
> 
> Is this needed? We do apr_pcalloc above.

We don’t need it, this is fixed.

>> +new->size = DEFAULT_BUFFER_SIZE;/* default size */
>> +new->cipher = DEFAULT_CIPHER;
>> +new->cipher = DEFAULT_MODE;
> 
> Shouldn't this be:
> 
> new->mode = DEFAULT_MODE;

Definitely.

All fixed in r1752348, thanks for this.

Regards,
Graham
—

Re: [Patch] mod_crypto: A general purpose crypto filter with HLS support

[Graham Leggett]

On 12 Dec 2014, at 12:59 AM, Yann Ylavic  wrote:

>> Incorporating the above, How about this?
> 
> Looks fine, modulo the above (didn't figure out before):

All of this has been included.

I’ve committed this in r1752099 so that further development can occur on trunk.

Regards,
Graham
—

Re: Mergine of Multiple Cookie Headers

[Graham Leggett]

On 28 Jun 2016, at 4:29 PM, Rainer Canavan  wrote:

> We've observed multiple gateways, operated by e.g. AT, COLT and
> Vodafone, that inject additional Cookie: headers into client requests,
> such as
> 
> Cookie: actually=from_the_client
> Cookie: Bearer-Type=w-TCP
> Cookie: network-access-type=UMTS
> 
> Apache httpd merges those headers into a single, comma separated list,
> and also appends the names and values of all Cookies set in the
> additional Cookie Headers to the value of the last Cookie of the first
> header. This can be seeen by logging  %{actually}C for the example
> above, which would contain
> 
> actually=from_the_client, Bearer-Type=w-TCP, network-access-type=UMTS
> 
> While RFC 6265 clearly requires that User-Agents send only a single
> Cookie: request header, I would argue that the Cookie header should be
> treated as an exception, similar to the Set-Cookie:-response header,
> and not be merged into a single header field. An alternative would be
> to use "; " as a separator.
> 
> Any thoughts?

What problem are you trying to solve?

Regards,
Graham
—

Re: RFC 7230..7235 Parsing Conformance?

[Graham Leggett]

On 17 May 2016, at 6:43 PM, William A Rowe Jr  wrote:

> Wondering what other contributors are thinking on this topic.
> 
> We have a number of changes in the ABNF grammar between
> RFC2616 and RFC7230..7235. Do we want trunk 2.6/3.0 to be 
> an entirely RFC723x generation server, and drop all support for 
> RFC2616? 
> 
> Do we want to backport these changes to 2.4.x? If so, what
> mechanism do we want to toggle the behavior of the server
> between 2616 and 7230..7235? 
> 
> We can presume a small performance hit in any conditional 
> operation, especially when those decisions apply to tight parsing 
> loop. Toggling between two different parser implementations would
> probably be a bit more efficient than conditionals within a parser
> itself.

Can you give some examples to get a sense of the extent of this?

Regards,
Graham
—

Re: core_output, files and setaside

[Graham Leggett]

On 04 May 2016, at 3:22 PM, Stefan Eissing  wrote:

> file_bucket_setaside() currently does not care about the refcount. setaside 
> affects *all* shared file buckets, wherever they currently reside. So it 
> moves the contained apr_file_t into the filter deferred pool, eventually 
> clears that pool and the file is closed via cleanup of the file (not the 
> bucket).

That’s broken, we need to fix that so it works properly.

> While dup'ing the file descriptor would work, it seems overly costly in this 
> case. What is the case?
> 
> The output filter already makes the distinction wether filter->r is set or 
> not. When filter->r is set, it uses filter->r->pool to setaside buckets it 
> wants to keep around. This is safe since it knows that some time in the 
> future, an EOR bucket will come along and cleanup - at the right time.
> 
> HTTP/2 has a similar bucket lifetime case: only now, there are several 
> requests ongoing and interleaved onto the one master connection. But the 
> basic assumption still holds: there will be some kind of EOR bucket that 
> manages the lifetimes of buckets before it correctly.
> 
> But the output filter does not know this and even if, would not know which 
> pool to setaside which bucket to.

That’s expected - during requests, we set aside into the request pool, where 
requests are one shot and you’re done. During connections however we cannot use 
the connection pool, because the connection pool lives potentially for a very 
long time. This is why the deferred pool exists.

None of this matters though, buckets should work correctly in both cases.

> One way for a generic approach is a new META bucket: POOL_LIFE that carries a 
> pool or NULL. It's contract is: all buckets that follow me have the lifetime 
> of my pool (at least) and this holds until another POOL_LIFE bucket comes 
> along. Pools announced in this way are promised to only disappear after some 
> kind of EOR or FLUSH has been sent.

This breaks the contract of pools - every bucket has a pool, and now there 
would be a second mechanism that duplicates this first mechanism, and as soon 
as there is a mismatch we crash.

Let’s just fix the original bug - make sure that file buckets behave correctly 
when setaside+refcount is used at the same time.

Regards,
Graham
—

Re: core_output, files and setaside

[Graham Leggett]

On 04 May 2016, at 11:13 AM, Stefan Eissing  
wrote:

> The problem is not the apr_bucket_destroy(). The file bucket setaside, calls 
> apr_file_setaside(), in core_output on a deferred pool, and then core_output 
> clears that pool. This invalidates all still existing file buckets using that 
> apr_file.

This scenario should still work properly, it shouldn’t cause anything to break.

First off, file buckets need reference counting to make sure that only on the 
last removal of the bucket the file is closed (it may already do this, I 
haven’t looked, but then if it did do this properly it should work).

Next, if a file bucket is setaside, but the reference count isn’t one (in other 
words, other file buckets exist pointing at the same file descriptor in other 
places), and the pool we’re setting aside into isn’t the same or a child pool, 
we should dup the file descriptor during the setaside.

The typical scenario for the deferred pool should be the first scenario above.

Regards,
Graham
—

Re: core_output, files and setaside

[Graham Leggett]

On 04 May 2016, at 10:45 AM, Stefan Eissing  
wrote:

> I have been wrong before...but...
> 
> mod_http2 needs to send out a file response:
> 1. it starts with the response body brigade: [file:0-len][eos]
> 2. it sends the first 16K frame by splitting the file bucket: 
>   -> passing to core output: [heap:frame header][file:0-16k]
>   -> remaining body:  [file:16K-len][eos]
> 3. core_output decides to setaside:
>   -> setaside (deferred pool): [heap:frame header][file:0-16k]
>   -> remaining body:  [file:16K-len][eos]
> 4. core_output sends and, sometimes, clears the deferred pool
>   -> which closes the file descriptor
> 5. next 16K frame: [heap:frame header][file:16k-32K] results in APR_EBADF

This smells wrong - if you split a file bucket (and there is nothing wrong with 
splitting a file bucket) you should end up with two file buckets, and 
destroying the first file bucket while the second file bucket still exists 
shouldn’t cause the second file bucket descriptor to close.

Regards,
Graham
—

Re: TIL

[Graham Leggett]

On 27 Apr 2016, at 2:49 PM, Stefan Eissing  wrote:

> I had a look into 2.4.x this morning and while there are changes in filter 
> brigade setaside code, the basic "cleanup" of empty and meta buckets before 
> the setaside is there already.
> 
> I think this has not bween noticed before as 
> 1. EOR is always followed by a FLUSH
> 2. most bucket types survive the destruction of r->pool quite well, even pool 
> buckets silently morph
> 3. even if transient buckets would reference pool memory, settings those 
> aside after destruction of r->pool, but before filter return would access 
> freed, but not re-used memory from the connection allocator - I think.
> 
> So far, I have seen no reasons why meta buckets should not just be setaside 
> in core filter. 0 length data buckets should also stay, IMHO, just ignored in 
> the actual write.
> 
> I can only guess the original intent: 0-length data buckets sometimes happen 
> during some brigade read modes and if there are several FLUSH buckets in the 
> brigade, it makes sense to get rid of them also. But I think the space 
> savings are minimal.
> 
> But there could be reasons for the current behavior, I overlooked. So, I am 
> asking around.

I don’t see any obvious reason for the current behaviour either - is it 
possible to go back in history and confirm the log entry when it was introduced?

Regards,
Graham
—

Re: TIL

[Graham Leggett]

On 26 Apr 2016, at 4:49 PM, Stefan Eissing  wrote:

> Today I Learned the difference between writing 
>  DATA + META 
> and 
>  DATA + META + FLUSH
> to the core output filters. I am astonished that
> my code ever worked.
> 
> Seriously, what is the reason for this kind of
> implementation? I would like to pass META buckets
> in non-blocking way and *not* lose the order of 
> bucket destruction. Any explanation or advice is
> appreciated!

Definitely looks like a bug - if we’re deleting buckets out of order things 
will definitely break. I say we must fix the core.

Regards,
Graham
—

Re: Allow SSLProxy* config in context?

[Graham Leggett]

On 13 Apr 2016, at 12:40 PM, Rainer Jung  wrote:

> I stumbled into a situation where a reverse proxy had two different backends 
> behind the same VHost of the proxy. Both backends demand client certs as 
> becomes more and more common for services today. Unfortunately the CA which 
> issues the client certs in both cases is the same CA, but the demanded client 
> cert is individual to the backend services.
> 
> As far as I can see, this is currently not configurable. The 
> SSLProxyMachineCertificateFile and SSLProxyMachineCertificatePath only work 
> on the VHost level and the client cert detection algo in 
> ssl_callback_proxy_cert() chooses the first client cert it can find which was 
> issued b the right CA. No way to distinguish further.
> 
> To me it looks like the "right" way of handling SSLProxy* config would be per 
> . Did anyone else already encounter a similar problem? Any thoughts or 
> experiments on how to solve this for the future?

I looked at this a while back.

The catch is that mod_ssl forces us to declare SSL certs and keys server wide, 
not per directory, loaded and parsed at startup. We however want to specify 
certs per directory.

What I had in mind was a syntax where the certs were named, for example:

SSLProxyCertificate foo /path/to/foo.cert

Followed somewhere else by:


  SSLProxyEnable foo


Regards,
Graham
—

Re: Status for 2.4.20

[Graham Leggett]

On 23 Mar 2016, at 1:58 PM, Noel Butler  wrote:

> as stated previously, this shit will happen when certain people push with a 
> release often mentality
> 
> AFAIK there is *ZERO* critical exploit bugs to be patched by any pending 
> release, so lets get house in order  S T A B L E , then worry about releases, 
> jesus christ, we are not ubuntu or redhat with set programs to release every 
> 3 or 6 months regardless if shit is ready or not…..

It sounds like you’re making drama where there is none.

We have a release process designed to catch problems before they reach the 
public. That release process caught a problem, and that problem is being dealt 
with as per that release process. The system is working, nothing to see here.

Regards,
Graham
—

Re: svn commit: r1736216 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_request.c server/mpm/event/event.c server/mpm/motorz/motorz.c server/mpm/simple/simple_io.c server/util_filter.c

[Graham Leggett]

On 22 Mar 2016, at 7:08 PM, yla...@apache.org wrote:

> URL: http://svn.apache.org/viewvc?rev=1736216=rev
> Log:
> Follow up to r1734656: restore c->data_in_input_filters usage to
> see if it helps unblocking test framework.

I created an almost identical patch before doing an update, the only difference 
was this:

Index: modules/http/http_request.c
===
--- modules/http/http_request.c (revision 1736263)
+++ modules/http/http_request.c (working copy)
@@ -453,7 +453,7 @@
 
 ap_process_async_request(r);
 
-if (!c->data_in_input_filters || ap_run_input_pending(c) != OK) {
+if (!c->data_in_input_filters && ap_run_input_pending(c) != OK) {
 bb = apr_brigade_create(c->pool, c->bucket_alloc);
 b = apr_bucket_flush_create(c->bucket_alloc);
 APR_BRIGADE_INSERT_HEAD(bb, b);

I get hangs without the above patch on the test suite, but it runs clean with. 
Can you give it a try?

Regards,
Graham
—

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Graham Leggett]

On 22 Mar 2016, at 7:11 PM, Yann Ylavic  wrote:

> In r1736216, I tried to restore c->data_in_input_filters usage as
> prior to r1734656.
> Does it help (or I'll revert)?

Got caught up in other stuff before I could get to the bottom of this :(

It looks like we need to restore c->data_in_input_filters usage.

What this c->data_in_input_filters does is do a poor man’s compensation for 
input filters that buffer data during this particular phase of the request 
handling without having a mechanism to tell anybody. Without telling us they’ve 
buffered data, we’ve got to work out ourselves whether we should loop round and 
poll for read, or not. If we make a mistake, we poll for a read that was 
already read, and we hang.

This is really hit and miss - if you have input filters that buffer data in the 
server you’ll hang, but if you don’t have input filters that buffer data in the 
server you don’t hang and the test suite works fine.

When v2.6 comes we must remove c-> data_in_input_filters and use the proper 
mechanism for input filters to signal they have buffered data.

Regards,
Graham
—

Re: svn commit: r1735176 - in /httpd/httpd/trunk: CHANGES modules/http/http_core.c

[Graham Leggett]

On 16 Mar 2016, at 1:08 PM, Stefan Eissing  wrote:

> I needed to revert r1735174 to make HTTP/1.1 work again. I submitted this to 
> trunk since other people are affected too.
> 
> Graham: if you are unable to reproduce this and need any additional data, pls 
> let me know.

The test suite runs clean for me - when it hangs can you confirm where it is 
hanging?

Regards,
Graham
—

Re: [Patch] Ensure HTTP1 filters are only added on HTTP1 requests

[Graham Leggett]

On 16 Mar 2016, at 5:57 AM, William A Rowe Jr  wrote:

> My concern is that this can't and shouldn't change on 2.4.x.
> 
> I love the concept and it is correct, however there are enough modules relying
> on the fact that they must remove the http protocol filters that changing the
> default behavior is effectively breaking binary ABI.
> 
> Thoughts?

Hmm…removing a filter that doesn’t exist isn’t an error, but relying on a side 
effect of a filter being there and now suddenly it is not there any more 
certainly will be. You’re right on the ABI.

There are two approaches that leap to my mind, one is provide an optional 
function that signals “don’t add http filters on non http connections”, the 
second is to keep this change on v2.5+ and use version check to remove filters 
on any code that is intended to run correctly on v2.4.

Regards,
Graham
—

Re: [Patch] mod_tcp / mod_proxy_tcp / mod_ssl_tcp

[Graham Leggett]

On 12 Mar 2016, at 5:46 PM, Graham Leggett <minf...@sharp.fm> wrote:

> The following patch provides support for TCP proxying to httpd.

Here is an updated patch with changes to the ap_mpm_register_poll* functions.

Regards,
Graham
--



httpd-tcp-proxy2.patch
Description: Binary data

Re: Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t

[Graham Leggett]

On 15 Mar 2016, at 1:09 PM, Yann Ylavic  wrote:

>> I just gave it a try and it was a lot more elegant than I thought.
>> 
>> You create an array the precise size you need, and off you go.
> 
> That's indeed elegant enough, but I think I prefer the plain array
> solution, still :p
> 
> Don't you like?
> {
>apr_pollfd_t pfds[3];
>/* or */
>apr_pollfd_t *pfds = apr_pcalloc(p, 3 * apr_pollfd_t);
> 
>pfds[0].desc_type = APR_POLL_SOCKET;
>pfds[0].desc.s = s0;
>...
>pfds[1].desc_type = APR_POLL_SOCKET;
>pfds[1].desc.s = s1;
>...
>pfds[2].desc_type = APR_NO_DESC;
> 
>ap_mpm_register_poll_callback_timeout(pfds, pool, ...);
> }

The trouble with the above is that because of the pool cleanup we now have, 
pfds[3] needs to live as long as pool p. In your example it does, but there is 
nothing stopping someone trying to allocate pfds[3] on the stack and then 
returning. Later the cleanup is run, and boom - difficult to debug crash or 
weird behaviour.

With the array you’re guaranteed the memory is allocated from a pool, which 
means the pool cleanup will always be safe.

What we should also do is drop the apr_pool_t *p parameter and read it from 
apr_header_array_t’s pool instead. This will be a further check to stop the 
caller from doing anything pathological, as we definitely know the cleanup and 
the array belong to each other, and our API becomes simpler still.

Attached patch does this.

Regards,
Graham
—



httpd-register-poll3.patch
Description: Binary data

[Patch] Ensure HTTP1 filters are only added on HTTP1 requests

[Graham Leggett]

Hi all,

The following patch fixes the problem where the HTTP1 filters are added on non 
HTTP1 requests.

The filters are now only added if ap_get_protocol() returns AP_PROTOCOL_HTTP1.

Regards,
Graham
--

Index: modules/http/http_core.c
===
--- modules/http/http_core.c(revision 1734998)
+++ modules/http/http_core.c(working copy)
@@ -34,6 +34,8 @@
 
 #include "mod_core.h"
 
+module AP_MODULE_DECLARE_DATA http_module;
+
 /* Handles for core filters */
 AP_DECLARE_DATA ap_filter_rec_t *ap_http_input_filter_handle;
 AP_DECLARE_DATA ap_filter_rec_t *ap_http_header_filter_handle;
@@ -48,6 +50,10 @@
  */
 static int async_mpm = 0;
 
+typedef struct {
+int is_http;
+} http_conn_conf;
+
 static const char *set_keep_alive_timeout(cmd_parms *cmd, void *dummy,
   const char *arg)
 {
@@ -242,9 +248,29 @@
 return OK;
 }
 
+static int http_pre_connection(conn_rec *c, void *csd)
+{
+const char *protocol = ap_get_protocol(c);
+
+http_conn_conf *cconf = apr_pcalloc(c->pool, sizeof(*cconf));
+
+if (!protocol || !strcmp(AP_PROTOCOL_HTTP1, protocol)) {
+cconf->is_http = 1;
+}
+
+ap_set_module_config(c->conn_config, _module, cconf);
+
+return OK;
+}
+
 static int ap_process_http_connection(conn_rec *c)
 {
-if (async_mpm && !c->clogging_input_filters) {
+http_conn_conf *cconf = ap_get_module_config(c->conn_config, _module);
+
+if (!cconf || !cconf->is_http) {
+return DECLINED;
+}
+else if (async_mpm && !c->clogging_input_filters) {
 return ap_process_http_async_connection(c);
 }
 else {
@@ -254,7 +280,6 @@
 
 static int http_create_request(request_rec *r)
 {
-/* FIXME: we must only add these filters if we are an HTTP request */
 if (!r->main && !r->prev) {
 ap_add_output_filter_handle(ap_byterange_filter_handle,
 NULL, r, r->connection);
@@ -295,6 +320,7 @@
 static void register_hooks(apr_pool_t *p)
 {
 ap_hook_post_config(http_post_config, NULL, NULL, APR_HOOK_MIDDLE);
+ap_hook_pre_connection(http_pre_connection,NULL,NULL, 
APR_HOOK_REALLY_LAST);
 ap_hook_process_connection(ap_process_http_connection, NULL, NULL,
APR_HOOK_REALLY_LAST);
 ap_hook_map_to_storage(ap_send_http_trace,NULL,NULL,APR_HOOK_MIDDLE);

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Graham Leggett]

On 14 Mar 2016, at 10:48 AM, Ruediger Pluem  wrote:

> This seems to cause frequent (no always) failures with test 8 of 
> t/ssl/proxy.t.
> The request times out with a 504 status. So it looks like the "backend" in 
> this request does not respond.
> Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
> closeset guess would be that the changes to
> checkpipeline cause this.

I commented out check_pipeline() completely, and it passed all the tests.

Looking at check_pipeline, it seems to try and eat trailing CRLFs, which is 
duplicating the functionality in read_request_line() which eats preceding CRLFs 
already.

check_pipeline() seems to have been overhauled recently, not sure what problem 
it is trying to solve. Can we remove it?

Regards,
Graham
—

Re: Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t

[Graham Leggett]

On 14 Mar 2016, at 11:13 AM, Stefan Eissing  
wrote:

> Like the direction this is going as well.
> 
> Do we need a MPM Query for detecting support before one actually has a handle
> for registration?

We do - most recent patch has that, and we managed to drop one directive out of 
mod_proxy_wstunnel.

Regards,
Graham
—

Re: Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t

[Graham Leggett]

On 14 Mar 2016, at 11:17 PM, Yann Ylavic  wrote
:
>> Having looked at this in more detail this isn’t as simple.
>> 
>> The sticking point is the cleanup, which needs to be passed a single struct 
>> to do it’s work. To pass both the pfds and the npdfs these two need to be 
>> wrapped in a structure of some kind, which the user has to feed in from the 
>> outside common to both register and unregister. This structure probably 
>> should be an apr_array_header_t.
> 
> Hmm right, either this or, say, the ending pfd is the one with
> ->reqevents=0, or ->p=NULL, or probably better
> ->desc_type=APR_NO_DESC.
> Up to the caller to ensure this, it could possibly be documented.
> 
>> 
>> Is it worth making the change from apr_pollfd_t **pfds to apr_array_header_t 
>> *pfds?
> 
> That would still require some "complex" initialization, so I'd prefer
> the above if it doesn't sound to hacky…

I just gave it a try and it was a lot more elegant than I thought.

You create an array the precise size you need, and off you go.

Regards,
Graham
—



httpd-register-poll2.patch
Description: Binary data

Re: Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t

[Graham Leggett]

On 14 Mar 2016, at 10:32 AM, Yann Ylavic  wrote:

> Since apr_pollfd_t is not opaque (unlike apr_socket_t), maybe we could
> remove the indirection here (and in the code below) with somthing like
> (apr_pollfd_t *pfds, size_t npfds, ...).
> That would allow a single allocation (all pfds in once) and possibly
> make things easier for the caller.

Having looked at this in more detail this isn’t as simple.

The sticking point is the cleanup, which needs to be passed a single struct to 
do it’s work. To pass both the pfds and the npdfs these two need to be wrapped 
in a structure of some kind, which the user has to feed in from the outside 
common to both register and unregister. This structure probably should be an 
apr_array_header_t.

Is it worth making the change from apr_pollfd_t **pfds to apr_array_header_t 
*pfds?

Regards,
Graham
—

Re: Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t

[Graham Leggett]

On 14 Mar 2016, at 10:32 AM, Yann Ylavic  wrote:

> Since apr_pollfd_t is not opaque (unlike apr_socket_t), maybe we could
> remove the indirection here (and in the code below) with somthing like
> (apr_pollfd_t *pfds, size_t npfds, ...).
> That would allow a single allocation (all pfds in once) and possibly
> make things easier for the caller.

This definitely makes sense.

I originally wondered whether we could pass an apr_array_header_t into it, but 
it felt like overkill. Doing it your way means that we could use an array if we 
wanted to, or we could just pass structures on the stack, which would be much 
more flexible.

Regards,
Graham
—

Re: [Patch] mod_tcp / mod_proxy_tcp / mod_ssl_tcp

[Graham Leggett]

On 14 Mar 2016, at 11:01 AM, Yann Ylavic  wrote:

>> The following patch provides support for TCP proxying to httpd.
>> 
>> It consists of the following three parts:
>> 
>> - mod_tcp: Allows the frontend to receive pure TCP connections
> 
> It looks like this module is only needed to remove HTTP filters from the 
> chain.
> Is the goal to have this core module instead of mod_http and make the
> latter dynamic?

Hmmm - good point.

What we need next is a proper protocol handling mechanism to efficiently 
determine the protocol in use on the connection, the same way we can 
efficiently determine the HTTP method.

Once we have that the core can be free of HTTP modules and we can just use the 
mod_tcp process_connection() handler.

>> - mod_proxy_tcp: Allows the proxy to make pure tcp or tls connections to a 
>> backend
> 
> Thanks, this will be very useful.
> 
>> - mod_ssl_tcp: Allows the proxy to route incoming connections based on the 
>> SNI header (tlsext)
> 
> Hmm, isn't mod_ssl (underlying-)protocol agnostic?
> Why couldn't it be used as-is (or adapted), and avoid code duplication?

It was like that to start with, but I split it all out so it could stand alone.

I see the value of mod_ssl just having this as an extra input filter, will 
simplify this.

>> In the following example config, incoming TCP connections are routed based 
>> on their SNI (the tlsext protocol) to given backend servers, which then 
>> complete the SSL connections as raw tunnels.
>> 
>> This allows you to use client certificates through the httpd proxy balancer 
>> all the way to the backend server without the proxy terminating any SSL 
>> along the way.
>> 
>> 
>>  Protocol tlsext
> 
> Maybe "tcps"? I agree that SNI extension is needed, but "tlsext" looks
> confusing.

The “tlsext” refers to the TLS extentions which are parsed to determine what 
the client is trying to talk to. These extensions are SNI and APLN (not yet 
supported but would be great if we could).

“tcps” implies “tcp over ssl”, which we already can do - just turn on SSLEnable.

Regards,
Graham
—

Re: Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t

[Graham Leggett]

On 13 Mar 2016, at 10:55 PM, Eric Covener  wrote:

> I also meant the original feature never made it, so we can whatever we
> want to it.

What do you think of this?

It includes a cleanup to the original pool to remove any unfired pollsets from 
the core.

Index: include/ap_mpm.h
===
--- include/ap_mpm.h(revision 1734657)
+++ include/ap_mpm.h(working copy)
@@ -207,50 +207,48 @@
void *baton);
 
 /**
- * Register a callback on the readability or writability on a group of sockets
- * @param s Null-terminated list of sockets
+ * Register a callback on the readability or writability on a group of
+ * sockets/pipes.
+ * @param pds Null-terminated list of apr_pollfd_t
  * @param p pool for use between registration and callback
- * @param for_read Whether the sockets are monitored for read or writability
  * @param cbfn The callback function
  * @param baton userdata for the callback function
- * @return APR_SUCCESS if all sockets could be added to a pollset, 
+ * @return APR_SUCCESS if all sockets/pipes could be added to a pollset,
  * APR_ENOTIMPL if no asynch support, or an apr_pollset_add error.
- * @remark When activity is found on any 1 socket in the list, all are removed 
+ * @remark When activity is found on any 1 socket/pipe in the list, all are 
removed
  * from the pollset and only 1 callback is issued.
  */
 
-AP_DECLARE(apr_status_t) ap_mpm_register_socket_callback(apr_socket_t **s,
- apr_pool_t *p,
- int for_read, 
- ap_mpm_callback_fn_t 
*cbfn,
- void *baton);
+AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(apr_pollfd_t **pds,
+   apr_pool_t *p,
+   ap_mpm_callback_fn_t 
*cbfn,
+   void *baton);
  /**
- * Register a callback on the readability or writability on a group of 
sockets, with a timeout
- * @param s Null-terminated list of sockets
+ * Register a callback on the readability or writability on a group of 
sockets/pipes,
+ * with a timeout.
+ * @param pds Null-terminated list of apr_polldf_t
  * @param p pool for use between registration and callback
- * @param for_read Whether the sockets are monitored for read or writability
  * @param cbfn The callback function
  * @param tofn The callback function if the timeout expires
  * @param baton userdata for the callback function
  * @param timeout timeout for I/O in microseconds, unlimited if <= 0
- * @return APR_SUCCESS if all sockets could be added to a pollset, 
+ * @return APR_SUCCESS if all sockets/pipes could be added to a pollset,
  * APR_ENOTIMPL if no asynch support, or an apr_pollset_add error.
- * @remark When activity is found on any 1 socket in the list, all are removed 
+ * @remark When activity is found on any 1 socket/pipe in the list, all are 
removed
  * from the pollset and only 1 callback is issued. 
  * @remark For each call, only one of tofn or cbfn will be called, never both.
  */
 
-AP_DECLARE(apr_status_t) ap_mpm_register_socket_callback_timeout(apr_socket_t 
**s,
+AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback_timeout(apr_pollfd_t 
**pfds,
  apr_pool_t *p,
- int for_read, 
  ap_mpm_callback_fn_t 
*cbfn,
  ap_mpm_callback_fn_t 
*tofn,
  void *baton,
  apr_time_t timeout);
 
 
-AP_DECLARE(apr_status_t) ap_mpm_unregister_socket_callback(apr_socket_t **s, 
-   apr_pool_t *p);
+AP_DECLARE(apr_status_t) ap_mpm_unregister_poll_callback(apr_pollfd_t **pfds,
+ apr_pool_t *p);
 
 typedef enum mpm_child_status {
 MPM_CHILD_STARTED,
Index: include/mpm_common.h
===
--- include/mpm_common.h(revision 1734657)
+++ include/mpm_common.h(working copy)
@@ -426,15 +426,15 @@
  * register the specified callback
  * @ingroup hooks
  */
-AP_DECLARE_HOOK(apr_status_t, mpm_register_socket_callback,
-(apr_socket_t **s, apr_pool_t *p, int for_read, 
ap_mpm_callback_fn_t *cbfn, void *baton))
+AP_DECLARE_HOOK(apr_status_t, mpm_register_poll_callback,
+(apr_pollfd_t **pds, apr_pool_t *p, ap_mpm_callback_fn_t 
*cbfn, void *baton))
 
 /* register the specified 

Re: Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t

[Graham Leggett]

On 13 Mar 2016, at 10:53 PM, Eric Covener  wrote:

> I don't think I had a good reason for going one way or the other, +1
> here.  Trunk-only and presumably nobody consuming it from the lack of
> feedback.

From what I can see, this was never backported to v2.4.

Regards,
Graham
—

Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t

[Graham Leggett]

Hi all,

I would like to propose a change to the mpm_register_socket_callback() hooks to 
add the ability to support both pipes as well as sockets. In addition, I need 
more control over the individual events to support the idea that one 
socket/pipe might want to read at the same time another might want to write.

What I have in mind is to swap the apr_socket_t with a apr_pollfd_t, with all 
operation staying the same.

Index: include/mpm_common.h
===
--- include/mpm_common.h(revision 1734657)
+++ include/mpm_common.h(working copy)
@@ -427,14 +427,14 @@
  * @ingroup hooks
  */
 AP_DECLARE_HOOK(apr_status_t, mpm_register_socket_callback,
-(apr_socket_t **s, apr_pool_t *p, int for_read, 
ap_mpm_callback_fn_t *cbfn, void *baton))
+(apr_pollfd_t **pds, apr_pool_t *p, ap_mpm_callback_fn_t 
*cbfn, void *baton))
 
 /* register the specified callback, with timeout 
  * @ingroup hooks
  *
  */
 AP_DECLARE_HOOK(apr_status_t, mpm_register_socket_callback_timeout,
-(apr_socket_t **s, apr_pool_t *p, int for_read, 
+(apr_pollfd_t **pds, apr_pool_t *p,
  ap_mpm_callback_fn_t *cbfn,  
  ap_mpm_callback_fn_t *tofn, 
  void *baton, 
@@ -444,7 +444,7 @@
  * @ingroup hooks
  */
 AP_DECLARE_HOOK(apr_status_t, mpm_unregister_socket_callback,
-(apr_socket_t **s, apr_pool_t *p))
+(apr_pollfd_t **pds, apr_pool_t *p))
 
 /** Resume the suspended connection 
  * @ingroup hooks

Regards,
Graham
—

Re: [Patch] mod_tcp / mod_proxy_tcp / mod_ssl_tcp

[Graham Leggett]

On 12 Mar 2016, at 6:26 PM, Eric Covener  wrote:

> Very cool stuff. Only looked on the surface so far, but one thing it
> reminded me of in the async case is that the MPM knows the "suspended"
> count but we never gave an API to really "unsuspend" when using a
> timed or socket callback.  Longer term we really need to find a way to
> share that raw forwarding code as it's now probably in three modules.

I am slowly working through this, teasing out each problem that prevents async 
behaviour.

A key part of mod_tcp is the “switch ladder”, which allows 
tcp_process_connection() to exit at any time and be called again, at which 
point it will continue where it left off. We need corresponding ladders deeper 
into the core to make each layer down able to continue where it left off if it 
got EAGAIN. We also need to fix some of the function signatures - 
ap_process_request() is void, and so cannot signal to us elegantly that it 
wants to be called again, or whether it is done and we can move on.

Another problem that has become apparent is that the MPMs are too limiting with 
respect to the events the core is willing to wait on. Right now the only event 
the core will handle for us is to wait for the underlying socket to be 
readable/writable. What I have in mind is a more general mechanism, where 
instead of “one socket” we have “a stack of sockets/pipes/files[1]”, and the 
event we wait for is the socket/pipe/file on the top of the stack. This gives 
us AND behaviour. If browser socket is writable AND backend socket is readable 
then [do stuff].

To state it another way, we wait for the browser socket to be writable, this 
triggers the normal pattern of events until we need to block on something else, 
for example we might block waiting for the backend proxy socket to be readable. 
That socket is added to the stack and the MPM now waits for that event. When 
that event triggers the top of the stack is popped off and we’re back to 
waiting on the underlying event (the socket).

I just need to find an elegant way to trigger all of this. It would be really 
nice if we had SOCKET, PIPE, FILE (etc) buckets that pushed themselves onto 
this stack as soon as they returned EAGAIN via some kind of callback.

I need to think this through to work out how to incorporate OR into this. 
Perhaps a list of stacks. This would allow a set of read events to run 
concurrently with write events. Then the sense of the connection has to be 
worked into it too so mod_ssl works non blocking.

[1] files are typically not pollable, but we shouldn't restrict our API. It’s 
pipes we want to block on and in our world that’s apr_file_t.

Regards,
Graham
—

[Patch] mod_tcp / mod_proxy_tcp / mod_ssl_tcp

[Graham Leggett]

Hi all,

The following patch provides support for TCP proxying to httpd.

It consists of the following three parts:

- mod_tcp: Allows the frontend to receive pure TCP connections
- mod_proxy_tcp: Allows the proxy to make pure tcp or tls connections to a 
backend
- mod_ssl_tcp: Allows the proxy to route incoming connections based on the SNI 
header (tlsext)

In the following example config, incoming TCP connections are routed based on 
their SNI (the tlsext protocol) to given backend servers, which then complete 
the SSL connections as raw tunnels.

This allows you to use client certificates through the httpd proxy balancer all 
the way to the backend server without the proxy terminating any SSL along the 
way.


  Protocol tlsext

  ServerName jira.example.com

  ProxyPass / tcp://john.example.com:443



  Protocol tlsext

  ServerName www.example.com

  ProxyPass / tcp://erica.example.com:443


In order for mod_ssl_tcp to work, it needs to read ahead to see if any client 
hello message is present, and then set aside any extra data so it could be read 
again. This is fundamentally incompatible with c->data_in_input_filters which 
only allows the core filter to set aside unread data. For this reason the 
ability to set aside data was rolled out to all filters.

mod_ssl_tcp just cares about SNI for now, but could conceivably support APLN 
too, making a configuration something like this:


  Protocol tlsext
  ServerName secure.example.com
  
ProxyPass / tcp://imap.example.com:993
  
  
ProxyPass / tcp://pop3.example.com:995
  


Regards,
Graham
--



httpd-tcp-proxy.patch
Description: Binary data

Re: conn_rec needs a context

[Graham Leggett]

On 02 Mar 2016, at 2:26 AM, Yann Ylavic  wrote:

>>> Would it make sense to add a vector of contexts that same way we have a 
>>> vector of configs, one slot for each module, which will allow any module to 
>>> add a context of it’s own to conn_rec without having to extend conn_rec?
>> 
>> Can't the existing ap_[gs]et_module_config(c->conn_config, ...) be
>> used for that purpose?
> 
> I mean, put the context in the module config?

If we can it would be great - but is there an API guarantee that the memory 
pointed to by the config is a per-connection copy of the config, and not the 
raw underlying server wide config instead?

Regards,
Graham
—

conn_rec needs a context

[Graham Leggett]

Hi all,

In order to get connections to have async behaviour, it must be possible for 
the process_connection hook to exit in the expectation of being called again 
when an async mpm is present - this is easy, the mpms already do that.

The missing bit is that conn_rec needs a context so that when is is called a 
second time it knows what state it is in.

There is a void *ctx variable that was added in r1565657, but I’m not sure it 
is enough.

Would it make sense to add a vector of contexts that same way we have a vector 
of configs, one slot for each module, which will allow any module to add a 
context of it’s own to conn_rec without having to extend conn_rec?

It would be nice to have the same for requests, for the same reason.

Thoughts?

Regards,
Graham
—

Re: state of h2 (long)

[Graham Leggett]

On 29 Feb 2016, at 10:33 PM, Jim Jagielski  wrote:

> I've been digging into how we could better leverage serf on the mod_proxy
> side w/o going the route of mod_serf itself...  Agreed that using
> pollsets et.al. would be useful. Maybe some of the motorz mpm logic
> could be used by mod_proxy…?

I currently think everything we need is now there.

I am in the process of adding TCP protocol support to the core and to the proxy 
(in other words we can be a TCP proxy in addition to an HTTP proxy), and am 
trying to see what problems we run into that stop us doing this asynchronously.

(What I want to achieve is an SNI proxy, in other words a TCP proxy that 
interprets enough of an SSL handshake to extract the SNI header, and use that 
name as the basis of a TCP tunnel to some backend server which then completes 
that SSL handshake. My end goal is that I can TLS client certificate all the 
way to a backend server without terminating that TLS connection on the proxy, 
and not use a dedicated IP per endpoint.)

Regards,
Graham
—

httpd + systemd

[Graham Leggett]

Hi all,

I am trying to come up with a vanilla systemd unit file so that our RPM 
packaging contains a sensible startup on systemd environments, but I’m 
struggling.

With the unit file below the “systemctl restart httpd” command hangs. Usually 
the server starts fine, but then the server is stopped by something a short 
while afterwards for no clear reason. According to google the hang in systemctl 
happens for many people, systemctl is waiting for a message that never comes, 
but the causes are widely varied and I am struggling to figure out exactly what 
is wrong.

What I am trying to achieve is a simple daemon start, with no mod_systemd 
(which isn’t part of core httpd).

Can anyone point out any obvious errors in the following?

[Unit]
Description=The Apache HTTP Server
After=network.target remote-fs.target nss-lookup.target
Documentation=man:httpd(8)
Documentation=man:apachectl(8)

[Service]
Type=forking
PIDFile=/var/run/httpd.pid
EnvironmentFile=/etc/sysconfig/httpd
ExecStart=/usr/sbin/httpd $OPTIONS
ExecReload=/usr/sbin/httpd $OPTIONS -k graceful
ExecStop=/bin/kill -WINCH ${MAINPID}
# We want systemd to give httpd some time to finish gracefully, but still want
# it to kill httpd after TimeoutStopSec if something went wrong during the
# graceful stop. Normally, Systemd sends SIGTERM signal right after the
# ExecStop, which would kill httpd. We are sending useless SIGCONT here to give
# httpd time to finish.
KillSignal=SIGCONT
PrivateTmp=true

[Install]
WantedBy=multi-user.target

Regards,
Graham
—

Re: svn commit: r1725149 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/ap_mmn.h include/http_core.h server/core.c

[Graham Leggett]

On 18 Jan 2016, at 1:40 AM, cove...@apache.org wrote:

> Author: covener
> Date: Sun Jan 17 23:40:09 2016
> New Revision: 1725149
> 
> URL: http://svn.apache.org/viewvc?rev=1725149=rev
> Log:
> allow expressions to be used in SetHandler. Opt-in with expr= prefix.

I am -0.9 on the opt-in with expr= syntax.

Almost all the directives with expression support simply support expressions. 
Except for the one or two that don’t, which forces our end users to either know 
or constantly check the manual to work out which is which. We can’t even 
cleanly detect a possible error - if the person puts in an expression by 
accident where an expr= is needed there is no way the server can detect that. 
This is really nasty.

If we want opt-in expression support we need some kind of Expression +Foo 
statement to control this.

Regards,
Graham
—

Re: async mpms + http2

[Graham Leggett]

On 15 Dec 2015, at 5:44 PM, Stefan Eissing  wrote:

>  However, there is another state in HTTP/2 processing, where a BLOCK_READ is 
> performed: flow control on streaming out responses. Basically, the client has 
> a window of n bytes, the server sends n response bytes and then needs to wait 
> for the client to increase the window again. This would be a good time to 
> resume processing back to the mpm, and initially I did so. But that breaks 
> under load.
> 
>  Under load, the event mpm feels free to close connections in certain states 
> - I think CONN_STATE_CHECK_REQUEST_LINE_READABLE. That is fine for a HTTP/1 
> connection as it always is between requests here. But HTTP/2 would also like 
> to use read events, but unloading a child by closing such connections is not 
> the right thing to do.
> 
>  Is there a way mod_http2 can achieve this or do we need another flag in 
> conn_rec/cs?

Does the CONN_STATE_WRITE_COMPLETION state fit this case?

The WRITE in the above name is a bit of a misnomer, because when mod_ssl 
returns WANTS_READ we return to the CONN_STATE_WRITE_COMPLETION state with the 
sense set to READ. This tells the MPM to block waiting until we’re readable, as 
opposed to block waiting until we’re writable.

> Observation 2: controlled close
>  HTTP/2 would like to send out a last frame when closing a connection in a 
> controlled/timeout way. Do we need another hook for that? Something like 
> "pre_close_connection”?

I’m assuming we would like a module to have control over the sending of that 
frame, if so, it makes sense yes.

> Observation 3: timeouts
>  a. If admins would like to have another Timeout for HTTP/2 connections, they 
> are currently stuck with configuring one Timeout which goes to 
> server_rec->timeout, right? For idle connections, clients already try to keep 
> it open longer. It would be nice to give admins a choice here for different 
> values based on the selected protocol. How to best do this?

Perhaps just as we can override the sense of the connection before returning, 
we might be able to modify the timeout before returning.

>  b. Handling own timeouts: if I want, during processing the connection, a 
> different socket timeout, I can just set it using apr_socket_timeout_set()? 
> Shall I restore it back to server->timeout before returning?

That feels wrong, ideally we want to be formally telling the MPM “give us this 
timeout” rather than doing something sneaky behind it’s back.

>  c. What would be the best way to wait for a file description *and* 
> conditional event simultaneously? mod_http2 has now a sort of pool loop with 
> double backup timer for situations where it needs to watch both client and 
> backend processes.

I’m not sure how portable it is watching for file descriptors. I have used 
libev to do this in the past and it was quite involved. However - if we can 
support this it would be really useful.

Regards,
Graham
—

Re: scoreboard and http2

[Graham Leggett]

On 09 Dec 2015, at 3:49 PM, Stefan Eissing  wrote:

> For the next 2.4 release, I would like to add scoreboard support to mod_http2.
> 
> There are several options for doing so and I would like your feedback on which
> route we should take. I try to outline what options I see:
> 
> A) Make mod_http2 workers general MPM workers. They would then become
>   part of scoreboard without changes to the board itself.

It is the goal of the MPM to care about the architecture of the machine, and 
know about processes/threads/etc.

Option A is the only real option - we cannot allow the h2 stuff to mess up the 
clean separation between the architecture of each platform and the core of the 
server itself.

Regards,
Graham
—

Re: AW: reverse proxy wishlist

[Graham Leggett]

On 3 Dec 2015, at 15:50, Plüm, Rüdiger, Vodafone Group 
 wrote:

> How about an async proxy that frees up the thread while waiting on a slow 
> backend?

This is generic httpd thing, not restricted to mod_proxy.

From what I have seen it shouldn't be difficult, we just need a way for EAGAIN 
to be properly handled with the ability to skip forward back to a hook when we 
come back.

I looked at doing this in the filter stack, which is also possible, but doing 
this generically across the server is better.

Regards,
Graham
--

Re: issue with "uncacheable" cache revalidations

[Graham Leggett]

On 1 Dec 2015, at 15:50, Eric Covener  wrote:

> I've been looking at an issue where a revalidation
> of a stale cache entry fails because the 304 response
> on the revalidation doesn't contain its own CC or Expires
> but the URL has a query string.  This kicks the revalidation
> response out of the cache save filter early and we never
> get to look at the stale handle and update the headers.

As I recall there is a specific code path for responses that say "we've asked 
for revalidation, revalidation was successful but not cacheable. Return cached 
data but invalidate it, it is the best we can do".

Regards,
Graham
--

Re: svn commit: r1715938 - /httpd/httpd/trunk/modules/cache/cache_util.c

[Graham Leggett]

On 24 Nov 2015, at 6:36 PM, Yann Ylavic  wrote:

> Sure, but my point is that the worst case is likely depend on the
> application,

It will depend on the application, yes.

> eg:
> 
>case 'm':
>case 'M':
>if (!strncmp(token, "max-age", 7)
>|| !ap_casecmpstrn(token, "max-age", 7)) {
>...
>}
>else if (!strncmp(token, "max-stale", 9)
> || !ap_casecmpstrn(token, "max-stale", 9)) {
>...
>}
>else if (!strncmp(token, "min-fresh", 9)
> || !ap_casecmpstrn(token, "min-fresh", 9)) {
>...
>}
>else if (!strcmp(token, "max-revalidate")
> || !ap_casecmpstr(token, "must-revalidate")) {

Oops - max-revalidate != must-revalidate

>...
>}
>else if ...
> 
> is going to be costly when matched against "must-revalidate", or worse
> "my-token”.

In that case make it cheaper for those cases.

Have the length handy to check for a minimum-sane-length, then do a switch on 
the 4th character.

> We could use all str[n]cmp() first, but still it's a lot of
> comparisons, and now duplicated code too.

The duplicated code is not a worry, the worry is to ensure the most common 
cases take the fastest path.

Regards,
Graham
—

Re: svn commit: r1715938 - /httpd/httpd/trunk/modules/cache/cache_util.c

[Graham Leggett]

On 24 Nov 2015, at 6:15 PM, Yann Ylavic  wrote:

> Not sure:
>if (!strcmp(h, "max-age")
>|| ap_cmpcasestr(h, "max-age"))
> is likely to be a bit faster than a single ap_cmpcasestr() when it
> matches, but much slower when it does not.

Yep, that’s the point.

The vast majority of comparisons are lowercase for tokens like this. Might as 
well test that fast path first before testing the worst case scenario.

Regards,
Graham
—

Re: svn commit: r1715938 - /httpd/httpd/trunk/modules/cache/cache_util.c

[Graham Leggett]

On 24 Nov 2015, at 5:21 PM, Jim Jagielski  wrote:

> For these types of paths, where we use a switch/case against
> the 1st char of the token, the real reason why we used that
> impl was to avoid the (expected) expensive string comparison.
> 
> This is really no longer an issue. Sure, we could still use
> this "fast path" short-circuit, but even that is non-optimal.
> For the below, what we really should be testing is:
> 
>   if (!ap_casecmpstr(token+1, "ublic")) {
> 
> for example.
> 
> So the question is: Do we remove these silly fast-paths
> that no longer make sense[1] or optimize for the "we know the
> 1st char" case ala the above?

A further optimisation - in many cases the fast path is really a case sensitive 
string comparison, given that the vast majority of the time the case being used 
is the case quoted in the spec.

In other words, while mAx-AgE is valid, it is almost always max-age.

Does it make sense to do a case sensitive comparison first, and then fall back 
to case insensitive?

Regards,
Graham
—

Re: 2.4.18?

[Graham Leggett]

On 18 Nov 2015, at 9:11 AM, Noel Butler  wrote:

> absolutely not! I personally only update  phpmyadmin once, on initial major 
> release, because I (like many others) were so of updating it every few days .

We’re catering for everybody here, not just your unique use case.

Regards,
Graham
—

Re: CentOS 7 and test framework

[Graham Leggett]

On 18 Nov 2015, at 5:45 PM, Jim Jagielski  wrote:

> Anyone else having troubles convincing the test framework
> in centOS 7 to recreate the various SSL certs and stuff???
> 
> usually a 'make realclean' will delete them and a t/TEST -clean
> followed by t/TEST will recreate them. No luck. Nothing
> removes 'em :/

I’ve had this before, but never found what caused it :(

Regards,
Graham
—

Re: 2.4.18?

[Graham Leggett]

On 17 Nov 2015, at 2:18 PM, Noel Butler  wrote:

> You run into complacency dangers if you end up in a "release very often" 
> mode, take phpmyadmin as one example, most admins I know gave up updating it, 
> because there were updates every week, sometimes  every few days, Marc has 
> accepted this is a serious problem and unless a critical security bug is 
> found, , normal bug fixes releases will be only once per month if need be, 
> take dovecot, not so bad now days but in years gone by, it too released 
> weekly or bi-weekly and a lot of people got pissed and stopped updating it 
> too.

I’m not understanding your point - you’re saying normal bug fix releases once a 
month is fine, but us doing so after 5 weeks isn’t fine?

We’ve just released HTTP/2 support for the very first time. People want to use 
it, people want to see problems in it fixed. I don’t see the number of releases 
as excessive at all.

Regards,
Graham
—

Re: 2.4.18?

[Graham Leggett]

On 17 Nov 2015, at 07:13, Noel Butler  wrote:

> ??? We only had 2.4.17  5 weeks ago, why the rush?

We have improvements, why make people wait?

Regards,
Graham
--

Re: Moderations for modules.apache.org

[Graham Leggett]

On 09 Nov 2015, at 2:41 PM, Daniel Gruno  wrote:

> You're welcome to try to clean it up ;)
> make a user account on the system and give me the UID of that user (the
> ID, not the username - there are tens of thousands of users, so I can't
> see them all in the admin interface anymore).

:)

> I am contemplating removing all users/mods and adding some recaptcha
> stuff to it soon, but enotime right now.

Is there a way to leverage LDAP at all? (Or whatever backs the JIRA et al 
instances)

Regards,
Graham
—

Re: Moderations for modules.apache.org

[Graham Leggett]

On 06 Nov 2015, at 6:55 PM, Daniel Gruno  wrote:

> I'm sorry to say modules.apache.org is so bot/spam infested now, that
> it's impossible to moderate it unless I spend more than an hour every
> day going through all the fake modules and users added on a daily basis.
> 
> I am contemplating scrapping it entirely, or possibly creating a new
> system at some point, with stronger anti-spam measures.

Need a hand with moderation?

Having what looks to be the formal module search engine being frozen in time is 
a problem for us, it makes us look stale when we aren’t.

Regards,
Graham
—

Moderations for modules.apache.org

[Graham Leggett]

Hi all,

I've had a module waiting to be approved at modules.apache.org for a while, 
anyone know who the moderator is?

Regards,
Graham
--

Re: trailer

[Graham Leggett]

On 05 Nov 2015, at 3:04 PM, Stefan Eissing  wrote:

> Looking at the source regarding trailer handling, am I correct in seeing:

> - no support for sending trailer to the client in http core

Did a quick look at this and it looks like it, yes.

Ideally the CHUNK filter should “do the right thing” with respect to 
r->trailers_in (and _out), both for HTTP/1.1 and HTTP/2.

Regards,
Graham
—

Re: Is Apache getting too patchy?

[Graham Leggett]

On 27 Oct 2015, at 12:40 AM, Tim Bannister  wrote:

>> That may not be easy.  You need to find someone who'll be interested in an 
>> idea or patch, and has the time to review it.
>> Plus, the community as a whole to agree it's a good idea, or at least not 
>> actively oppose it.
>> 
>> I wonder if workflow would be improved if we had named maintainers for 
>> particular parts of the codebase - for example individual modules?  Not 
>> necessarily to do all the work, but to take primary responsibility to see 
>> that your ideas don't just fall through the gaps and get ignored?
> 
> How does the word “sponsor” sound?

It rhymes with “bureaucracy”. :)

Remember the end goal is to improve quality without causing stagnation, and 
giving people the impression that they need to ask a “sponsor” first who may no 
longer be involved with the project will cause that stagnation.

The most exciting work on httpd for me has been Christophe Jaillet’s memory 
optimisation patches. With each patch the code gets cleaner and the server gets 
faster. We need more of this kind of stuff.

Regards,
Graham
—

Re: Improve Apache performance on high load (prefork MPM) with multiple Accept mutexes (Patch attached)

[Graham Leggett]

On 26 Oct 2015, at 10:45 AM, Yehezkel Horowitz  wrote:

> Any chance someone could take a short look and provide me a feedback (of any 
> kind)?
>  
> I know your focus is on 2.4 and trunk, but there are still many 2.2 servers 
> out there…
>  
> Patch attached again for you convenience.…

Just to clarify, all updates to httpd need to be made to trunk first, which 
then allow it to be backported to v2.4, and then v2.2.

The reason for this is we don’t want features added to v2.2 that then 
subsequently vanish when v2.4 comes out (and so on).

The following patch was recently backported to v2.4, how similar is your patch 
to this one?

  *) MPMs: Support SO_REUSEPORT to create multiple duplicated listener
 records for scalability. [Yingqi Lu ,
 Jeff Trawick, Jim Jagielski, Yann Ylavic]

Regards,
Graham
—

Re: Question about CACHE_SEPARATOR in modules/cache/cache_util.h

[Graham Leggett]

On 26 Oct 2015, at 9:05 AM, Christophe JAILLET  
wrote:

> in modules/cache/cache_util.h, CACHE_SEPARATOR is defined as:
> 
> #define CACHE_SEPARATOR ",   "
> 
> 
> I don't see any reason to have 3 spaces here.
> It is only used within calls to 'cache_strqtok' and scanning 3 times for the 
> same thing is just a waste of time.
> 
> Did I miss something obvious, or can it be turned in:
> 
> #define CACHE_SEPARATOR ", “

Double check whether the spaces are significant, it might be a space and a tab 
(which would be weird, because it should just say /t then).

This might have RFC compliance issues if it was changed, we need to verify what 
the code does.

Regards,
Graham
—

Re: Question about CACHE_SEPARATOR in modules/cache/cache_util.h

[Graham Leggett]

On 26 Oct 2015, at 2:15 PM, Rainer Jung  wrote:

> The line goes back to
> 
> https://bz.apache.org/bugzilla/show_bug.cgi?id=50199
> 
> where Nick reported the problem and Graham provided the initial patch.
> 
> The defined tokenizer chars were "comma-space-space-space" from the 
> beginning. But I think Graham pointed into the right direction, probably it 
> should have been "comma-space-tab”.

Following the links further into the RFC, this corresponds with LWS, which is 
either a space or a tab:

#define CACHE_SEPARATOR “, \t"

Regards,
Graham
—

Re: [users@httpd] Chunked transfer delay with httpd 2.4 on Windows.

[Graham Leggett]

On 22 Oct 2015, at 5:31 PM, Andy Wang  wrote:

> I already had to do that.  We have this weird scenario where when our 
> installer installs httpd, httpd hangs.  Packet captures show the request 
> arriving but no ACK or any form of response.
> 
> If I kill the installer process, the problem goes away.  I have no idea how 
> or why that could possibly occur, but Acceptfilter http none resolves that 
> issue.

Sounds like a request that the http accept filter doesn’t believe is complete?

Could be an RFC violation on the client side.

Regards,
Graham
—

Re: buckets and connections (long post)

[Graham Leggett]

On 22 Oct 2015, at 6:04 PM, Stefan Eissing  wrote:

>> mod_ssl already worries about buffering on it’s own, there is no need to 
>> recreate this functionality. Was this not working?
> 
> As I wrote "it has other bucket patterns", which do not get optimized by the 
> coalescing filter of mod_ssl.

Then we must fix the coalescing filter in mod_ssl.

Regards,
Graham
—

Re: buckets and connections (long post)

[Graham Leggett]

On 22 Oct 2015, at 6:03 PM, Stefan Eissing  wrote:

> This is all true and correct - as long as all this happens in a single 
> thread. If you have multiple threads and create sub pools for each from a 
> main pool, each and every create and destroy of these sub-pools, plus any 
> action on the main pool must be mutex protected. I found out. 

Normally if you’ve created a thread from a main pool, you need to create a pool 
cleanup for that thread off the main pool that is registered with 
apr_pool_pre_cleanup_register(). In this cleanup, you signal the thread to shut 
down gracefully and then apr_thread_join to wait for the thread to shut down, 
then the rest of the pool can be cleaned up.

The “pre” is key to this - the cleanup must run before the subpool is cleared.

> Similar with buckets. When you create a bucket in one thread, you may not 
> destroy it in another - *while* the bucket_allocator is being used. 
> bucket_allocators are not thread-safe, which means bucket_brigades are not, 
> which means that all buckets from the same brigade must only be used inside a 
> single thread.

“…inside a single thread at a time”.

The event MPM is an example of this in action.

A connection is handled by an arbitrary thread until that connection must poll. 
At that point it goes back into the pool of connections, and when ready is 
given to another arbitrary thread. In this case the threads are handled “above” 
the connections, so the destruction of a connection doesn’t impact a thread.

> This means for example that, even though mod_http2 manages the pool lifetime 
> correctly, it cannot pass a response bucket from a request pool in thread A 
> for writing onto the  main connection in thread B, *as long as* the response 
> is not complete and thread A is still producing more buckets with the same 
> allocator. etc. etc.
> 
> That is what I mean with not-thread-safe.

In this case you have different allocators, and so must pass the buckets over.

Remember that being lock free is a feature, not a bug. As soon as you add 
mutexes you add delay and slow everything down, because the world must stop 
until the condition is fulfilled.

A more efficient way of handling this is to use some kind of IPC so that the 
requests signal the master connection and go “I’ve got data for you”, after 
which the requests don’t touch that data until the master has said “I;ve got 
it, feel free to send more”. That IPC could be a series of mutexes, or a socket 
of some kind. Anything that gets rid of a global lock.

That doesn’t mean request processing must stop dead, that request just gets put 
aside and that thread is free to work on another request.

I’m basically describing the event MPM.

Regards,
Graham
—

Re: buckets and connections (long post)

[Graham Leggett]

On 22 Oct 2015, at 5:43 PM, Stefan Eissing  wrote:

>> The blocking read breaks the spirit of the event MPM.
>> 
>> In theory, as long as you enter the write completion state and then not 
>> leave until your connection is done, this problem will go away.
>> 
>> If you want to read instead of write, make sure the CONN_SENSE_WANT_READ 
>> option is set on the connection.
> 
> This does not parse. I do not understand what you are talking about. 
> 
> When all streams have been passed into the output filters, the mod_http2 
> session does a 
> 
>status = ap_get_brigade(io->connection->input_filters,...)  (h2_conn_io.c, 
> line 160)
> 
> similar to what ap_read_request() -> ap_rgetline_core() does. (protocol.c, 
> line 236)
> 
> What should mod_http2 do different here?

What ap_read_request does is:

- a) read the request (parse)
- b) handle the request (make decisions on what to do, internally redirect, 
rewrite, etc etc)
- c) exit, and let the MPM complete the request in the write_completion phase.

What you want to do is move the request completion into a filter, like 
mod_cache does. You start by setting up your request, you parse headers, you do 
the HTTP2 equivalent of ap_read_request(), then you do the actual work inside a 
filter. Look at the CACHE_OUT and CACHE_SAVE filters as examples.

To be more specific, in the handler that detects HTTP/2 you add a filter that 
processes the data, then write an EOS bucket to kick off the process and leave. 
The filter takes over.

The reason for this is you want to escape the handler phase as soon as 
possible, and leave the MPM to do it’s work.

Regards,
Graham
—

Re: buckets and connections (long post)

[Graham Leggett]

On 22 Oct 2015, at 5:55 PM, Stefan Eissing  wrote:

>> With the async filters this flow control is now made available to every 
>> filter in the ap_filter_setaside_brigade() function. When mod_http2 handles 
>> async filters you’ll get this flow control for free.
> 
> No, it will not. The processing of responses is very different.
> 
> Example: there is individual flow control of responses in HTTP/2. Clients do 
> small window sizes on images, like 64KB in order to get small images 
> completely or only the meta data of large ones. For these large files, the 
> client does not send flow-control updates until it has received all other
> resources it is interested in. *Then* it tells the server to go ahead and 
> send the rest of these images.
> 
> This means a file bucket for such images will hang around for an indefinite 
> amount of time and a filter cannot say, "Oh, I have n file buckets queued, 
> let's block write them first before I accept more." The server cannot do that.

What you’re describing is a DoS.

A client can’t tie up resources for an arbitrary amount of time, the server 
needs to be under control of this. If a client wants part of a file, the server 
needs to open the file, send the part, then close the file and be done. If the 
client wants more, then the server opens up the file again, sends more, and 
then is done.

> I certainly do not want to reinvent the wheel here and I am very glad about 
> any existing solution and getting told how to use them. But please try to 
> understand the specific problems before saying "we must have already a 
> solution for that, go find it. you will see…"

The http2 code needs to fit in with the code that is already there, and most 
importantly it needs to ensure it doesn’t clash with the existing mechanisms. 
If an existing mechanism isn’t enough, it can be extended, but they must not be 
bypassed.

The mechanism in the core keeps track of the number of file buckets, in-memory 
buckets and requests “in flight”, and then blocks if this gets too high. Rather 
block and live to fight another day than try an open too many files and get 
spurious failures as you run out of file descriptors.

The async filters gives you the ap_filter_should_yield() function that will 
tell you if downstream is too full and you should hold off sending more data. 
For example, don’t accept another request if you’ve already got too many 
requests in flight.

Regards,
Graham
—

Re: master connection + mod_ssl + http2

[Graham Leggett]

On 21 Oct 2015, at 2:42 PM, Stefan Eissing  wrote:

> The basic changes:
> 1. conn_rec->master is NULL for HTTP/1.1 connections, but points to the 
> "real" connection for HTTP/2 requests.
> 2. mod_ssl no longer initalizes any SSLConnRec* for slave connections 
> (conn_rec->master != NULL)
> 3. lookup of ssl variables uses the master's sslconn->ssl if none is found on 
> the connection itself
> 4. ssl_hook_Access() that checks renegotiation fails with a FORBIDDEN for a 
> slave connection with a note for the reason.
>   This should allow mod_http2 to generate the correct HTTP/2 stream error
> 5. ssl_hook_ReadReq() that checks for wrong host names now has an additional 
> check for TLS compatiblity which compares
>   protocol, cipher suite, certificate and key file/path names and verify mode 
> of the request server against the
>   handshake server. This compatibility is strict equality and not as 
> sophisticated as the renegotiation checks.
> 
> With these changes, mod_http2 has less work for the slave connection setup 
> and no longer needs to disable ssl for those. While mod_ssl continues to be 
> ignorant of mod_http2, as the same restrictions would apply to any protocol 
> with slave connections. With a minor bump in MMN we can have this in the next 
> 2.4.

Not having looked at the patch yet, the above seems to make sense.

Regards,
Graham
—

Re: buckets and connections (long post)

[Graham Leggett]

On 21 Oct 2015, at 4:18 PM, Stefan Eissing  wrote:

> How good does this mechanism work for mod_http2? On the one side it's the 
> same, on the other quite different.
> 
> On the real, main connection, the master connection, where the h2 session 
> resides, things are
> pretty similar with some exceptions:
> - it is very bursty. requests continue to come in. There is no pause between 
> responses and the next request.
> - pauses, when they happen, will be longer. clients are expected to keep open 
> connections around for
>  longer (if we let them).
> - When there is nothing to do, mod_http2 makes a blocking read on the 
> connection input. This currently
>  does not lead to the state B) or C). The worker for the http2 connection 
> stays assigned. This needs
>  to improve.

The blocking read breaks the spirit of the event MPM.

In theory, as long as you enter the write completion state and then not leave 
until your connection is done, this problem will go away.

If you want to read instead of write, make sure the CONN_SENSE_WANT_READ option 
is set on the connection.

(You may find reasons that stop this working, if so, these need to be isolated 
and fixed).

> This is the way it is implemented now. There may be other ways, but this is 
> the way we have. If we
> continue along this path, we have the following obstacles to overcome:
> 1. the master connection probably can play nicer with the MPM so that an idle 
> connection uses less
>   resources
> 2. The transfer of buckets from the slave to the master connection is a COPY 
> except in case of
>   file buckets (and there is a limit on that as well to not run out of 
> handles).
>   All other attempts at avoiding the copy, failed. This may be a personal 
> limitation of my APRbilities.

This is how the proxy does it.

Buckets owned by the backend conn_rec are copied and added to the frontend 
conn_rec.

> 3. The amount of buffered bytes should be more flexible per stream and 
> redistribute a maximum for 
>   the whole session depending on load.
> 4. mod_http2 needs a process wide Resource Allocator for file handles. A 
> master connection should
>   borrow n handles at start, increase/decrease the amount based on load, to 
> give best performance
> 5. similar optimizations should be possible for other bucket types (mmap? 
> immortal? heap?)

Right now this task is handled by the core network filter - it is very likely 
this problem is already solved, and you don’t need to do anything.

If the problem still needs solving, then the core filter is the place to do it. 
What the core filter does is add up the resources taken up by different buckets 
and if these resources breach limits, blocking writes are done until we’re 
below the limit again. This provides the flow control we need.

With the async filters this flow control is now made available to every filter 
in the ap_filter_setaside_brigade() function. When mod_http2 handles async 
filters you’ll get this flow control for free.

> 6. pool buckets are very tricky to optimize, as pool creation/destroy is not 
> thread-safe in general
>   and it depends on how the parent pools and their allocators are set up. 
>   Early hopes get easily crushed under load.

As soon as I see “buckets aren’t thread safe” I read it as “buckets are being 
misused” or “pool lifetimes are being mixed up".

Buckets arise from allocators, and you must never try add a bucket from one 
allocator into a brigade sourced from another allocator. For example, if you 
have a bucket allocated from the slave connection, you need to copy it into a 
different bucket allocated from the master connection before trying to add it 
to a master brigade.

Buckets are also allocated from pools, and pools have different lifetimes 
depending on what they were created for. If you allocate a bucket from the 
request pool, that bucket will vanish when the request pool is destroyed. 
Buckets can be passed from one pool to another, that is what “setaside” means.

It is really important to get the pool lifetimes right. Allocate something 
accidentally from the master connection pool on a slave connection and it 
appears to work, because generally the master outlives the slave. Until the 
master is cleaned up first, and suddenly memory vanishes unexpectedly in the 
slave connections - and you crash.

There were a number of subtle bugs in the proxy where buckets had been 
allocated from the wrong pool, and all sorts of weirdness ensued. Make sure 
your pool lifetimes are allocated correctly and it will work.

> 7. The buckets passed down on the master connection are using another buffer 
> - when on https:// -
>   to influence the SSL record sizes on write. Another COPY is not nice, but 
> write performance
>   is better this way. The ssl optimizations in place do not work for HTTP/2 
> as it has other
>   bucket patterns. We should look if we can combine this into something 
> without COPY, but with
>   good sized SSL 

Re: Forwarded buckets' lifetime (was: [Bug 58503] segfault in apr_brigade_cleanup()...)

[Graham Leggett]

On 20 Oct 2015, at 10:42 AM, Stefan Eissing  
wrote:

>> So if at all a change for trunk only that requires a very deep review of all 
>> brigade usages.
> 
> As I learned, bucket brigades are very optimized for the common HTTP/1 case: 
> appearing during one
> request and going away together with the request - preferably with only one 
> thread involved.
> 
> When (sub-)request are used - and you may look at HTTP/2 processing as 
> involving sub-requests, this
> model no longer holds and such functionality has a heavy price to pay. It 
> needs to step very
> carefully to avoid the pitfalls of pool destroy cleanups on EOR.

I’m not fully following why this involves a heavy price - we already have (and 
have solved) the same problem in the proxy. The proxy welds backend conn_rec’s 
to frontend conn_rec’s, while mod_http2 welds a child conn_rec to a parent 
conn_rec.

In the proxy case the EOR bucket only ever lives in one conn_rec, as long as 
you stick to that - ie destroy the EOR bucket in the child conn_rec, it should 
work fine.

> Given the complexity and the modules out there, I do not think it wise to 
> change the behaviour
> of the existing apr_bucket_brigade in 2.4.x. I think it is better to think of 
> an alternate bucket
> brigade design and maybe how that could live next to the old one in 2.4.x. Or 
> skip 2.4.x entirely
> for this.

I am very wary of alternate designs that throw away the investment the 
community has made in existing code, existing tooling, and existing knowledge. 
I would far rather we fix these bugs in the current design than try come up 
with alternatives.

So far, all the work I’ve done in expanding support for async filters has 
simply involved finding and fixing bugs.

Regards,
Graham
—

Re: master connections

[Graham Leggett]

On 20 Oct 2015, at 6:14 PM, Stefan Eissing  wrote:

> In /trunk we have the master connection member of conn_rec which we cannot 
> backport to 2.4.x - as I understand.

We can do, as long as the value goes at the very end of the struct, and we have 
a minor MMN bump in ap_mmn.h.

Regards,
Graham
—

XSLT filter for httpd

[Graham Leggett]

Hi all,

In the light of the move to simple services that talk XML/JSON and HTML and 
Javascript based clients that become more and more capable, I find myself 
wanting an XSLT filter quite often these days to sit in the middle and 
translate between the two.

As a complement to mod_xmlenc, would it make sense to include an XSLT filter in 
httpd out the box?

Regards,
Graham
—

Re: XSLT filter for httpd

[Graham Leggett]

On 19 Oct 2015, at 3:20 PM, Nick Kew  wrote:

> There are several old modules: for example mod_transform.
> I expect they still serve for sites without i18n requirements.
> One option would be to overhaul that.
> 
> Note, mod_transform is GPL.  Originally my decision when I released
> its earlier predecessor, before I was part of the dev@httpd team.
> I'd be happy to re-license it as Apache, and I don't think
> any of my co-developers would object.

I’ve been using mod_transform v0.6.0 for a while, and have wanted to develop it 
further. It would be a great starting point.

Regards,
Graham
—

Re: H2 compatible ciphers (was: svn commit: r1708593)

[Graham Leggett]

On 16 Oct 2015, at 12:56 PM, Stefan Eissing  
wrote:

> I am not blacklisting ciphers for the whole server. I try to define
> the security settings required for HTTP/2 as defined in the standard -
> as a configurable directive.
> 
> There is no problem with denying HTTP/2 support for an IE8.

I am wondering whether the cipher blacklist shouldn’t be a configurable list 
with a default set of RFC compliant values in the default config file, perhaps 
with shortcuts like naming a blacklist after an RFC.

Fitting this in with the existing infrastructure this could be as simple as 
extending the SSLCipherSuite directive to support this:

SSLCipherSuite -RFC7540

Maybe this is actually an openssl problem rather than an httpd problem, it 
could be that openssl needs to be taught how to blacklist RFC7540 as a group.

Regards,
Graham
—

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Graham Leggett]

On 11 Oct 2015, at 7:00 PM, Stefan Eissing  wrote:

> Ok, analyzed the code. Here is what seems to be happening:
> 
> - mod_http2, in the connection hook, does a blocking, speculative read to
>  a) make sure the ALPN has been triggered

Looking at the code inside the event MPM that calls 
ap_run_process_connection(), it looks like you can just do a non blocking read, 
and if you haven’t received enough bytes, return DECLINED and expect to be 
called again.

This presupposes that other connection filters aren’t trying to be excessively 
cleaver by stealing your connection away from you and then responding to your 
data (for example by error-ing out), which they may do by doing blocking reads.

Regards,
Graham
—

Re: about those async filters

[Graham Leggett]

On 07 Oct 2015, at 4:30 PM, Stefan Eissing  wrote:

> In http2 land, the request happen on "pseudo" connections, connections 
> properly created by ap_run_create_connection(), but with own filters of type 
> AP_FTYPE_PROTOCOL and AP_FTYPE_NETWORK, registered by mod_h2. 
> 
> These filters copy the data (or move the files) across the processing filters 
> thread into the h2 multiplexer (h2_mplx) where the master connection thread 
> reads it and send it out to the client, properly framed for HTTP/2.

Having gone through this in a bit more detail (and slept on it) I 
simultaneously see how we needed to do this to reach this point given the 
limitations of the server, but at the same time this approach causes a number 
of problems that we need to solve in the medium term (which is doable from what 
I can see).

What mod_http2 is doing is creating it’s own “mini-worker-mpm” inside the 
module, and this creates a number of problems. The first is that code out there 
that cares about being single threaded (historically php, for example) suddenly 
finds itself running in a “pseudo-worker” MPM when the actual MPM in use is 
prefork. This will cause unexpected breakage for people trying to use http2.

The second problem is that with mod_http2 being a “pseudo-mpm”, it needs to 
emulate all the functions of an mpm in order to correctly do so. I believe the 
problems with async filters are a manifestation of mod_http2 not implementing 
all the capabilities of an MPM. We could try and make mod_http2 do so, but I 
think ultimately we’re making life hard for ourselves - we should really be 
leveraging the MPMs directly to make workers for us instead of trying to bake 
workers of our own.

So, bringing this down to actual practical stuff we can do to make this happen, 
the basic problems to solve are:

- The master connection currently runs under an existing MPM, and this works 
fine today.
- The slave connection needs to run under an existing MPM instead of the 
“pseudo MPM” it works under today.
- In theory, we can let the master connection and slave connection talk to each 
other over two pairs of sockets otherwise created with socketpair() (workaround 
for Windows at 
http://code.activestate.com/recipes/525487-extending-socketsocketpair-to-work-on-windows/).
- We would need a mechanism to inject the just-created socket into the MPM’s 
worker queue (ie in event MPM, call push2worker()), that would be an additional 
call to MPMs that could support it.
- When the slave connection is accepted by the worker, the normal request 
acceptance mechanism applies. We might signal the connection is an HTTP2 slave 
connection on the request line in some way, or perhaps emulate HTTP/1.1 (not 
sure if workable), will need to come up with a solution.

Regards,
Graham
—