Segfault in mod_include (PR 62855)

[Micha Lenk]

Hi all,

a few weeks ago Ewald was filing this Bugzilla but didn't get any 
reaction yet. https://bz.apache.org/bugzilla/show_bug.cgi?id=62855


As this looks like it is easy to fix, could someone please take a look?

Regards,
Micha

Exposing SSL certificates on SNI mismatch

[Micha Lenk]

Hi all,

I have a customer asking about whether the SSL handshake can be made to 
fail in case the SNI from the "Client Hello" message doesn't match at 
all any server name of the configured virtual hosts. E.g. consider a 
setup like this


DNS records:
domain-a.tld resolves to 
domain-b.tld also resolves to 

Listen :443 https
Listen :80  http

:80>
  ServerName domain-a.tld
  ...


:80>
  ServerName domain-b.tld


:443>
  ServerName domain-b.tld
  SSLEngine On
  SSLCertificateFile <...a certificate file only valid for domain-b.tld>
  ...


I.e. no virtual host with SSL enabled for domain-a.tld exists.

In this setup the customer wants the server to not expose the SSL 
certificate of other, unrelated virtual hosts in case a client/browser 
tries to access https:/domain-a.tld/ (which isn't configured in Apache).


Is this currently possible? If yes, in 2.4.x or only trunk?

I remember a discussion thread of this topic floating by, but I can't 
find it anymore. Does anybody else maybe have a pointer to some public 
mail archive?


Thanks a lot in advance for any comment,
Micha

Re: Wherefor 2.4.36?

[Micha Lenk]

On 08/06/2018 07:37 PM, William A Rowe Jr wrote:

It appears 2.4.34 is unusable [...]


BTW: How usable is it compared to trunk?

Regards,
Micha


... poking for a 2.6 release.

Re: [apache/httpd] Add Option to generate multiple error logs of different format in Apache 2.4.x (#52)

[Micha Lenk]

Hi all,

on Github the pull request #52 https://github.com/apache/httpd/pull/52 
looks like it has gone wild, receiving new commits from asfgit every 
once in a while. The pull request itself looks unusable. Does anybody 
know who created it and who is updating it?


Regards,
Micha


On 06/05/2018 08:59 PM, asfgit wrote:

@asfgit  pushed 1 commit.

  * 2274d83  Use define
for serverroot with Windows conf files.

—
You are receiving this because you are subscribed to this thread.
View it on GitHub 
 
or mute the thread 
.

Re: mod_proxy_html and special characters

[Micha Lenk]

Hi Eric,

On 05/25/2018 06:57 PM, Eric Covener wrote:

http://internal/!%22%23$/;>A link with special characters
>> ProxyHTMLURLMap "http://internal/!\"#$/; "http://external/!\"#$/;

Is it reasonable to expect mod_proxy_html to rewrite URL encoded URLs as
well?

> IMO no, I don't think the literals in the first argument should be
expected to match the URL-encoded content


The reason I am asking this is, because for Location matching, Apache 
httpd apparently does map a request with a URL encoded path to the 
non-encoded configured path. For example, if I have configured in a 
virtual host:


  
ProxyPass "http://internal/!\"#$/;
ProxyHTMLURLMap "http://internal/!\"#$/; "http://external/!\"#$/;
...
  

... then for matching the location container it does not matter whether 
the path of the request is URL encoded or not.


I consider this behavior a bit inconsistent. URL-encoded requests get 
proxied to the internal resource as if they were not URL-encoded. But 
URL-encoding a few characters in the path is sufficient to bypass HTML 
rewriting.


Regards,
Micha

mod_proxy_html and special characters

[Micha Lenk]

Hi all,

I'm currently facing an issue where the directive ProxyHTMLURLMap does 
not work. And I am not sure whether that is by design or not, and where 
I would appreciate some feedback.


Let's assume an imaginary backend server delivers a HTML page that 
contains a link like this:


http://internal/!%22%23$/;>A link with special characters

Please note that %22 is the double quote that needs to be encoded to not 
break the HTML, and %23 is the '#' character, which we don't want to get 
treated as anchor in this case. So, the unencoded URL would look like this:


http://internal/!"#$/

Now, Apache configured as reverse proxy should rewrite this link to 
http://external/!"#$/ (or http://external/!%22%23$/), but not any other 
links outside the sub directory /!"#$/ (nor /!%22%23$/). An imaginary 
configuration to achieve that and to showcase the issue I am trying to 
get feedback on looks like this:


ProxyHTMLURLMap "http://internal/!\"#$/; "http://external/!\"#$/;

Please note that the double quote is only escaped here with a backslash 
to cater for the Apache configuration syntax requirements. This does not 
work, i.e. the URL in the HTML document doesn't get rewritten.


Let's try to better understand what exactly is happening here. Looking 
into the code of mod_proxy_html.c (trunk, SVN rev. 1832252), this is 
where the string comparison happens:


 524  s_from = strlen(m->from.c);
 525  if (!strncasecmp(ctx->buf, m->from.c, s_from)) {
 ...  ... do the string replacement ...


... where ctx->buf is the URL found in the HTML document, and m->from.c 
is the first configured argument of ProxyHTMLURLMap. So, if the latter 
is a prefix of the first, this condition should be true and the string 
replacement should happen. When the expected string replacement doesn't 
happen, the condition is false and the values of the variables are:


ctx->buf  = http://internal/!%22%23$/
m->from.c = http://internal/!"#$/

So, the strings don't match and are not replaced for that reason.

Going forward I am not interested in finding a work around for this, but 
more how to approach a fix (if this is a bug at all).


Is it reasonable to expect mod_proxy_html to rewrite URL encoded URLs as 
well?


Let's assume this needs to be fixed. To make the strings match, we could 
either URL escape the value from the Apache directive ProxyHTMLURLMap, 
or URL temporarily URL-decode the string found in the HTML document just 
for the purpose of the string comparison. What is the right thing to do?


If you have managed read all this down to this line, I am curious about 
your feedback. :)


Regards,
Micha

Re: Experimental C unit test suite available for hacking

[Micha Lenk]

Hi Yann,

On 05/24/2018 12:31 PM, Yann Ylavic wrote:

Well, first things first. Let's first fix trunk to be buildable again on
build systems that really only link the needed symbols and thus rely on the
correct library order during linking.


I think this is*the*  dependency issue, the order in
PROGRAM_DEPENDENCIES should make modules depend on core and not the
other way around.


Yes, sounds reasonable. With the little knowledge about the build system 
I was just blindly moving library orders around without looking into the 
semantical meaning of these dependencies.



With this patch, both static and shared builds work for me:


Confirmed. Works for me too.

The only thing that I am a bit concerned about is, why is the HTTP ETag 
functionality now part of the core, and not part of the http module 
anymore? Isn't this somewhat adverse to other efforts to move more code 
from core to modules?


Regards,
Micha

Re: Experimental C unit test suite available for hacking

[Micha Lenk]

Hi Yann,

FWIW I've found a very good explanation of what's going on during 
linking and why the library order in static linking is so important.

https://eli.thegreenplace.net/2013/07/09/library-order-in-static-linking

On 05/24/2018 12:00 PM, Yann Ylavic wrote:

Looks like the right order to me, however it fails with shared modules
because "server/core.c" (in libmain) uses ap_set_etag() function from
"modules/http/http_etag.c" (in BUILTIN_LIBS's libmod_http).


This could indicate that the default_handler() function which calls 
ap_set_etag() should better be moved to some file in modules/http/. Of 
course that opens a totally different can of worms...



I think "core" shouldn't depend on a module (even builtin), for
instance ap_set_{last_modified,accept_range,content_length,...} also
used by the core are defined in "server/protocol.c".

WDYT?


Well, first things first. Let's first fix trunk to be buildable again on 
build systems that really only link the needed symbols and thus rely on 
the correct library order during linking.


Regards,
Micha

Re: Experimental C unit test suite available for hacking

[Micha Lenk]

Hi Yann,

On 05/24/2018 10:41 AM, Yann Ylavic wrote:

--- Makefile.in (revision 1832123)
+++ Makefile.in (working copy)
@@ -7,9 +7,9 @@ PROGRAM_SOURCES  = modules.c
  PROGRAM_LDADD= buildmark.o $(HTTPD_LDFLAGS) $(PROGRAM_DEPENDENCIES) 
$(HTTPD_LIBS) $(EXTRA_LIBS) $(AP_LIBS) $(LIBS)
  PROGRAM_PRELINK  = $(COMPILE) -c $(top_srcdir)/server/buildmark.c
  PROGRAM_DEPENDENCIES = \
+  $(MPM_LIB) \
server/libmain.la \
$(BUILTIN_LIBS) \
-  $(MPM_LIB) \
os/$(OS_DIR)/libos.la
  
  sbin_PROGRAMS   = $(PROGRAM_NAME)


I've just tested several different library orders, but none of them work 
for me (even without the problematic modules I mentioned in the other 
mail). The library order as you are suggesting above is the only one 
that works for me.


I'll look into the modules issue later.

Regards,
Micha

Re: Experimental C unit test suite available for hacking

[Micha Lenk]

Hi Yann,

On 05/24/2018 11:23 AM, Yann Ylavic wrote:

Yes, for me too, except that the linker problem with undefined symbols now
seems to shift to the modules. I had to disable a few modules
(--enable-mods-static=most --disable-apreq --disable-proxy-fcgi
--disable-session-cookie --disable-session-dbd --disable-dav) to finally get
to a successful build.


Oh indeed, the correct order seems to be:


I'd rather expect some fixes in the affected modules linker order.

Regards,
Micha

Re: Experimental C unit test suite available for hacking

[Micha Lenk]

Hi Yann,

On 05/24/2018 10:41 AM, Yann Ylavic wrote:

./configure --prefix=/home/mlenk/Upstream/Apache/target
--with-apr=/home/mlenk/Upstream/Apache/target
--with-apr-util=/home/mlenk/Upstream/Apache/target --with-mpm=worker
--with-mpms-shared=all --enable-mods-static=most --enable-load-all-modules


This should be --enable-mpms-shared=all (not --with-...).


Drat. What a good catch! I should have enjoyed a coffee before trying 
the suggested work around.



Btw, as Jacob noted, the attached patch seems to work for me (even
without the above option).


Yes, for me too, except that the linker problem with undefined symbols 
now seems to shift to the modules. I had to disable a few modules 
(--enable-mods-static=most --disable-apreq --disable-proxy-fcgi 
--disable-session-cookie --disable-session-dbd --disable-dav) to finally 
get to a successful build.


Thanks for your help.

Regards,
Micha

Re: Experimental C unit test suite available for hacking

[Micha Lenk]

On 05/23/2018 10:21 PM, Christophe Jaillet wrote:
I can reproduce the issue if I don't pass any --enable-mpms-shared 
paramater to ./configure.

Having --with-mpm=xx only also triggers the building issue.

What is your ./configure command line?


The initial ./configure command line was:

./configure --prefix=/home/build/target --with-apr=/home/build/target 
--with-apr-util=/home/build/target --enable-modules=most


Can you try to add --enable-mpms-shared=event (or =all) and re-configure 
and build?


I tried several combinations of --with-mpm=worker and 
--with-mpms-shared=all, but none of them worked (all attempts with "make 
clean"). The most recent attempt used the following ./configure command 
line:


./configure --prefix=/home/mlenk/Upstream/Apache/target 
--with-apr=/home/mlenk/Upstream/Apache/target 
--with-apr-util=/home/mlenk/Upstream/Apache/target --with-mpm=worker 
--with-mpms-shared=all --enable-mods-static=most --enable-load-all-modules


Regards,
Micha

Re: Experimental C unit test suite available for hacking

[Micha Lenk]

Am 23.05.2018 um 20:18 schrieb Marion et Christophe JAILLET:
> Could you please try to 'make clean' and 'make' to see if you still have
> the build issue?

No change. :(

Regards,
Micha

Re: Experimental C unit test suite available for hacking

[Micha Lenk]

Hi Joe,

On 05/23/2018 04:21 PM, Joe Orton wrote:

On Wed, May 23, 2018 at 04:14:39PM +0200, Micha Lenk wrote:

Hi Eric,

On 05/23/2018 02:59 PM, Eric Covener wrote:

I guess the CI setup needs to be updated to at least build the unit tests?


I did not configure the build explicitly to run the unit tests, so it is
just the plain "make" that causes this target to get build. I would expect
the CI setup to implicitly build it as well. Yes, no?!

Does the target 'test/httpdunit' not get build in your local builds?


It should only get built if you configure --with-test-suite=... and
specify the path to a perl-framework wc.  It builds for me in trunk.


Hmm, no difference. It seems to get built independent from whether you 
specify --with-test-suite or not. As a cross check I've just added 
"--with-test-suite=../httpd-test" to the configure flags, but I still 
get the same build failure.


Regards,
Micha

Re: Experimental C unit test suite available for hacking

[Micha Lenk]

Hi Eric,

On 05/23/2018 02:59 PM, Eric Covener wrote:

I guess the CI setup needs to be updated to at least build the unit tests?


I did not configure the build explicitly to run the unit tests, so it is 
just the plain "make" that causes this target to get build. I would 
expect the CI setup to implicitly build it as well. Yes, no?!


Does the target 'test/httpdunit' not get build in your local builds?

Cheers,
Micha Lenk

Re: [VOTE] Allow for defect fix releases at httpd

[Micha Lenk]

On Wed, May 02, 2018 at 02:56:03PM -0400, Jim Jagielski wrote:
> On May 2, 2018, at 10:45 AM, Micha Lenk <mi...@lenk.info> wrote:
> > On 05/01/2018 04:33 PM, Graham Leggett wrote:
> >> What has been missing is input from the major distributors of our
> >> software (Fedora, Ubuntu, Redhat, Debian, Apple, Windows, Linux from
> >> Scratch, etc), who I believe are probably going “httpd is a mature
> >> project, we have nothing to worry about”. I would recommend against
> >> making changes to our approach without soliciting the views of these
> >> people and making sure they’re all catered for.
> > Why would you make a proposed change dependent on the (almost
> > necessarily contradicting) views of external entities? Is the
> > feedback from the major distributors through existing channels
> > really so bad that the httpd project can't get to an opinion of what
> > it would like to accomplish on its own? What exactly are you afraid
> > of?
> 
> Due to the modular aspect of httpd, we are lucky to have an extremely large,
> vibrant and diverse eco-system of module authors. Some are companies
> that provide functionality via binary modules, others are single-author
> GitHub authors.
> 
> A change on versioning and what versioning means and guarantees
> related to versioning affects this extremely large community. There is
> also the ISV and commercial *providers* of httpd to be considered as
> well, and how these changes would affect them.
> 
> With all that in mind, you can't just "willy-nilly" decide to change
> things without knowledge of how such changes will affect the eco-
> system as well as without a really solid rationale for said change.
> I don't consider "I can point out a handful of projects that do it
> different than httpd" as a solid rationale.

Yet I fail to see how this could be seen as an argument not to agree on
something like trying to provide bugfix only releases.  The vote was
about /what/ to accomplish, not even /how/ (i.e. versioning was not even
mentioned in the vote).  Am I missing something?

Regards,
Micha

Re: [VOTE] Allow for defect fix releases at httpd

[Micha Lenk]

Hi Graham,

On 05/01/2018 04:33 PM, Graham Leggett wrote:

What has been missing is input from the major distributors of our
software (Fedora, Ubuntu, Redhat, Debian, Apple, Windows, Linux from
Scratch, etc), who I believe are probably going “httpd is a mature
project, we have nothing to worry about”. I would recommend against
making changes to our approach without soliciting the views of these
people and making sure they’re all catered for.
Why would you make a proposed change dependent on the (almost 
necessarily contradicting) views of external entities? Is the feedback 
from the major distributors through existing channels really so bad that 
the httpd project can't get to an opinion of what it would like to 
accomplish on its own? What exactly are you afraid of?


Regards,
Micha

Re: Start using RCs

[Micha Lenk]

On 04/23/2018 07:24 PM, Paul Querna wrote:

https://svn.apache.org/repos/asf/httpd/httpd/trunk/VERSIONING

Contains a much more verbose... non-semver versioning scheme.


Interesting. Did you realize this already covers the recently suggested 
use of release candidates? Even on the 2.4.x branch...


Regards,
Micha

Re: Start using RCs

[Micha Lenk]

On 04/23/2018 06:33 PM, William A Rowe Jr wrote:

On Mon, Apr 23, 2018 at 11:12 AM, Micha Lenk <mi...@lenk.info> wrote:

On Fri, Apr 20, 2018 at 08:54:09AM -0400, Jim Jagielski wrote:

We have a history, as well as a published "agreement" on what minor
version numbering means.


Just to make sure I am on the same page, would you mind to make that
explicit? Where is that published?


http://httpd.apache.org/dev/release.html


The only paragraph on that page that contains the word "minor" is the 
second one in the Introduction section. It isn't very verbose either. Am 
I missing something obvious?


Regards,
Micha

Re: A proposal...

[Micha Lenk]

Just a side node, some days ago I just realized that the source package 
of the apache2 package in Debian seems to include the test suite for the 
purpose of running it as part of the continuous integration test 
'run-test-suite': https://ci.debian.net/packages/a/apache2/


In my recently provided bugfix (#62186) I included a change of the test 
suite, but so far it looks like it isn't integrated yet (do I really 
need to file a separate bugzilla in the other project for that?).


From the experience with doing so, I agree with others that in the long 
run maintaining some Perl-based test framework will probably make 
contributions pretty unpopular, especially for contributors that didn't 
work with Perl before.


For the addition of new regression tests (as others suggested) it would 
be pretty cool if they can be added in a somewhat more popular 
(scripting) language (Python and pytest were already mentioned). Yet the 
number of test frameworks to execute should stay at a manageable low number.


That being said, I am all for extending the use of any test framework.

Regards,
Micha

Re: Start using RCs

[Micha Lenk]

Hi Jim,

On Fri, Apr 20, 2018 at 08:54:09AM -0400, Jim Jagielski wrote:
> We have a history, as well as a published "agreement" on what minor
> version numbering means. 

Just to make sure I am on the same page, would you mind to make that
explicit? Where is that published?

> Our module eco-system is huge and we need to factor/consider not only
> the big players, but also the solitary developers who have modules. It
> is a long, long history and if/when we need to reconsider versioning,
> the impact will not be insignificant.
>
> [...]
> Again, it is all in balance, and likely we've just not achieved that
> lately due to the extreme complexities of the eco-system around,
> internal, external and dependent upon us.

But isn't the very purpose of the major.minor construct (that renders
all modules incompatible on a minor version bump) to get a handle on the
"extreme complexity" of the eco-system? If so, is the major.minor
construct used correctly to get the most out of it?

Regards,
Micha

Re: Revisit Versioning? (Was: 2.4.3x regression w/SSL vhost configs)

[Micha Lenk]

On Fri, Apr 20, 2018 at 08:14:16AM -0400, Jim Jagielski wrote:
> On Apr 20, 2018, at 8:04 AM, Micha Lenk <mi...@lenk.info> wrote:
> > [...], I value the ability to distinguish between bugfix-only
> > releases and feature addition releases.
> 
> I understand that, thx. I also understand how a minor bump makes that
> easier. It would make, however, other people's lives and jobs *more*
> difficult, I think, so it's all about balance. I can see how
> distinguishing the difference is also nice, but what "value" do you
> derive from it? I am genuinely curious. Thx!

To be honest, our commercial interest in bugfixes is simply higher than
getting new features. So, I expect integrating a bugfix-only release to
be much less effort (in terms of porting our own modules, patches,
additional testing scrutiny) than a release that re-works internal core
functionality like the request handling for the sake of adding a new
feature like the entirely new support for h2.

But I am equally genuinely curious what would make "other people's lives
and jobs *more* difficult". What exactly do you refer to here?

> This is a "hack", of course, but what if CHANGES specifically called
> out new features like we do w/ SECURITY items?

Not being a native speaker I am not sure I understand your question
correctly. Can you please elaborate that question a bit?

Regards,
Micha

Re: Start using RCs (Was: Re: So... when should we do 2.4.34? [WAS: Re: Revisit Versioning? (Was: 2.4.3x regression w/SSL vhost configs)])

[Micha Lenk]

Hi Jim,

On 04/20/2018 01:46 PM, Jim Jagielski wrote:

Where numbers and versioning DOES matter is how it affects
distributors and vendors of httpd and the entire module eco-system.
No, it doesn't. There are way too many variants of versioning schemes 
out there in use by so many OSS projects that the distributors and 
vendors need to be prepared to encounter any variant you can imagine.


What matters is quality (here I do agree with you). A versioning scheme 
can help to establish certain rules of what to do and more importantly 
what to *not* do on a given version pattern or branch. And with the 
current rate of successful releases, apparently the current rules either 
were not enforced well enough or otherwise were not good enough and thus 
need to be changed. A new versioning scheme then could help to establish 
new, hopefully better rules.


Just my 2¢.

Regards,
Micha

Re: Revisit Versioning? (Was: 2.4.3x regression w/SSL vhost configs)

[Micha Lenk]

Hi all,

On 04/20/2018 01:34 PM, Jim Jagielski wrote:

But why does it matter that h2 was added in 2.4.x instead of
a 2.6.0?


Because it sets a bad precedence (or even continues to do so)?


Every new feature must bump the minor? Even if
there is no corresponding ABI issue?


Why not?

In my role as Debian Developer maintaining the Debian packages of other 
OSS projects, and also in my role of maintaining a commercial reverse 
proxy product based on Apache httpd during my day job, I value the 
ability to distinguish between bugfix-only releases and feature addition 
releases.


This does not mean that a minor bump needs to happen at almost every 
release. But not bumping the minor for years (which seems to be the 
current pattern of the httpd project) is just worse, because it 
increases the incentive to squeeze features like h2 into releases that 
are meant (or perceived) as bugfix-only releases.


Regards,
Micha

Re: [PATCH 62186] POST request getting logged as GET request

[Micha Lenk]

This is a kind reminder that I still didn't get any response yet.

Is there any additional information needed from my side?


On 03/29/2018 09:09 PM, Micha Lenk wrote:

Hi Apache httpd committers,

I think I've found a bug which triggers under following conditions:

* Apache is configured to serve a local customized error page, e.g.
using something like "ErrorDocument 404 /var/www/errors/404.html"

* Apache is configured to log the original request's method, e.g.
using something like (please note, the "%<m" is the part that
matters):
CustomLog logs/mylog "%h %l %u %t \"%r\" %>s %b method=\"%<m\""

* HTTP request is a POST request (request body content doesn't matter)

* the request destination path results in a 404 Not Found error

If all these conditions are met, the request will get logged to
logs/mylog with a line ending with 'method="GET"', even though this was
a POST request.

For easier reproduction of this case, I've created a patch that extends
the Apache httpd test suite to cover this case. Please see the attached
file bz62186_httpd-test.patch.

An explanation of this behavior can be found in the code of ap_die_r(),
which explicitly sets r->method to "GET" and r->method_number to M_GET
before it is calling ap_internal_redirect(custom_response, r) to serve
the configured error document.

I've tried to fix this issue by taking a backup of the original
request's method and restoring it as soon as ap_internal_redirect()
returns (see attached patch bz62186_httpd_bugfix.patch). So far the
tests I've done are successful, i.e. the request is now correctly logged
as POST request.

I've filed this issue some days ago as
https://bz.apache.org/bugzilla/show_bug.cgi?id=62186 , but so far it
didn't get any comments yet. Could anybody please take a look?


Kind regards,
Micha

[PATCH 62186] POST request getting logged as GET request

[Micha Lenk]

Hi Apache httpd committers,

I think I've found a bug which triggers under following conditions:

* Apache is configured to serve a local customized error page, e.g.
   using something like "ErrorDocument 404 /var/www/errors/404.html"

* Apache is configured to log the original request's method, e.g.
   using something like (please note, the "%s %b method=\"%method to "GET" and r->method_number to M_GET
before it is calling ap_internal_redirect(custom_response, r) to serve
the configured error document.

I've tried to fix this issue by taking a backup of the original
request's method and restoring it as soon as ap_internal_redirect()
returns (see attached patch bz62186_httpd_bugfix.patch). So far the
tests I've done are successful, i.e. the request is now correctly logged
as POST request.

I've filed this issue some days ago as
https://bz.apache.org/bugzilla/show_bug.cgi?id=62186 , but so far it
didn't get any comments yet. Could anybody please take a look?


Kind regards,
Micha

--- t/apache/errordoc_method_logging.t	(nonexistent)
+++ t/apache/errordoc_method_logging.t	(working copy)
@@ -0,0 +1,34 @@ 
+use strict;
+use warnings FATAL => 'all';
+
+use Data::Dumper;
+use Apache::Test;
+use Apache::TestRequest;
+use Apache::TestUtil qw/t_cmp
+t_start_file_watch
+t_finish_file_watch/;
+
+Apache::TestRequest::module('error_document');
+
+plan tests => 3, need_lwp;
+
+{
+t_start_file_watch 'method_log';
+
+my $response = POST '/method_logging', content => 'does not matter';
+chomp(my $content = $response->content);
+
+ok t_cmp($response->code,
+ 404,
+ 'POST /method_logging, code');
+
+ok t_cmp($content,
+ 'Error 404 Test',
+ 'POST /method/logging, content');
+
+my @loglines = t_finish_file_watch 'method_log';
+chomp @loglines;
+ok t_cmp($loglines[0],
+ qr/"POST \/method_logging HTTP\/1.1" .* method="POST"/,
+ 'POST /method/logging, log');
+}
--- t/conf/extra.conf.in	(revision 1826815)
+++ t/conf/extra.conf.in	(working copy)
@@ -742,7 +742,11 @@ 
 ## 
 
 ErrorDocument 404 "per-server 404
- 
+
+CustomLog logs/method_log "%h %l %u %t \"%r\" %>s %b method=\"%s %b"
+
+
 
 ErrorDocument 404 "per-dir 404
 
@@ -760,6 +764,10 @@ 
 ErrorDocument 404 default
 
 
+
+ErrorDocument 404 /apache/errordoc/404.html
+
+
 
  ErrorDocument 404 "testing merge
 
--- t/htdocs/apache/errordoc/404.html	(nonexistent)
+++ t/htdocs/apache/errordoc/404.html	(working copy)
@@ -0,0 +1, @@ 
+Error 404 Test

--- modules/http/http_request.c	(revision 1826989)
+++ modules/http/http_request.c	(working copy)
@@ -187,7 +187,8 @@ 
 apr_table_setn(r->headers_out, "Location", custom_response);
 }
 else if (custom_response[0] == '/') {
-const char *error_notes;
+const char *error_notes, *original_method;
+int original_method_number;
 r->no_local_copy = 1;   /* Do NOT send HTTP_NOT_MODIFIED for
  * error documents! */
 /*
@@ -205,9 +206,13 @@ 
  "error-notes")) != NULL) {
 apr_table_setn(r->subprocess_env, "ERROR_NOTES", error_notes);
 }
+original_method = r->method;
+original_method_number = r->method_number;
 r->method = "GET";
 r->method_number = M_GET;
 ap_internal_redirect(custom_response, r);
+r->method = original_method;
+r->method_number = original_method_number;
 return;
 }
 else {

Re: AddOutputFilterByType in Apache 2.4 inserts filters as AP_FTYPE_RESOURCE

[Micha Lenk]

Hi Nick,

if the patch looks good, as you wrote, what is needed to get it applied 
to trunk and backported to 2.4.x?


Have you seen my follow-up questions in the other mail?

Best regards,
Micha


Am 13.01.2016 22:44, schrieb Nick Kew:

On Wed, 2016-01-13 at 17:59 +0100, Micha Lenk wrote:

Hi,

The directive AddOutputFilterByType can be used to insert filters to 
the
output filter chain depending on the content type of the HTTP 
response.

So far so good.

PROBLEM DESCRIPTION


This is probably worth a bugzilla entry.

I think I can clarify a little of what's happened.
AddOutputFilterByType was something of a hacked afterthought
to filtering back in the days of httpd 2.0.  On the one hand,
it met a need.  On the other hand, it worked only in a very
limited range of situations where the content type was known
at the time the filter was to be added.  It had no capacity
to respond to other aspects of the content, or indeed the
request/response.  And there were other issues.

Then came mod_filter and a generalised framework.
AddOutputFilterByType was now obsolete, but too widely-used
to dispense with entirely.  As a compromise it was re-implemented
within mod_filter, where it could co-exist with other dynamic
filter configuration.

Your observation tells us the semantics aren't quite compatible.
And your patch looks good - thanks.

Re: AddOutputFilterByType in Apache 2.4 inserts filters as AP_FTYPE_RESOURCE

[Micha Lenk]

Hi Nick,

Am 13.01.2016 22:44, schrieb Nick Kew:

PROBLEM DESCRIPTION


This is probably worth a bugzilla entry.


Done. https://bz.apache.org/bugzilla/show_bug.cgi?id=58856

Nick, would you mind to provide some insights on these comments from my 
initial mail:

For setups with both, FilterDeclare and AddOutputFilterByType (as
described above as fix), I observed some issues with properly merging
the two filter harnesses. However, I have no clue what semantics the
original author wanted to have in this situation.


Assumed that my patch gets applied, the filter type should be correctly 
set in the filter harness. But what if a user wants to override it? I 
got a few questions in this context:


1.) Should "FilterDeclare" with filter-name "BYTYPE:DEFLATE" (i.e. 
colliding

with the implicit filter-name created by AddOutputFilterByName) be
supported at all?

1a.) If yes, the handling of AddOutputFilterByType needs to be fixed so 
that:

 - a globally configured FilterDeclare is also effective for an
   AddOutputFilterByType within a (location) sub-container.
 - a filter type set by "FilterDeclare BYTYPE: type>"

   does not get overwritten by a subsequent
   "AddOutputFilterByType ".

1b.) If no, the code should detect and reject such configurations.

2.) On a related note to 1., Should FilterDeclare allow a filter-name of
existing filter providers at all? If yes, behavior would we expect?

Nick, I would be really glad if you could share your thoughts.

Best regards,
Micha

AddOutputFilterByType in Apache 2.4 inserts filters as AP_FTYPE_RESOURCE

[Micha Lenk]

Hi,

The directive AddOutputFilterByType can be used to insert filters to the 
output filter chain depending on the content type of the HTTP response. 
So far so good.


PROBLEM DESCRIPTION

I observed that the behavior of this directive changed in Apache 2.4 for 
filters. Starting with Apache 2.4 filters are inserted at an earlier 
place in the filter chain than they were inserted with Apache 2.2. For 
example, if you use the directive


AddOutputFilterByType DEFLATE text/html

The filter is inserted with AP_FTYPE_RESOURCE, even though it was 
registered in mod_deflate.c as AP_FTYPE_SET_CONTENT.
The effect is that using "AddOutputFilterByType DEFLATE text/html" the 
resulting filter chain is ordered diferrently compared to using 
"SetOutputFilter DEFLATE".


This can be fixed in configuration by adding the following directive 
right after AddOutputFilterByType:


FilterDeclare BYTYPE:DEFLATE CONTENT_SET

Unfortunately the order and placement of FilterDeclare seems to be 
relevant, i.e. this fix only works if FilterDeclare comes *after* 
AddOutputFilterByType within the same container.


I wonder whether this is an intentional behavior change of 
AddOutputFilterByType or not.


ANALYSIS

Apparently the filter type (ap_filter_rec_t struct member ftype) of the 
filter provider isn't respected anymore.


The intended filter type is provided when registering the output filter 
by calling ap_register_output_filter(). In branch 2.2.x this was 
sufficient, because the conditional filter (based on the MIME type) was 
added directly by name (i.e. by calling ap_add_output_filter() in 
ap_add_output_filters_by_type). In branches 2.4.x and trunk the 
implementation of AddOutputFilterByType apparently moved to mod_filter 
and a layer of indirection (the filter harness) is introduced. But the 
filter harness is apparently created unconditionally with 
AP_FTYPE_RESOURCE.


FIX APPROACH

When implicitly creating a filter harness by calling the function 
add_filter(), we have access to the provider's ap_filter_rec_t anyways. 
So I recommend to just copy it from the filter provider (e.g. DEFLATE) 
to the filter harness (e.g. BYTYPE:DEFLATE).


I've tested this approach with the patch below for a setup without any 
FilterDeclare directive, and it fixed the regression; the DEFLATE filter 
was ordered correctly at AP_FTYPE_CONTENT_SET again.


- 8>< 
--

diff --git a/modules/filters/mod_filter.c b/modules/filters/mod_filter.c
index 7b69223..5b5ecf6 100644
--- a/modules/filters/mod_filter.c
+++ b/modules/filters/mod_filter.c
@@ -444,6 +444,12 @@ static const char *add_filter(cmd_parms *cmd, void 
*CFG,

 ap_expr_info_t *node;
 const char *err = NULL;

+/* if provider has been registered, we can look it up */
+provider_frec = ap_get_output_filter_handle(pname);
+if (!provider_frec) {
+return apr_psprintf(cmd->pool, "Unknown filter provider %s", 
pname);

+}
+
 /* fname has been declared with DeclareFilter, so we can look it up 
*/

 frec = apr_hash_get(cfg->live_filters, fname, APR_HASH_KEY_STRING);

@@ -454,17 +460,13 @@ static const char *add_filter(cmd_parms *cmd, void 
*CFG,

 return c;
 }
 frec = apr_hash_get(cfg->live_filters, fname, 
APR_HASH_KEY_STRING);

+frec->ftype = provider_frec->ftype;
 }

 if (!frec) {
 return apr_psprintf(cmd->pool, "Undeclared smart filter %s", 
fname);

 }

-/* if provider has been registered, we can look it up */
-provider_frec = ap_get_output_filter_handle(pname);
-if (!provider_frec) {
-return apr_psprintf(cmd->pool, "Unknown filter provider %s", 
pname);

-}
 provider = apr_palloc(cmd->pool, sizeof(ap_filter_provider_t));
 if (expr) {
 node = ap_expr_parse_cmd(cmd, expr, 0, , NULL);
- 8>< 
--


For setups with both, FilterDeclare and AddOutputFilterByType (as 
described above as fix), I observed some issues with properly merging 
the two filter harnesses. However, I have no clue what semantics the 
original author wanted to have in this situation.


I hope my explanations are clear enough for others to follows. If not, 
please don't hesitate to ask.


Best regards,
Micha

Re: Segfault on graceful reload with OCSP stapling enabled?

[Micha Lenk]

Hi all,

Am 18.12.2015 12:35, schrieb Micha Lenk:

I am currently observing a httpd segfault that is triggered on my
system by every second graceful reload (i.e. SIGUSR1).

Unfotunately I won't be able to trace this down before Monday, so this
is merely a heads-up for those interested.
Is anybody able to reproduce this behavior?


Just for the records, the segfault seems to be caused by a bad patch.
So sorry for the noise...

Best regards,
Micha

Segfault on graceful reload with OCSP stapling enabled?

[Micha Lenk]

Hi all,

I am currently observing a httpd segfault that is triggered on my system 
by every second graceful reload (i.e. SIGUSR1).


Unfotunately I won't be able to trace this down before Monday, so this 
is merely a heads-up for those interested.

Is anybody able to reproduce this behavior?

This is my configuration:

SSLStaplingCache shmcb:cache/ocsp_cache(128000)
SSLUseStapling on
SSLStaplingResponderTimeout 5
SSLStaplingReturnResponderErrors off

Thanks in advance for any helpful comments...

Cheerds,
Micha

BTW: The backtrace looks like this (however, please note that this is a 
patched http 2.4.16, so line numbers might be off a bit):


(gdb) bt full
#0  0x7d50537b in ?? ()
No symbol table info available.
#1  0x0829af5c in find_entry (ht=0x88e7f68, key=0xcfc0, klen=20, 
val=0x0) at tables/apr_hash.c:294

hep = 
he = 
hash = 
#2  0x0829b04b in apr_hash_get (ht=0x88e7f68, key=0xcfc0, klen=20) 
at tables/apr_hash.c:367

he = 
#3  0x0818f059 in ssl_stapling_init_cert (s=0x8602c78, p=0x84cf0a8, 
ptemp=0x8520e98, mctx=0x89e87a8, x=0x8a29a58) at ssl_util_stapling.c:120

idx = "\226i\327w\347\200[=\272ς\333\364f\263\232PKKJ"
cinf = 
issuer = 
cid = 
aia = 
#4  0x0818145d in ssl_init_server_ctx (pphrases=, 
sc=, ptemp=, p=, 
s=) at ssl_engine_init.c:1457

No locals.
#5  ssl_init_ConfigureServer (s=0x8602c78, p=0x84cf0a8, ptemp=0x8520e98, 
sc=0x89e8660, pphrases=0x865a0f0) at ssl_engine_init.c:1499

rv = 0
#6  0x08181b3a in ssl_init_Module (p=0x84cf0a8, plog=0x850e2e0, 
ptemp=0x8520e98, base_server=0x8502fd0) at ssl_engine_init.c:314

mc = 
sc = 
s = 0x8602c78
rv = 0
pphrases = 0x865a0f0
#7  0x080d32c0 in ap_run_post_config (pconf=0x84cf0a8, plog=0x850e2e0, 
ptemp=0x8520e98, s=0x8502fd0) at config.c:103

n = 2
rv = 2102416251
#8  0x080b6fa5 in main (argc=139251872, argv=0x864ef88) at main.c:777
c = 68 'D'
showcompile = 0
showdirectives = 0
confname = 0xd42e "/var/lib/usp/hsp/hts/conf/httpd.conf"
def_server_root = 0xd416 "/var/lib/usp/hsp/hts"
temp_error_log = 0x0
error = 
process = 0x84cd130
pconf = 0x84cf0a8
plog = 0x850e2e0
ptemp = 0x8520e98
pcommands = 0x84f1130
opt = 0x84f11d0
rv = 
opt_arg = 0xd455 "SSL"

Re: How to wait on a global lock with timeout

[Micha Lenk]

Hi Yann,

thanks for your explanation.

Am 16.05.2015 um 19:05 schrieb Yann Ylavic:
 Trying to get locked mutex... timed out after 1 second (expected)
 Child trying to unlock the mutex got 1 (unexpected)
 Trying to get locked mutex... timed out after 2 seconds (unexpected)

 Did I do anything wrong?
 
 A mutex must be unlocked by the thread which locked it (error 1 is EPERM).

Ok, makes sense to me.

Regards,
Micha

Re: How to wait on a global lock with timeout

[Micha Lenk]

Hi Yann,

Am 25.03.2015 um 09:56 schrieb Yann Ylavic:
 To mitigate that design flaw I would provide the timeout by reference
 and update it by the functions using it. This has also the nice benefit
 that the caller is able to retrieve the time it needed to wait.
 
 By doing so, you have to get the current time (call apr_time_now())
 each time, and I wanted to avoid it when the native functions don't
 need it.
 The remaining time is not important if you can pass an absolute time (IMHO).

Ok, understood. You're right.

Sorry, took me a while to give your patch a try. As the patch didn't
apply cleanly to trunk (ie. SVN rev. 1676013), I assume that it is
already applied in trunk. So I went ahead without applying your patch.
Is this correct?

I wrote a small test program that should prove me that timed global
locks work across process boundaries (see attached source). But for some
reason apr_global_mutex_unlock() return 1 if called in the child
process. This is the output that I get from the compiled test program:

Trying to get locked mutex... timed out after 1 second (expected)
Child trying to unlock the mutex got 1 (unexpected)
Trying to get locked mutex... timed out after 2 seconds (unexpected)

Did I do anything wrong?


Best regards,
Micha

#include assert.h
#include stdio.h
#include unistd.h
#include apr.h
#include apr_pools.h
#include apr_global_mutex.h


#define LOCKFILE /tmp/apr-synctest-lockfile


int main(int argc, const char *argv[]) {
apr_pool_t *pool = NULL;
apr_global_mutex_t *mutex = NULL;
apr_time_t timeout;
apr_status_t rv;
pid_t child;
int child_rc;
int rc = 0;

assert(apr_initialize() == APR_SUCCESS);
assert(apr_pool_create(pool, NULL) == APR_SUCCESS);

apr_global_mutex_create(mutex, LOCKFILE, APR_LOCK_DEFAULT_TIMED, pool);
/*
 * LOCK
 */
apr_global_mutex_lock(mutex);
timeout = apr_time_from_sec(1);
printf(Trying to get locked mutex...);
rv = apr_global_mutex_timedlock(mutex, timeout, 0);
if (rv == APR_TIMEUP) {
printf( timed out after 1 second (expected)\n);
} else {
printf( FAILED: %d (unexpected)\n, rv);
rc = 1;
	goto cleanup;
}

child = fork();
if (child == 0) { // child process
assert(apr_global_mutex_child_init(mutex, LOCKFILE, pool) == APR_SUCCESS);
	(void)sleep(1);
/*
 * UNLOCK in forked child
 */
	rv = apr_global_mutex_unlock(mutex);
if (rv != APR_SUCCESS) {
rc = 1;
	printf(Child trying to unlock the mutex got %d (unexpected)\n, rv);
}
} else if (child  0) { // parent process
timeout = apr_time_from_sec(2);
	printf(Trying to get locked mutex...);
rv = apr_global_mutex_timedlock(mutex, timeout, 0);
	if (rv == APR_SUCCESS) {
	printf( OK (expected)\n);
} else if (rv == APR_TIMEUP) {
printf( timed out after 2 seconds (unexpected)\n);
rc = 1;
	} else {
	printf( FAILED: %d (unexpected)\n, rv);
	rc = 1;
	}
	wait(child_rc);
	if (rc == 0)
	rc = child_rc;
} else { // fork() failed
printf(failed to fork(): %m);
	rc = 1;
	goto cleanup;
}


cleanup:
apr_pool_destroy(pool);
apr_terminate();
return rc;
}

Re: How to wait on a global lock with timeout

[Micha Lenk]

Hi Yann,

Am 20.03.2015 um 02:59 schrieb Yann Ylavic:
 a few times later, commited in [1] and [2] :)

Wow, thanks for coming back to that issue.

 Can you please check if it works for you?

I only had time to review the code changes. I looked at the
implementation of apr_global_mutex_timedlock(). There I noticed that, if
APR_HAS_THREADS, you are doing 3 timed operations without checking the
remaining time. If all operations take almost the given timeout, this
can in the end result in almost 3 times the allowed value.

To mitigate that design flaw I would provide the timeout by reference
and update it by the functions using it. This has also the nice benefit
that the caller is able to retrieve the time it needed to wait.

I hope to have some time to check out the new functions in my code soon.

 I also attached the patch against 1.6.x, if that can help...
 It should also apply to 1.5.x, though it will never be backported
 there (due to API changes).

Backports are not needed. Thanks for offering.

Regards,
Micha

Re: Differentiating incoming/outgoing connections from pre_connection hook

[Micha Lenk]

Hi all again,

Am 30.01.2015 14:28, schrieb Micha Lenk:

Now, how do I avoid executing the code hooked into the pre_connection
hook more than once?


I just realized that I can check the scoreboard handle (.sbh) in 
conn_rec to achive that. Proxy connections are not associated to a 
scoreboard, so if .sbh is NULL, this means the pre_connection hook was 
called for a backend connection.


I will prefer this approach as it doesn't involve patching httpd.

Anyways, if you want I am still interested in any comments.

Best regards,
Micha

Differentiating incoming/outgoing connections from pre_connection hook

[Micha Lenk]

Hi all,

I am currently investigating why and how to cope with the pre_connection 
hook is called twice for a single request. As I understand it, the 
reason is that the module in question handles also reverse proxy 
requests. And ap_run_pre_connection() is not only called by the core in 
ap_process_connection(), but also by several proxy related modules, e.g. 
from within ap_proxy_connection_create().


Now, how do I avoid executing the code hooked into the pre_connection 
hook more than once?


For most other hooks the request_rec is available which has the needed 
information in the proxyreq member. But in the pre_connection hook it 
isn't.


To give module authors a means to differentiate between the incoming 
connection that lead to creating a request and outgoing connection that 
are created because a request is being processed, I want to suggest to 
add a struct member 'direction' to the conn_rec to keep track of the 
direction of the connection. When zero-ing the struct (e.g. by using 
pcalloc), .direction is initialized to AP_CONN_INCOMING (= 0). Whenever 
a module creates another connection and intends to call 
ap_run_pre_connection on it, this module needs to make sure to set 
.direction to AP_CONN_OUTGOING. This way the code called from the 
pre_connection hook can decide whether to run on the provided conn_rec 
or not.


The patch below is a draft (based on some patched httpd 2.4.10 version) 
for how this could be implemented. It definitely needs an additional 
check for other places where ap_run_pre_connection() is called. But I 
want to provide it to get early feedback on my idea. So, what do you 
think?


Best regards,
Micha



diff --git a/include/httpd.h b/include/httpd.h
index e1510be..00dd5c4 100644
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -1063,6 +1063,11 @@ typedef enum {
 AP_CONN_KEEPALIVE
 } ap_conn_keepalive_e;

+typedef enum {
+AP_CONN_INCOMING = 0,
+AP_CONN_OUTGOING
+} ap_conn_direction_e;
+
 /**
  * @brief Structure to store things which are per connection
  */
@@ -1151,6 +1156,8 @@ struct conn_rec {
  */
 const char *log_id;

+ap_conn_direction_e direction;
+

 /** This points to the current thread being used to process this 
request,
  * over the lifetime of a request, the value may change. Users of 
the connection

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index bc84049..26e731c 100644
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -2853,6 +2853,9 @@ PROXY_DECLARE(int) 
ap_proxy_connection_create(const char *proxy_function,

 return HTTP_INTERNAL_SERVER_ERROR;
 }

+/* Proxy connections are by definition outgoing connections */
+conn-connection-direction = AP_CONN_OUTGOING;
+
 /* For ssl connection to backend */
 if (conn-is_ssl) {
 if (!ap_proxy_ssl_enable(conn-connection)) {

Re: Time for 2.4.11

[Micha Lenk]

Hi Jeff,

Am 10.01.2015 18:30, schrieb Jeff Trawick:

On Fri, Jan 9, 2015 at 4:35 PM, Micha Lenk mi...@lenk.info wrote:


Hi,

Am 08.01.2015 um 12:11 schrieb Jim Jagielski:

Let's shoot for a TR next week. The work will keep me
warm :)


Can we please get another vote on this?

* core: Fix -D[efined] or Define[d] variables lifetime
accross restarts. PR 57328.
trunk patch: http://svn.apache.org/r1643825 [1]
2.4.x patch: trunk works (module CHANGES)
+1: ylavic, rjung

Thanks for considering,

Micha


now approved for 2.4.11...


Thanks for the update.

I was a bit confused by the commit message of SVN rev. 1651084 not 
matching at all the committed changes, until I realized that the changes 
I want have been committed in SVN rev. 1651083 (using the same commit 
message). Weird...


Best regards,
Micha

Re: Reverse proxy: invalid Content-Length leads to 413 + 400 errors mixed up

[Micha Lenk]

Sorry, this shouldn't have gone to dev@httpd.apache.org.

Re: Reverse proxy: invalid Content-Length leads to 413 + 400 errors mixed up

[Micha Lenk]

Hi Ewald,

zunächst mal ein gutes neues Jahr!

Am 08.01.2015 um 17:29 schrieb Ewald Dieterich:
 On 01/08/2015 04:15 PM, Yann Ylavic wrote:
 Can you test this (attached) patch please (without yours applied)?

 Or with yours and just changing return
 ap_map_http_request_error(status, HTTP_BAD_REQUEST); by return
 (status == AP_FILTER_ERROR) ? DONE : ap_map_http_request_error(status,
 HTTP_BAD_REQUEST);
 
 Looks good. I tested both your patch and my modified one and now error
 response and access log entry are both OK.

Was meinst Du? Wären diese Änderungen auch für's mod_proxy_msrpc hilfreich?

Viele Grüße aus Zürich,
Micha

Re: Time for 2.4.11

[Micha Lenk]

Hi,

Am 08.01.2015 um 12:11 schrieb Jim Jagielski:
 Let's shoot for a TR next week. The work will keep me
 warm :)

Can we please get another vote on this?

 * core: Fix -D[efined] or Define[d] variables lifetime
 accross restarts. PR 57328.
   trunk patch: http://svn.apache.org/r1643825
   2.4.x patch: trunk works (module CHANGES)
   +1: ylavic, rjung

Thanks for considering,

Micha

Re: How to wait on a global lock with timeout

[Micha Lenk]

Hi Yann,

On 30.09.2014 18:16, Yann Ylavic wrote:

On Tue, Sep 30, 2014 at 5:30 PM, Yann Ylavic ylavic@gmail.com wrote:

I have been working on a patch to provide
apr_[thread/proc]_mutex_timedlock() in APR, [...]


I think that is exactly what I was looking for...

Your idea to use the native mutex functions is probably better than 
nothing. It provides the needed functionality at least for some platforms.



I think I'll give my patch a new chance, at least for proc-mutexes
(and hence global-mutexes), but this is not a today's solution in any
case since I would be at best an APR-1.6 feature (if accepted)...


That would be great. I am looking forward to test that, as soon as 
available... ;)


Regards,
Micha

Re: mod_proxy's aside connections proposal

[Micha Lenk]

Hi,

On 30.09.2014 16:47, Yann Ylavic wrote:

Do you think this can/should (not) be applied to httpd?


I would love to see this applied to httpd.

Regards,
Micha

How to wait on a global lock with timeout

[Micha Lenk]

Hi,

in an Apache module I am in the need to wait for a global lock (e.g. an 
apr_global_mutex_t), but in theory the lock might not get released by 
the other process in a timely manner, so I would like to limit the time 
to wait for the lock. What do you suggest me to do?


Any suggestions are highly appreciated.

Best regards,
Micha

Re: [PATCH ASF bugzilla #56288] mod_proxy_html could rewrite URLs in HTML style attributes too

[Micha Lenk]

Hi,

the patch was missing in my previous mail. See attachment.

Regards,
Micha


On 19.03.2014 22:26, Micha Lenk wrote:

Hi Apache developers,

In HTML you can have div tags that have a background image by
providing a style attribute. E.g. this can be done by something fancy
like this:

div style=background:url(http://www.example.com/fancy-background.png)
right 0px no-repeat; height:325px;

Currently mod_proxy_html doesn't rewrite the URL in such style
attributes (Thomas, Ewald, this is issue #22583 in our Mantis).

The attached patch tries to fix that by applying the configured string
replacements also on style attributes.

Additionally it tries to make the code a bit more readable by renaming
the function dump_content() to apply_urlmap(). This is what it actually
does, especially after the call to AP_fwrite() has been moved outside of
this function.

Please note that this patch requires the patch from issue #56284 to be
applied first. Otherwise the last chunk will not apply. The attached
patch is based on httpd trunk, rev. 1579365 plus the patch for issue
#56284 applied.

Regards,
Micha


diff --git a/modules/filters/mod_proxy_html.c b/modules/filters/mod_proxy_html.c
index 04e4ab7..12fb008 100644
--- a/modules/filters/mod_proxy_html.c
+++ b/modules/filters/mod_proxy_html.c
@@ -201,7 +201,7 @@ static void pappend(saxctxt *ctx, const char *buf, const size_t len)
 ctx-offset += len;
 }
 
-static void dump_content(saxctxt *ctx)
+static void apply_urlmap(saxctxt *ctx)
 {
 urlmap *m;
 char *found;
@@ -281,7 +281,6 @@ static void dump_content(saxctxt *ctx)
 }
 }
 }
-AP_fwrite(ctx, ctx-buf, strlen(ctx-buf), 1);
 }
 static void pcdata(void *ctxt, const xmlChar *uchars, int length)
 {
@@ -338,8 +337,9 @@ static void pendElement(void *ctxt, const xmlChar *uname)
 /* nah.  Keeping the stack is too much overhead */
 
 if (ctx-offset  0) {
-dump_content(ctx);
-ctx-offset = 0;/* having dumped it, we can re-use the memory */
+apply_urlmap(ctx);
+AP_fwrite(ctx, ctx-buf, strlen(ctx-buf), 1);
+ctx-offset = 0;/* having written the buffer, we can re-use it */
 }
 if (!desc || !desc-empty) {
 ap_fprintf(ctx-f-next, ctx-bb, /%s, name);
@@ -628,6 +628,12 @@ static void pstartElement(void *ctxt, const xmlChar *uname,
anything that needs it in the value.
 */
 ap_fputstrs(ctx-f-next, ctx-bb,  , a[0], =\, NULL);
+if (!strcasecmp(a[0], style)) {
+/* apply URL map on attribute value */
+ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, ctx-f-r,
+  applying URLMap on style attribute value: '%s', ctx-buf);
+apply_urlmap(ctx);
+}
 pcharacters(ctx, (const xmlChar*)ctx-buf, strlen(ctx-buf));
 ap_fputc(ctx-f-next, ctx-bb, '');
 }
@@ -1075,7 +1081,9 @@ static int proxy_css_filter(ap_filter_t *f, apr_bucket_brigade *bb)
 ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, f-r, Processing chunk: [%s], buf_log);
 
 // Replace matching regexps
-dump_content(fctx);
+apply_urlmap(fctx);
+AP_fwrite(fctx, fctx-buf, strlen(fctx-buf), 1);
+fctx-offset = 0;/* having written the buffer, we can re-use it */
 }
 if (APR_BUCKET_IS_EOS(current_bucket)) {
 APR_BUCKET_REMOVE(current_bucket);

[PATCH] mod_proxy_html deletes wrong data from HTML when meta http-equiv tag specifies Content-Type behind other meta http-equiv tag

[Micha Lenk]

Hi Apache developers,

over the time I've accumulated some patches for mod_proxy_html which I 
would like to get reviewed, and get applied in SVN (best both, trunk and 
then backported to 2.4). This is something that I feel to owe the Apache 
httpd community.


So, lets first start with a bug that lets mod_proxy_html delete the 
wrong data from HTML code when a http-equiv meta tag specifies a 
Content-Type behind any other http-equiv meta tag (Thomas, Ewald, this 
is issue #21648 in our Mantis). For better understanding of the issue, 
please consider the following HTML file treated by mod_proxy_html:


html
 head
  meta http-equiv=X-Dummy-Header content=dummy value
  style type=text/cssdiv.ok { color: green; }   /style
  meta http-equiv=Content-Type content=text/html; charset=utf8  
 head
 body
  div class=okIf the metafix is not broken, this text should get 
rendered in green color./div

 /body
/html

Without the attached patch, mod_proxy_html will remove the style tag 
inside the head tag as soon as it parses the meta tag with the 
http-equiv=Content-Type attribute. With the attached patch applied, 
mod_proxy_html removes the meta tag with the http-equiv=Content-Type 
attribute instead. I guess this is what the code intended to do.


The attached patch is based on httpd trunk, rev. 1579365.

Please provide feedback whether I should file an issue in Apaches 
Bugzilla or whether this isn't needed in this case.


Regards,
Micha
Index: modules/filters/mod_proxy_html.c
===
--- modules/filters/mod_proxy_html.c	(Revision 1579365)
+++ modules/filters/mod_proxy_html.c	(Arbeitskopie)
@@ -688,8 +688,8 @@
 }
 else if (!strncasecmp(header, Content-Type, 12)) {
 ret = apr_palloc(r-pool, sizeof(meta));
-ret-start = pmatch[0].rm_so;
-ret-end = pmatch[0].rm_eo;
+ret-start = offs+pmatch[0].rm_so;
+ret-end = offs+pmatch[0].rm_eo;
 }
 if (header  content) {
 #ifndef GO_FASTER

[PATCH] mod_proxy_html sometimes adds random characters to HTML pages smaller than 4 bytes

[Micha Lenk]

Hi Apache developers,

next is a bug that causes mod_proxy_html to add some random characters 
(+html code) to HTML pages, if the document is smaller than 4 bytes. 
(Thomas, Ewald, this is issue #18378 in our Mantis). It looks like the 
output is from some kind of uninitialized memory. The added string 
sometimes matches part of a previously delivered request. Also, it looks 
like this only happens when doing multiple HTTP requests with the same 
browser and using HTTP Keep Alive.


The root cause is that the charset guessing with xml2enc needs to 
consume at least 4 bytes from the document to come to a conclusion. The 
consumed bytes are buffered so that they can later get prepended to the 
output again. But apparently it is assumed that there are always at 
least 4 bytes available, which in some cases is not the case. In these 
cases the buffer may contain some bytes left behind from the previous 
request on the same connection.


The attached patch fixes that issue by simply skipping documents smaller 
than 4 bytes. The rationale behind this is, that for HTML rewriting to 
do something useful, it needs to work on an absolute URL (i.e. including 
a schema). But as the schema http is already 4 bytes, there would be 
nothing to rewrite.


The patch is based on httpd trunk, rev. 1579365.

Please provide feedback whether I should file an issue in Apaches 
Bugzilla or whether this isn't needed in this case.


Regards,
Micha
Index: modules/filters/mod_proxy_html.c
===
--- modules/filters/mod_proxy_html.c	(Revision 1579365)
+++ modules/filters/mod_proxy_html.c	(Arbeitskopie)
@@ -885,6 +885,15 @@
 else if (apr_bucket_read(b, buf, bytes, APR_BLOCK_READ)
  == APR_SUCCESS) {
 if (ctxt-parser == NULL) {
+/* For documents smaller than four bytes, there is no reason to do
+ * HTML rewriting. The URL schema (i.e. 'http') needs four bytes alone.
+ * And the HTML parser needs at least four bytes to initialise correctly.
+ */
+if ((bytes  4)  APR_BUCKET_IS_EOS(APR_BUCKET_NEXT(b))) {
+ap_remove_output_filter(f) ;
+return ap_pass_brigade(f-next, bb) ;
+}
+
 const char *cenc;
 if (!xml2enc_charset ||
 (xml2enc_charset(f-r, enc, cenc) != APR_SUCCESS)) {

[PATCH] mod_proxy_html removes the !doctype tag and breaks XHTML documents

[Micha Lenk]

Hi Apache developers,

the next patch fixes the behavior of mod_proxy_html to remove any 
!doctype tags from the beginning of HTML and XHTML documents. (Thomas, 
Ewald, this is issue #19803 in our Mantis). This !doctype tag is 
needed by some browsers to correctly render XHTML documents.


Additionally it fixes the issue in XHTML documents that empty tags don't 
get terminated correctly unless ProxyHTMLDoctype XHTML is explicitly 
configured (Thomas, Ewald, this is issue #17643 in our Mantis).


The attached patch fixes the first issue by using the internalSubset 
hook of the libxml2 SAX parser to parse and output the !doctype tag. 
To address the second issue, the same hook is used to automatically 
detect whether the current document is an XHTML document or not. In case 
of an XHTML document the XHTML-style for closing empty tags is enabled 
for the current request. In my opinion the suggested patch makes the 
directive ProxyHTMLDoctype more or less obsolete. Of course it is kept 
for backwards compatibility.


The patch is based on httpd trunk, rev. 1579365.

Please provide feedback whether I should file an issue in Apaches 
Bugzilla or whether this isn't needed in this case.


Regards,
Micha
Index: modules/filters/mod_proxy_html.c
===
--- modules/filters/mod_proxy_html.c	(Revision 1579365)
+++ modules/filters/mod_proxy_html.c	(Arbeitskopie)
@@ -101,6 +101,7 @@
 typedef struct {
 ap_filter_t *f;
 proxy_html_conf *cfg;
+const char *etag;
 htmlParserCtxtPtr parser;
 apr_bucket_brigade *bb;
 char *buf;
@@ -280,6 +281,25 @@
 }
 AP_fwrite(ctx, ctx-buf, strlen(ctx-buf), 1);
 }
+
+static void pinternalSubset(void* ctxt, const xmlChar *name, const xmlChar *externalID, const xmlChar *sysID)
+{
+if (!ctxt) return;
+if (!name) return;
+saxctxt* ctx = (saxctxt*) ctxt;
+if (ctx-cfg-doctype  ctx-cfg-doctype[0]) return;
+ap_fprintf(ctx-f-next, ctx-bb, !DOCTYPE %s, (const char *)name);
+if (externalID) {
+if ((strcasecmp((const char*)name, html) == 0) 
+(strncasecmp((const char *)externalID, -//W3C//DTD XHTML , 18) == 0))
+ctx-etag = xhtml_etag;
+ap_fprintf(ctx-f-next, ctx-bb,  PUBLIC \%s\, (const char *)externalID);
+if (sysID)
+ap_fprintf(ctx-f-next, ctx-bb,  \%s\, (const char *)sysID);
+}
+ap_fprintf(ctx-f-next, ctx-bb, \n);
+}
+
 static void pcdata(void *ctxt, const xmlChar *uchars, int length)
 {
 const char *chars = (const char*) uchars;
@@ -632,7 +652,7 @@
 }
 ctx-offset = 0;
 if (desc  desc-empty)
-ap_fputs(ctx-f-next, ctx-bb, ctx-cfg-etag);
+ap_fputs(ctx-f-next, ctx-bb, ctx-etag);
 else
 ap_fputc(ctx-f-next, ctx-bb, '');
 
@@ -833,6 +853,7 @@
 fctx-bb = apr_brigade_create(f-r-pool,
   f-r-connection-bucket_alloc);
 fctx-cfg = cfg;
+fctx-etag = cfg-etag;
 apr_table_unset(f-r-headers_out, Content-Length);
 
 if (cfg-interp)
@@ -1236,6 +1257,7 @@
 sax.characters = pcharacters;
 sax.comment = pcomment;
 sax.cdataBlock = pcdata;
+sax.internalSubset = pinternalSubset;
 xml2enc_charset = APR_RETRIEVE_OPTIONAL_FN(xml2enc_charset);
 xml2enc_filter = APR_RETRIEVE_OPTIONAL_FN(xml2enc_filter);
 if (!xml2enc_charset) {

[PATCH ASF bugzilla #56284] Extend mod_proxy_html to rewrite URLs in CSS documents too

[Micha Lenk]

Hi Apache developers,

next is a feature that extends mod_proxy_html to rewrite URLs in CSS 
documents too. This is done by applying the configured regex and string 
replacements for the content of HTML tags also on whole CSS documents, 
i.e. documents with Content-Type: text/css.


The attached patch is based on httpd trunk, rev. 1579365. I've also 
filed this in the ASF bugzilla as issue #56284.


Regards,
Micha
Index: modules/filters/mod_proxy_html.c
===
--- modules/filters/mod_proxy_html.c	(Revision 1579365)
+++ modules/filters/mod_proxy_html.c	(Arbeitskopie)
@@ -65,6 +65,8 @@
 #define M_INTERPOLATE_TO0x100
 #define M_INTERPOLATE_FROM  0x200
 
+#define M_PROXY_CSS_MIN_BUFFER_SIZE 4096
+
 typedef struct {
 const char *val;
 } tattr;
@@ -103,6 +105,7 @@
 proxy_html_conf *cfg;
 htmlParserCtxtPtr parser;
 apr_bucket_brigade *bb;
+apr_bucket_brigade *bbsave;
 char *buf;
 size_t offset;
 size_t avail;
@@ -953,6 +956,137 @@
 return APR_SUCCESS;
 }
 
+static int proxy_css_filter(ap_filter_t *f, apr_bucket_brigade *bb)
+{
+apr_status_t rv;
+proxy_html_conf* cfg = ap_get_module_config(f-r-per_dir_config, proxy_html_module);
+saxctxt *fctx = f-ctx;
+if (fctx == NULL) {
+int skip_filter = 0;
+// Skip this filter if this is no proxy request
+if ( ! f-r-proxyreq ) {
+skip_filter = 1;
+// init filter only if content type is set
+} else if ( ! f-r-content_type ) {
+skip_filter = 1;
+// init filter only for MIME type text/css
+} else if ( strncmp(f-r-content_type, text/css, 8) != 0 ) {
+skip_filter = 1;
+}
+
+if (skip_filter) {
+ap_filter_t* fnext = f-next;
+ap_remove_output_filter(f);
+return ap_pass_brigade(fnext, bb);
+}
+
+// The CSS filter changes the output length
+apr_table_unset(f-r-headers_out, Content-Length) ;
+
+// Initialize CSS output filter
+fctx = f-ctx = apr_pcalloc(f-r-pool, sizeof(saxctxt));
+fctx-f = f;
+fctx-cfg = ap_get_module_config(f-r-per_dir_config, proxy_html_module);
+fctx-bb = apr_brigade_create(f-r-pool, f-c-bucket_alloc);
+fctx-bbsave = apr_brigade_create(f-r-pool, f-c-bucket_alloc);
+
+// Add rules for variable interpolation etc.
+if (cfg-interp) fixup_rules(fctx);
+else fctx-map = fctx-cfg-map;
+}
+
+int process_bb_out = 0;
+apr_bucket* current_bucket;
+apr_bucket* next_bucket = NULL;
+for (current_bucket = APR_BRIGADE_FIRST(bb);
+ current_bucket != APR_BRIGADE_SENTINEL(bb);
+ current_bucket = (next_bucket ? next_bucket : APR_BUCKET_NEXT(current_bucket))) {
+char* out_data = NULL;
+apr_size_t out_data_length = 0;
+next_bucket = NULL;
+if (APR_BUCKET_IS_EOS(current_bucket)) {
+process_bb_out = 1;
+} else if (APR_BUCKET_IS_METADATA(current_bucket)) {
+continue;
+} else {
+const char* in_data;
+apr_size_t in_data_length;
+rv = apr_bucket_read(current_bucket, in_data, in_data_length, APR_BLOCK_READ);
+if (rv != APR_SUCCESS) return rv;
+
+// skip empty data bucket
+if (in_data_length == 0) continue;
+
+// scan for newline characters
+apr_size_t newline_offset = 0;
+const char *le_r = memchr(in_data, '\r', in_data_length);
+const char *le_n = memchr(in_data, '\n', in_data_length);
+if (le_r) newline_offset = (le_r - in_data) + 1;
+if (le_n  le_r) newline_offset = (le_n - in_data) + 1;
+
+// Bucket without newline character: simply append to bbsave and process it later
+if (newline_offset == 0) {
+next_bucket = APR_BUCKET_NEXT(current_bucket);
+APR_BUCKET_REMOVE(current_bucket);
+apr_bucket_setaside(current_bucket, f-r-pool);
+APR_BRIGADE_INSERT_TAIL(fctx-bbsave, current_bucket);
+continue;
+}
+
+// If newline is in the middle of the bucket: split the bucket
+if (newline_offset  in_data_length) {
+apr_bucket_split(current_bucket, newline_offset);
+}
+
+// Append bucket (with data up to and including the newline character)
+// to fctx-bbsave and trigger its processing
+next_bucket = APR_BUCKET_NEXT(current_bucket);
+APR_BUCKET_REMOVE(current_bucket);
+APR_BRIGADE_INSERT_TAIL(fctx-bbsave, current_bucket);
+process_bb_out = 1;
+}
+
+if (process_bb_out == 0) continue;
+
+// Check buffer size requirements
+apr_off_t needed_buffer_length;
+apr_brigade_length(fctx-bbsave, 1, needed_buffer_length);
+if 

Re: [PATCH] mod_proxy_html removes the !doctype tag and breaks XHTML documents

[Micha Lenk]

Hi all,

just for the records: I've just filed this in the ASF bugzilla as
issue #56285.

Regards,
Micha

Re: [PATCH] mod_proxy_html deletes wrong data from HTML when meta http-equiv tag specifies Content-Type behind other meta http-equiv tag

[Micha Lenk]

Hi,

Just for the records, I've just filed this issue in ASF bugzilla as 
issue #56286.


Regards,
Micha


On 19.03.2014 20:40, Micha Lenk wrote:

Hi Apache developers,

over the time I've accumulated some patches for mod_proxy_html which I
would like to get reviewed, and get applied in SVN (best both, trunk and
then backported to 2.4). This is something that I feel to owe the Apache
httpd community.

So, lets first start with a bug that lets mod_proxy_html delete the
wrong data from HTML code when a http-equiv meta tag specifies a
Content-Type behind any other http-equiv meta tag (Thomas, Ewald, this
is issue #21648 in our Mantis). For better understanding of the issue,
please consider the following HTML file treated by mod_proxy_html:

html
  head
   meta http-equiv=X-Dummy-Header content=dummy value
   style type=text/cssdiv.ok { color: green; }   /style
   meta http-equiv=Content-Type content=text/html; charset=utf8  
  head
  body
   div class=okIf the metafix is not broken, this text should get
rendered in green color./div
  /body
/html

Without the attached patch, mod_proxy_html will remove the style tag
inside the head tag as soon as it parses the meta tag with the
http-equiv=Content-Type attribute. With the attached patch applied,
mod_proxy_html removes the meta tag with the http-equiv=Content-Type
attribute instead. I guess this is what the code intended to do.

The attached patch is based on httpd trunk, rev. 1579365.

Please provide feedback whether I should file an issue in Apaches
Bugzilla or whether this isn't needed in this case.

Regards,
Micha

Re: [PATCH] mod_proxy_html deletes wrong data from HTML when meta http-equiv tag specifies Content-Type behind other meta http-equiv tag

[Micha Lenk]

Hi again,

Err, #56287 that is.

Regards,
Micha

On 19.03.2014 22:05, Micha Lenk wrote:

Just for the records, I've just filed this issue in ASF bugzilla as
issue #56286.

Re: [PATCH] mod_proxy_html sometimes adds random characters to HTML pages smaller than 4 bytes

[Micha Lenk]

Hi,

On 19.03.2014 21:19, Jim Jagielski wrote:

It's always best, imo, to follow-up with a bugzilla entry with
description and patch.


Ok, this issue is now filed in ASF bugzilla as #56286.

Regards,
Micha

[PATCH ASF bugzilla #56288] mod_proxy_html could rewrite URLs in HTML style attributes too

[Micha Lenk]

Hi Apache developers,

In HTML you can have div tags that have a background image by 
providing a style attribute. E.g. this can be done by something fancy 
like this:


div style=background:url(http://www.example.com/fancy-background.png) 
right 0px no-repeat; height:325px;


Currently mod_proxy_html doesn't rewrite the URL in such style 
attributes (Thomas, Ewald, this is issue #22583 in our Mantis).


The attached patch tries to fix that by applying the configured string 
replacements also on style attributes.


Additionally it tries to make the code a bit more readable by renaming 
the function dump_content() to apply_urlmap(). This is what it actually 
does, especially after the call to AP_fwrite() has been moved outside of 
this function.


Please note that this patch requires the patch from issue #56284 to be 
applied first. Otherwise the last chunk will not apply. The attached 
patch is based on httpd trunk, rev. 1579365 plus the patch for issue 
#56284 applied.


Regards,
Micha

[PATCH ASF bugzilla #56289] Buffer overflow in mod_proxy_html's string replacement can cause a segfault

[Micha Lenk]

Hi developers,

The attached patch fixes a buffer overflow in at least one of the six 
string replacement implementations in mod_proxy_html.


Unfortunately I don't remember anymore how to reproduce the issue 
properly, but I know that some long time ago I fixed a segfault with 
this patch.


The patch tries to address the buffer overflow by introducing a new 
function preplace() dedicated to clean string replacement. This function 
is now used on all six places where the error-prone string replacement 
was previously implemented manually with memcpy() and memmove().


The patch is based on SVN trunk rev. 1579365.
I've filed it also in ASF bugzilla as issue #56289.

Regards,
Micha
Index: modules/filters/mod_proxy_html.c
===
--- modules/filters/mod_proxy_html.c	(Revision 1579365)
+++ modules/filters/mod_proxy_html.c	(Arbeitskopie)
@@ -29,6 +29,8 @@
 #define VERBOSEB(x) if (verbose) {x}
 #endif
 
+#include assert.h
+
 /* libxml2 */
 #include libxml/HTMLparser.h
 
@@ -191,6 +193,9 @@
 }
 }
 
+/**
+ * Appends the string buf with length len to ctx-buf at position ctx-offset
+ */
 static void pappend(saxctxt *ctx, const char *buf, const size_t len)
 {
 preserve(ctx, len);
@@ -198,6 +203,34 @@
 ctx-offset += len;
 }
 
+/**
+ * In ctx-buf replaces the substring starting at offset start with length old_length
+ * by the string new with length new_length
+ */
+static void preplace(saxctxt* ctx, const size_t start, const size_t old_length, const char* new, const size_t new_length) {
+  // check arguments
+  assert(ctx-offset  start + old_length);
+  assert(new != NULL);
+
+  // do the string replacement
+  if (old_length  new_length) {
+int old_buffer_length = ctx-offset;
+preserve(ctx, new_length - old_length);
+memmove(ctx-buf + start + new_length,
+ctx-buf + start + old_length,
+   ctx-offset - (start + old_length));
+ctx-offset += new_length - old_length;
+  }
+  memcpy(ctx-buf + start, new, new_length);
+  if (old_length  new_length) {
+memmove(ctx-buf + start + new_length,
+ctx-buf + start + old_length,
+   ctx-offset - (start + old_length));
+assert(ctx-offset = old_length - new_length);
+ctx-offset -= old_length - new_length;
+  }
+}
+
 static void dump_content(saxctxt *ctx)
 {
 urlmap *m;
@@ -236,17 +269,7 @@
 ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, ctx-f-r,
   C/RX: match at %s, substituting %s, f, subs);
 )
-if (s_to  s_from) {
-preserve(ctx, s_to - s_from);
-memmove(ctx-buf+offs+s_to, ctx-buf+offs+s_from,
-len + 1 - s_from - offs);
-memcpy(ctx-buf+offs, subs, s_to);
-}
-else {
-memcpy(ctx-buf + offs, subs, s_to);
-memmove(ctx-buf+offs+s_to, ctx-buf+offs+s_from,
-len + 1 - s_from - offs);
-}
+preplace(ctx, offs, s_from, subs, s_to);
 offs += s_to;
 }
 }
@@ -264,17 +287,7 @@
 VERBOSE(ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, ctx-f-r,
   C: matched %s, substituting %s,
   m-from.c, m-to));
-if (s_to  s_from) {
-preserve(ctx, s_to - s_from);
-memmove(ctx-buf+match+s_to, ctx-buf+match+s_from,
-len + 1 - s_from - match);
-memcpy(ctx-buf+match, m-to, s_to);
-}
-else {
-memcpy(ctx-buf+match, m-to, s_to);
-memmove(ctx-buf+match+s_to, ctx-buf+match+s_from,
-len + 1 - s_from - match);
-}
+preplace(ctx, match, s_from, m-to, s_to);
 }
 }
 }
@@ -481,19 +494,7 @@
 })
 s_to = strlen(subs);
 len = strlen(ctx-buf);
-if (s_to  s_from) {
-preserve(ctx, s_to - s_from);
-memmove(ctx-buf+offs+s_to,
-ctx-buf+offs+s_from,
-len + 1 - s_from - offs);
-memcpy(ctx-buf+offs, subs, s_to);
-}
-else {
-memcpy(ctx-buf + offs, subs, s_to);
-memmove(ctx-buf+offs+s_to,
-ctx-buf+offs+s_from,
-len + 1 - s_from - offs);
-}
+   

Re: mod_proxy, oooled backend connections and the keep-alive race condition

[Micha Lenk]

Hi Yann,

Am 09.12.2013 00:30, schrieb Yann Ylavic:
 Now, if trying to send the first bytes of the request immediately fails
 because the socket isn't connected anymore (e.g. EPIPE), you *know* that
 exactly *none* bits of your current request reached the server. For this
 reason it should be safe to retry by establishing a new connection to
 the server and to try to send the request again over the newly
 established connection.
 
 
 The send() will never fail at this point if the peer FIN-closed the
 connection (EPIPE or whatever on non-unixes), since it is only half-closed.
 You can't read anymore, but you still can write, and this is precisely
 why the error log is error *reading* status line and not error
 writing 
 A second send() could fail however, though possibly with ECONNRESET, and
 you are no better here.
 
 The point is that mod_proxy already checks whether the connection is
 alive before trying to send a (successive) request on the kept alive
 connection, by using a recv(MSG_PEEK) (see ap_proxy_is_socket_connected).
 If that fails, a new connection is established and the request is sent
 on it, this is already the current behaviour.
 The race condition is when this check succeeds, but the peer closes the
 connection before the request reaches it (in the meantime).

Okay, got that. If it really works like it should then I don't know how
to improve the code. So the remaining question is, whether the checks
whether the connection is alive are sufficient. Unfortunately I have no
resources to investigate this any further.

Regards,
Micha

Re: Reverse proxy, mod_security, segmentation fault

[Micha Lenk]

Hi Ewald,

Am 12.12.2013 10:16, schrieb Ewald Dieterich:
 [...] Is this a bug or am I doing something wrong?

I would consider the segmentation faults to be bugs. The question is
whether they are bugs in httpd or in mod_security...

Looking at the backtraces I noticed that most threads are busy in a
syscall (i.e. poll(), read() and the like) or waiting on a thread mutex
(i.e. pthread_cond_wait() etc.). Those threads are blocked and most
likely do not actively contribute to a segfault. In both segfaults you
can find a thread that is currently running a signal handler (i.e.
kill()). If I remember correctly, it is quite error prone to handle
signals in multi-threaded applications. I would assume your segfaults
are related to these typical signal handling issues with multithreading.

Hope that helps...

Regards,
Micha

Re: r-handler being overwritten with mime type

[Micha Lenk]

Hi Graham,

Am 12.12.2013 14:28, schrieb Graham Leggett:
 Does anyone know offhand where I should be looking for something that
 sets a mime type? It seems that something is setting the handler
 without checking first to see if the handler has been set already,
 and this breaks form login.

You could try to chase that down with GDB:
- set a breakpoint in line 1044
- run until the breakpoint triggers
- add a watchpoint on r-handler
- run until the watchpoint triggers and see where it is...

Hope that helps...

Cheers,
Micha

Re: mod_proxy, oooled backend connections and the keep-alive race condition

[Micha Lenk]

Hi all,

Am 05.12.2013 22:03, schrieb Yann Ylavic:
 I'm not talking about retrying requests but retrying writing on the
 socket after trying to re-open a connection. When mod_proxy tries
 to forward the client request to the backends and encounters a
 closed connection due to the described race condition it will fail
 back to the client. Instead, I suggest trying to re-open the
 connection once and then either send the client request over that
 connection or go back to the client with an error. No double-sending
 requests anywhere.
 
 I must be missing something.
 
 When mod_proxy encounters a closed (reused) connection while forwarding
 the client's request, it has just sent the request once, right?

That depends on the actual state. What Thomas is talking about (I know
because I worked with him on the same product he is referring to here)
is the state where you have everything in place to send out the next
request headers on a connection that was established by a previous
request and that was kept open using HTTP Keep-Alive. Let me stress the
fact that in this state you haven't sent a single bit of the current
request over the wire yet.

Now, if trying to send the first bytes of the request immediately fails
because the socket isn't connected anymore (e.g. EPIPE), you *know* that
exactly *none* bits of your current request reached the server. For this
reason it should be safe to retry by establishing a new connection to
the server and to try to send the request again over the newly
established connection. To avoid an endless retry loop, you certainly
should do this retry approach only once per request.

In the sense of getting the reverse proxy request correctly processed by
the backend server, this approach *does* fix the keep-alive race
condition issue where the server closes the backend connection because
it sees no (new) client request for too long time -- IIRC this is what
this discussion started with.

I hope I could shed some more some light on this issue and on the idea
how to possibly tackle it in Apache httpd.

Regards,
Micha

Re: make mod_auth_form tell you where the credentials came from

[Micha Lenk]

Hi Thomas,

Am 03.12.2013 18:04, schrieb Thomas Eckert:
 Now suppose the following
 
 [...]
 32 user fills in and submits form
 32 custom auth provider receives the user credentials
 33 custom auth provider looks up it's own session in it's module
 internal session cache
 34 custom auth provider realizes the provider internal session expired

I think what is missing here is something between 32 and 33 that
re-validates the custom auth provider's internal session, whenever valid
user credentials where received from a filled form.

Or why is it in 34 that the provider's internal session is expired?

Regards,
Micha

Re: unsetting encrypted cookies when encryption key changes

[Micha Lenk]

Hi Thomas,

Am 04.12.2013 10:53, schrieb Thomas Eckert:
   1 user tries to browse protected resource
   2 user is redirected to form
   3 user fills in and submits form
   4 user is redirected to AuthFormLoginSuccessLocation and receives
 encrypted session cookie (encrypted with key A)
   5 encryption key changes from key A to key B
   6 user tries to browse protected resource
   7 apache fails to load the session
   8 user is redirected to form
   9 user fills in and submits form
 10 user is redirected to AuthFormLoginSuccessLocation
 11 apache logs the 'failed to decrypt' and 'failed to load session' again
 12 user is redirected to form
 continue at step 9
 
 At this point the only way I found to have the user regain access is to
 delete the encrypted session cookie in the user's client. This is
 exactly where my original question sets in because I want to configure
 mod_session and friends in such a way that any cookie which failed
 decryption is simply dropped and replaced with a new one.
 
 All redirets are 302s. I did not see any 401s.
 
 The encrypted session cookie, sent out in step 4, is never changed. I
 can not see any Set-Cookie headers coming from apache, not even in step 10.

Not sending a new cookie in step 10 is probably the issue here. I would
expect apache to send a new Set-Cookie header whenever the user
succeeded authentication (i.e. is redirected to
AuthFormLoginSuccessLocation).

... hope that helps.

Regards,
Micha

Re: ap_proxy_location_reverse_map()

[Micha Lenk]

Hi,

Am 27.11.2013 12:54, schrieb Jim Jagielski:
 [...] I'm guessing that if we
 standardized on using apr_uri_parse() instead of just
 trying to parse the stuff ourselves, we'd be better off.

+1

Regards,
Micha

Re: mod_proxy, oooled backend connections and the keep-alive race condition

[Micha Lenk]

Hi Yann,

Am 03.10.2013 15:33, schrieb Yann Ylavic:
 On Thu, Oct 3, 2013 at 2:07 PM, Micha Lenk mi...@lenk.info wrote:
Independent from how the HRS issue (CVE-2005-2088) was fixed at that
time, I still believe that it is a bad idea in terms of security to
flush the buffer and forward its content to the backend before the
*whole* request body has been received.
 
 mod_proxy won't forward anything before the whole request *header* is
 received (it isn't even called before), with or without my patch(es),
 and that prevent HRS provided the httpd checks
 Content-Length/Transfer-Encoding against
 http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23#page-31
 (which is the case, at least in trunk).

 mod_proxy will also prevent HRS by checking that the received data do
 not exceed the announced size, again my patch(es) don't change that.

Well, assessing HRS alike issues, you need to not only look at the first
HTTP request, but more importantly at a possible subsequent HTTP request
sent on the same connection. So, if you flush the buffer and forward its
content to the backend, you need to make sure to not accidentally
forwarding the next request without parsing it. That is all I wanted to
stress when talking about a potential HRS issue.

 It currently prefetches 16K of the body (if any) and forwards that and
 everything after to the backend from there, it won't retain the
 *whole* body unless one plays with the proxy-sendcl env.

If this is the same behavior as with current unpatched httpd, I am fine
with it. But then I guess this behavior is unrelated to your patch, right?

 Do you mean you always setenv proxy-sendcl to force mod_proxy in
 full-fetching mode because of security issues ?

No.

 I'm not sure what changes you are talking about, though.
 Is it about the flushall or the prefetch before connect patch ?

It is about the flushall patch.

Regards,
Micha

Re: mod_proxy, oooled backend connections and the keep-alive race condition

[Micha Lenk]

Hi Yann,

Am 01.10.2013 17:08, schrieb Yann Ylavic:
 As far as I understand the issue, the main point of prefetch was to fix
 CVE-2005-2088, a HTTP Request Smuggling attack (see also
 http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2005-2088).
 
 This is discussed in PR40029 and is not related to HRS, the real fix
 regarding HRS was about both CL/TE sent by th client
 (https://issues.apache.org/bugzilla/show_bug.cgi?id=40029#c4).

Independent from how the HRS issue (CVE-2005-2088) was fixed at that
time, I still believe that it is a bad idea in terms of security to
flush the buffer and forward its content to the backend before the
*whole* request body has been received. At least I would recommend to
carefully review this change to make sure you don't create a new
security issue similar to the HRS issue.

Regards,
Micha

Re: mod_proxy, oooled backend connections and the keep-alive race condition

[Micha Lenk]

Hi all,

Am 01.10.2013 14:36, schrieb Plüm, Rüdiger, Vodafone Group:
 That's time when the proxy *thinks* the connection is valid but
 the backend thinks the connection is idle.  And in most
 reverse-proxy cases that prefetch is adding basically no value
 AFAICT - the backend is a known vintage and probably HTTP/1.1.
 So... could we make the prefetch stuff configurable away?
 
 IMHO no issue with this. Let's hear what others say. I guess the main
 point of prefetch was to make better decisions whether to use
 chunked encoding when sending to the backend. Or provide a CL to the
 backend when the real client does not.

As far as I understand the issue, the main point of prefetch was to fix
CVE-2005-2088, a HTTP Request Smuggling attack (see also
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2005-2088).

This is not an argument against making the prefetch stuff configurable,
but if this ever gets implemented, this CVE should definitely needs to
be mentioned in the documentation so that users are aware of it.

Regards,
Micha

Re: mod_proxy seg faulting ?

[Micha Lenk]

Hi Stefan,

Am 03.05.2013 14:09, schrieb Stefan Fritsch:
 On Thursday 02 May 2013, Thomas Eckert wrote:
  Lately, I've been seeing httpd/mod_proxy seg faulting in reverse
  proxy setups, frequency increasing.
 
 I am pretty sure that this is a thread-unsafe pool usage. 
 create_proxy_config() puts the global config pool into 
 (proxy_server_conf)-pool. It is later (during request processing) 
 used all over the place without further locking. This must be a sub-
 pool instead, and it must be protected with a mutex. Or new sub-pools 
 must be created wherever conf-pool is used.

Can you please elaborate the problem of thread-unsafe pool usage a bit
more so that we can better understand why this is causing a segmentation
fault? Or alternatively, do you have any pointer to documentation,
mailinglists, what ever, where to read more about it?

Thanks in advance,
Micha

Re: URL scanning by bots

[Micha Lenk]

Hi,

Am 03.05.2013 11:27, schrieb Dirk-Willem van Gulik:
 FWIIW - the same sentiments where expressed when 'greylisting[1]' in
 SMTP came in vogue. For small relays (speaking just from personal
 experience and from the vantage of my own private tiny MTA's) that
 has however not been the case. Greylisting did dampen things
 significantly - and the effect lasts to this day.

The main difference I see here is, that a SMTP server that uses
greylisting really can close the client connection almost immediately
with keeping minimal state, usually on cheap disk. So, until the client
retries, neither the kernel nor any processes have to deal with the
greylisting during the delay period.

In HTTP this is totally different. You can't just return a temporary
error code and assume that web browser will retry some reasonable
moments later. For this reason you would have to delay the real HTTP
response. And this has a substantial resource usage impact, as you have
to maintain state across all operating system layers. The network stack
needs to maintain the TCP connection open, the kernel needs to maintain
an open socket for the server process, the server process needs to
maintain an (some kind of) active HTTP request -- for every single
delayed request. These resources would just wait for the delay timer to
expire, so essentially hanging around without doing anything useful, and
without changing any outcome of the actual HTTP transaction. As others
already pointed out, this opens the doors for denial of service attacks
by excessive resource usage. From a security point of view, you
definitely don't want such HTTP response delays.

Regards,
Micha

Re: URL scanning by bots

[Micha Lenk]

Hi André

Am 03.05.2013 14:37, schrieb André Warnier:
 Basically, after a few cycles like this, all his 100 pool connections
 will be waiting for a response, and it would have no choice between
 either waiting, or starting to kill the connections that have been
 waiting more than a certain amount of time.
 Or, increasing its number of connections and become more conspicuous (***).

If I were about to run such a bot (having read this thread) I would just
let it wait for the responses on all 100 connections. As long as the bot
is busy, I could easily work on other stuff. It's just a shift in the
numbers game. So why should I care about it?

Regards,
Micha

Re: mod_proxy_websocket

[Micha Lenk]

Hi Jim,

On 03/07/2013 11:45 AM CEST +02:00, Jim Jagielski wrote:

Today, I think that I will actually rename it to mod_proxy_tunnel,
which is a more accurate description of the module... I expect
it will be used mostly to proxy tunnel websockets, but still,
the new name is better.


I would prefer mod_proxy_websocket over mod_proxy_tunnel. The term 
'tunnel' is a bit to generic in this context. E.g. it is certainly 
unable to tunnel RPC (bugzilla issue #40029), so it is definitely not a 
generic tunnel.


Regards,
Micha

Re: mod_proxy_websocket

[Micha Lenk]

Hi Jim,

On 03/07/2013 03:08 PM CEST +02:00, Jim Jagielski wrote:

It's an http* tunnel.


Yet I bet it isn't able to tunnel RPC over HTTP, as described in 
Bugzilla #40029. If my humble opinion counts, please stick with 
mod_proxy_websocket.


Micha

Re: mod_proxy_websocket

[Micha Lenk]

Hi Jim,

On 03/06/2013 02:51 PM CEST +02:00, Jim Jagielski wrote:

As trunk and commit watchers may have noticed, I've added
a rough mod_proxy_websocket extension module to trunk. The
basic idea was to have a simple tunnel that could be used
to proxy websocket requests, and that's the design... I still
need to add the basic 'send initial rec'd headers' to the
backend to get it functional.

Also not sure if a true tunnel makes the most sense,
but it's a start ;)


How is your module related to this module?
https://github.com/disconnect/apache-websocket
I guess yours is intended to proxy websockets, whereas the latter is 
intended to handle websockets locally somehow.


However, using mod_websocket from the mentioned Github location, I 
discovered that it has timeout issues when mod_reqtimeout is loaded too 
(unless request body timeouts are disabled). Apparently mod_reqtimeout 
now enforces timeouts in non-blocking I/O mode independently from 
apr_socket timeouts. I.e. the call to apr_socket_timeout_set() in 
mod_websocket.c doesn't disable all active timeouts anymore. The result 
is that the websocket is disconnected after 20 seconds 
(RequestReadTimeout default for body timeout).


How does your module handle these timeouts?

Regards,
Micha

Re: mod_proxy_websocket

[Micha Lenk]

Hi Jim,

Am 06.03.2013 18:22, schrieb Jim Jagielski:
 How does your module handle [the mod_reqtimeout] timeouts?

Still unanswered,,,

Regards,
Micha

Re: [PATCH] NTLM via Apache as reverse proxy [Bug 39673]

[Micha Lenk]

Hi,

On 02/26/2013 06:55 PM CEST +02:00, I wrote:

I am currently working on bugzilla #39673. The issue here is that NTLM
authentication as used in Microsoft products don't work. [...]
Looking at how mod_proxy_ftp.c solves a similar problem, I tried to
solve that issue with the attached patch (also attached to bugzilla). [...]


Please note that this patch currently causes segfaults, so please don't 
use it unless you intend to improve it. I will improve it as soon as 
time permits.


Regards,
Micha

[PATCH] NTLM via Apache as reverse proxy [Bug 39673]

[Micha Lenk]

Hi all,

I am currently working on bugzilla #39673. The issue here is that NTLM 
authentication as used in Microsoft products don't work. The reason is 
that the brilliant engineers at Microsoft who invented this 
authentication scheme assumed that subsequent client requests sent over 
a single connection towards the server (i.e. using keep alive) also 
reach the server on a single connection. But as Rüdiger Plüm noted some 
years ago in bugzilla #39673:


  the current 2.2.x proxy implementation does NOT support
   NTLM, because there is no guarantee that the same backend
   connection is used for the next request on a keepalive
   frontend connection. Each request from a frontend connection
   leases a backend connection from a connection pool for the
   request and returns it back to the pool immediately after
   the request has been processed. If the next request on
   this keepalive frontend connection is processed it may
   lease a different backend connection from the pool. As far
   as I understand NTLM this approach is not compatible with
   NTLM.

Looking at how mod_proxy_ftp.c solves a similar problem, I tried to 
solve that issue with the attached patch (also attached to bugzilla). If 
a NTLM request is detected, the cleanup of the currently leased backend 
connection is skipped. Instead the backend connection is registered with 
the client connection pool, so that it is closed and cleaned up as soon 
as the client disconnects. Additionally the backend connection is 
registered as a config record in the client connection, so that it can 
get re-used for subsequent requests on the same client connection.


I would like to solicit some feedback about the approach. What would I 
need to change for the patch being accepted in trunk?


Regards,
Micha
diff -aur httpd-2.2.22/modules/proxy/mod_proxy_http.c httpd-2.2.22.patched/modules/proxy/mod_proxy_http.c
--- httpd-2.2.22/modules/proxy/mod_proxy_http.c	2011-02-14 09:04:57.0 +0100
+++ httpd-2.2.22.patched/modules/proxy/mod_proxy_http.c	2013-02-26 18:21:07.0 +0100
@@ -1911,6 +1911,26 @@
 }
 
 /*
+ * This is a hook to be registered with the client connection pool that should
+ * ensure the backend connection is closed as soon as the client connection is
+ * closed. As NTLM appears to authenticate TCP connections instead of HTTP
+ * requests (which is broken by design), we want to really close the backend
+ * connection and not let other clients re-use it. Otherwise the other clients
+ * may get authenticated with foreign auth credentials they don't own.
+ */
+typedef struct {
+const char *proxy_function;
+proxy_conn_rec *backend;
+server_rec *server;
+} http_backend_cleanup_t;
+static apr_status_t proxy_http_ntlm_disconnect_backend(void *data) {
+http_backend_cleanup_t *d = (http_backend_cleanup_t *)data;
+d-backend-close = 1;
+ap_proxy_release_connection(d-proxy_function, d-backend, d-server);
+return APR_SUCCESS;
+};
+
+/*
  * This handles http:// URLs, and other URLs using a remote proxy over http
  * If proxyhost is NULL, then contact the server directly, otherwise
  * go via the proxy.
@@ -1929,6 +1949,9 @@
 char *scheme;
 const char *proxy_function;
 const char *u;
+const char *auth_hdr;
+int is_ntlm_request = 0;
+http_backend_cleanup_t *cleanup_data;
 proxy_conn_rec *backend = NULL;
 int is_ssl = 0;
 conn_rec *c = r-connection;
@@ -1973,6 +1996,12 @@
 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r-server,
  proxy: HTTP: serving URL %s, url);
 
+/* check whether we need to re-use an already established backend connection */
+backend = (proxy_conn_rec *)ap_get_module_config(c-conn_config, proxy_http_module);
+if (backend) {
+ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r-server,
+ proxy: re-using already established connection to backend server);
+} else {
 
 /* create space for state information */
 if ((status = ap_proxy_acquire_connection(proxy_function, backend,
@@ -2027,6 +2056,32 @@
 }
 }
 
+/*
+ * check whether we have an NTLM request that requires
+ * client connections mapping to exactly one backend connection
+ * and no shared usage of backend connections by multiple clients.
+ */
+auth_hdr = apr_table_get(r-headers_in, WWW-Authenticate);
+if (auth_hdr  (!strcasecmp(auth_hdr, NTLM) ||
+ !strncasecmp(auth_hdr, NTLM , 5))) {
+is_ntlm_request = 1;
+
+/* enforce that this backend connection is not shared with other
+ * clients. */
+backend-close = 1;
+
+/* Register backend connection with client connection. */
+cleanup_data = apr_pcalloc(c-pool, sizeof(cleanup_data));
+cleanup_data-proxy_function = proxy_function;
+cleanup_data-backend = backend;
+cleanup_data-server = r-server;
+

Re: Documentation on mod_slotmem_shm

[Micha Lenk]

Hi Rainer,

On 02/05/2013 08:04 PM CEST +02:00, Rainer Jung wrote:

Example code using it: modules/cluster/mod_heartmonitor.c should be
simplest, look for slotmem and SLOTMEM in there. That should at least
show how to bootstrap and use slotmem.


Okay, the longer I am reading anything related to mod_slotmem_shm, the 
more I get the impression that it is the wrong tool for my task. If I 
understood it correctly, mod_slotmem_shm organizes data in slots. This 
appears to make it unnecessarily limited to the number of slots I 
initialize it with.


What I need is a way to transfer information from one long living 
request to another entirely independent request some time later. The 
first request contains a cookie (i.e. a random string), which is also 
contained in the second request. I would like to use this cookie as a 
handle to store some data (fixed size, if needed) generated during 
processing of the first request somewhere (at least for the life time of 
the first request). The second request then needs to be able to use the 
cookie to access the data the first request already generated. Moreover, 
an arbitrary number of such request pairs can happen at the same time.


Given the requirements above, is the mod_slotmem_shm API still the right 
tool for this purpose?


I just started looking at mod_socache_shmcb -- is this maybe better 
suited for the purpose described above?


I could also go for a custom shared memory usage implementation, but I 
thought making use of any already existing API might be a better idea 
(trying avoid implementing things twice). So, any hint is appreciated...


Regards,
Micha

Documentation on mod_slotmem_shm

[Micha Lenk]

Hi all,

I am currently working on an Apache module that needs to maintain some 
shared data that is used by all requests, on all workers. This data does 
not need to be persistent, so I thought about using shared memory for 
this task. Someone pointed me to mod_slotmem_shm (hi Rainer), but now I 
am a bit lost on how to use it.


Does anybody have a pointer to documentation or example source code on 
how to use mod_slotmem_shm? Any suggestions are welcome.


Regards,
Micha

Re: [users@httpd] Proxy CONNECT HTTP version

[Micha Lenk]

Hi Pavel,

On 01/30/2013 11:41 AM CEST +02:00, Pavel Mateja wrote:

The wget package will be in next stable debian which sux because all CONNECT
requests thru apache will fail.

I just compiled wget from git (1.14.31-3be7) and the bug is still there.

Any idea what to do?
Should somebody fill bug for debian and wget or should we ignore that RFC
request?


If wget fails to send a Host: header when RFC mandates it, I recommend 
you to file a bug against wget in the Debian bug tracker.


Regards,
Micha

Apache httpd at FOSDEM?

[Micha Lenk]

Hi all,

I am going to attend (first time) the FOSDEM this year and would like to
know if there is a chance to meet Apache httpd developers there. I
didn't find a dedicated httpd event in the schedule. Is there any other
typical event or occasion where to meet some of you?

Regards,
Micha

Re: [Bug 51489] ProxyPassReverse adds an additional slash in load balancer setups

[Micha Lenk]

Hi,

On 11/09/2012 01:23 PM CEST +02:00, Mario A. del Riego wrote:
 Anyone knows about this bug but for Apache 2.2? or a workaround?
 I saw the patch for Apache 2.4 only.

It should be easy to backport the fix for the httpd-2.2 branch.
What is needed to get it done?

Regards,
Micha

Re: [PATCH] mod_xml2enc eats end of file

[Micha Lenk]

Hi Nick,

On 11/02/2012 07:25 PM CEST +02:00, Nick Kew wrote:
 just debugged a case where Apache used as reverse proxy filters a
 text/javascript file through mod_proxy_html and mod_xml2enc. As
 mod_proxy_html sees no business in filtering that file, it removes
 itself from the filter chain, but mod_xml2enc still tries to do its job.

 That looks like a logic bug you've found!

yes, that's also possible.

 It looks like an edge case: one you'll only see when the charset coming
 from the backend is not supported by libxml2 on your platform, so that
 mod_xml2enc converts it using apr_iconv.

No, not exactly this edge case. The backend server sends the response
header Content-Type: text/javascript, i.e. without any information
about the used charset. From what I've seen in GDB, mod_xml2enc seems to
resort to assume that the server sends ISO-8859-1 and without an error
converts that to UTF-8 (even though in fact it seems to be mixed
ISO-8859-1 and UTF-8).

 The attached patch based on httpd-trunk fixes that issue by removing the
 Content-Length header entirely. Please review it. I would appreciate it,
 if it could get applied to trunk and then backported to the httpd-2.4.x
 branch.

 Your patch fixes the immediate bug (thanks!), but the fact that
 mod_xml2enc is doing anything at all in the case you describe is a
 bigger bug.

Ok, I too wondered about mod_xml2enc staying active being a bug, but was
not sure. So I only fixed the immediate bug. If you consider this a bug,
I assume the Content-Type check just needs to be unified. In
mod_proxy_html check_filter_init() checks for text/html or
application/xhtml+xml, whereas in mod_xml2enc xml2enc_ffunc() checks
for prefix text/ or xml anywhere in the content type string. This is
not consistent, and causes mod_proxy_html to skip text/javascript (or
text/css) files, while mod_xml2enc takes them.

 There's no easy solution: mod_proxy_html delays some of the checks
 until it has a first chunk of data, to allow for cases where an earlier
 filter (e.g. XSLT) might affect Content-Type.  But by that time it's
 too late to insert or uninsert the xml2enc filter, as that needs to go
 in front of the proxy_html filter.

Yes, the delayed checks also seem necessary for the charset guessing in
case no charset is specified.

But what about making the Content-Type check consistent?

Regards,
Micha

[PATCH] mod_xml2enc eats end of file

[Micha Lenk]

Hi,

I just debugged a case where Apache used as reverse proxy filters a
text/javascript file through mod_proxy_html and mod_xml2enc. As
mod_proxy_html sees no business in filtering that file, it removes
itself from the filter chain, but mod_xml2enc still tries to do its job.
In course of mod_xml2enc's processing, the file grows a few bytes due to
some charset conversions. But as the original server sends a
Content-Length header which is not removed by mod_xml2enc, no more than
the original length of the file is sent to the client.

The attached patch based on httpd-trunk fixes that issue by removing the
Content-Length header entirely. Please review it. I would appreciate it,
if it could get applied to trunk and then backported to the httpd-2.4.x
branch.

Regards,
Micha
Index: modules/filters/mod_xml2enc.c
===
--- modules/filters/mod_xml2enc.c	(revision 1405021)
+++ modules/filters/mod_xml2enc.c	(working copy)
@@ -370,6 +370,9 @@
 /* nah, we only have one action here - call it inline */
 fix_skipto(f-r, ctx);
 
+/* we might change the Content-Length, so let's force its re-calculation */
+apr_table_unset(f-r-headers_out, Content-Length);
+
 /* consume the data we just sniffed */
 /* we need to omit any meta we just invalidated */
 ctx-flags |= ENC_INITIALISED;

Re: Bug with ProxyPass / and mod_proxy_balancer + double-slashes (httpd-2.4.3)

[Micha Lenk]

Hi Zisis,

Am 06.09.2012 08:07, schrieb Zisis Lianas:

Tom, thanks for your feedback.

The main difference between our configurations is that you do
ProxyPassReverse the single BalancerMember (http://app05/...),
which is also working for me - in my configuration I ProxyPassReverse
the balancer://cluster. And this is the configuration which does
not work correctly.

So...
  ProxyPassReverse / http://app05/ =  WORKS
  ProxyPassReverse / balancer://cluster/ =  DOES NOT WORK CORRECTLY

As documented the balancer://... ProxyPassReverse should work:
http://httpd.apache.org/docs/2.4/mod/mod_proxy_balancer.html#example

So I think this is a bug.


As Tom confirmed, this is a bug not with ProxyPass but with 
ProxyPassReverse, which I also tried to report as issue #51489 (thanks 
for mentioning it). I already traced it down with gdb to one particular 
code path that seems to be broken. Checkout my most recently patch 
attached to issue #51489 for details.


Jim, you asked what exactly DOES NOT WORK CORRECTLY mean, I will try to 
describe that again in my own words. It means that ProxyPassReverse does 
its job, but adds an additional slash to the redirect target URL, which 
is inserted after the hostname. E.g. if the HTTP response from backend 
with status code 301 contains a Location header like this:


Location: http://backend01.foo:5080/clusterjsp/

then the HTTP response from reverse proxying Apache after processing the 
ProxyPassReverse configuration would look like this:


Location: http://mydomain.foo:8080//clusterjsp/
   ^--- this slash shouldn't be there

If you leave out the trailing slash in the ProxyPassReverse directive, 
Apache is unable to process the request (HTTP 500 error mentioned by 
Zisis) due to the broken configuration. If the ProxyPassReverse 
configuration matches the ProxyPass configuration, you get the 
additional slash.


As I wrote already above, I analyzed the code flow already and 
identified where in the code the problem exists. Apparently the corner 
case is hit when the destination URL in ProxyPassReverse has a 
slash-only path component (i.e. '/').


The available patch attached to issue #51489 should fix the issue, 
however someone more familiar with the Apache code should probably 
review the patch before applying it in SVN.


Regards,
Micha

Re: [Bug 51489] ProxyPassReverse adds an additional slash in load balancer setups

[Micha Lenk]

Hi Joe,

as you are apparently not subscribed to Bugzilla PR 51489, I am
answering to your comment on that PR via mail. Please apologize in case
you now got my answer twice.

On 08/17/2012 03:56 PM CEST +02:00, bugzi...@apache.org wrote:
 https://issues.apache.org/bugzilla/show_bug.cgi?id=51489
 
 Joe Orton jor...@redhat.com changed:
 
What|Removed |Added
 
  Status|NEW |RESOLVED
  Resolution|--- |FIXED
 
 --- Comment #11 from Joe Orton jor...@redhat.com ---
 Ah, I missed this bug before, sorry.  Yes, this is fixed for the next 2.4.x
 release, I'll update CHANGES to ref this bug since it's a different bug to the
 2.2 issue, and a regression.

I fear you got me wrong. The changeset you committed in SVN rev. 1374264
is wrong and should be reverted ASAP. The bug that got fixed in branch
httpd-2.4.x by SVN rev. 1365604 is PR 45434 only. The issue that I
reported in PR 51489 is *not* fixed by changing the CHANGES file only
(as in SVN rev. 1374264).

Or did I miss anything?

Regards,
Micha

Re: [Bug 51489] ProxyPassReverse adds an additional slash in load balancer setups

[Micha Lenk]

Hi Joe,

On 08/17/2012 05:00 PM CEST +02:00, Joe Orton wrote:
 On Fri, Aug 17, 2012 at 04:53:54PM +0200, Micha Lenk wrote:
 as you are apparently not subscribed to Bugzilla PR 51489, I am
 answering to your comment on that PR via mail. Please apologize in case
 you now got my answer twice.
 
 Thanks Micha, I get the bug mail via bugs@, but no problem.  I've 
 reverted the CHANGES entry again!

Thanks.

@everybody: What about applying and committing the new suggested fix
(see attachment)? It is based on branch httpd-2.4.x, SVN rev. 1374290.

Regards,
Micha
Index: modules/proxy/proxy_util.c
===
--- modules/proxy/proxy_util.c	(revision 1374290)
+++ modules/proxy/proxy_util.c	(working copy)
@@ -895,6 +895,8 @@
 }
 else if (l1 = l2  strncasecmp((*worker)-s-name, url, l2) == 0) {
 u = apr_pstrcat(r-pool, ent[i].fake, url[l2], NULL);
+if ((ent[i].fake[0] == '/')  (ent[i].fake[1] == 0)  (url[l2] == '/'))
+u++;
 return ap_is_url(u) ? u : ap_construct_url(r-pool, u, r);
 }
 worker++;

Re: mod_proxy_html

[Micha Lenk]

Hi,

Am 11.10.2011 00:37, schrieb Aaron Bannert:
 I've been working with mod_proxy_html for the last few months, and I
 think this would be a good addition to HTTPD. mod_proxy_html could
 also benefit from some better collaboration, and I personally have
 some bug fixes and improvements lined up that I could provide. I
 don't know what the tradeoffs are between having it be a normal
 included module or a subproject module, does anyone have any
 insight?

+1

Me too. I have some patches for mod_proxy_html around that I would like
to contribute. For instance I have a nice patch that lets it
automatically detect whether the parsed document is XHTML or HTML and
automatically switches to the appropriate output mode. This is
especially usefull for websites with mixed HTML and XHTML documents...

Regarding httpd integration I don't care. I could live with both,
mod_proxy_html being a separate module or released as part of httpd.

Regards,
Micha

Re: With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

Hi Rüdiger,

Am 09.09.2011 11:45, schrieb Plüm, Rüdiger, VF-Group:
 Please update the bug report:
 
 1. Declare the unneeded patch obsolete.
 2. Update the needed patch to be against trunk.

Done. See https://issues.apache.org/bugzilla/show_bug.cgi?id=51709
What else is needed to get it applied to trunk?

Regards,
Micha

Re: [PATCH 51709] With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

Hi,

Am 09.09.2011 11:45, schrieb Plüm, Rüdiger, VF-Group:
 2. Update the needed patch to be against trunk.

Once again with attached patch for issue 51709, ready for review. Please
review it and consider it for commit.

Thanks,
Micha
Index: httpd-trunk/server/vhost.c
===
--- httpd-trunk/server/vhost.c	(Revision 1171988)
+++ httpd-trunk/server/vhost.c	(Arbeitskopie)
@@ -860,9 +860,11 @@
 const char *host = r-hostname;
 apr_port_t port;
 server_rec *s;
+server_rec *virthost_s;
 server_rec *last_s;
 name_chain *src;
 
+virthost_s = NULL;
 last_s = NULL;
 
 port = r-connection-local_addr-port;
@@ -889,23 +891,34 @@
 
 s = src-server;
 
-/* does it match the virthost from the sar? */
-if (!strcasecmp(host, sar-virthost)) {
-goto found;
+/* If we still need to do ServerName and ServerAlias checks for this
+ * server, do them now.
+ */
+if (s != last_s) {
+/* does it match any ServerName or ServerAlias directive? */
+if (matches_aliases(s, host)) {
+goto found;
+}
 }
-
-if (s == last_s) {
-/* we've already done ServerName and ServerAlias checks for this
- * vhost
- */
-continue;
-}
 last_s = s;
 
-if (matches_aliases(s, host)) {
-goto found;
+/* Fallback: does it match the virthost from the sar? */
+if (!strcasecmp(host, sar-virthost)) {
+/* only the first match is used */
+if (virthost_s == NULL) {
+virthost_s = s;
+}
 }
 }
+
+/* If ServerName and ServerAlias check failed, we end up here.  If it
+ * matches a VirtualHost, virthost_s is set. Use that as fallback
+ */
+if (virthost_s) {
+s = virthost_s;
+goto found;
+}
+
 return;
 
 found:

Re: With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

Hi Rüdiger,

On 08/23/2011 12:25 PM CEST +02:00, Plüm, Rüdiger, VF-Group wrote:
 IMHO the patch does not solve the issue mentioned in the comment
 and is not needed.
 Keep in mind the difference between ap_matches_request_vhost and
 check_host_alias:
 
 ap_matches_request_vhost checks only the data of *one* server_rec
 and hence only *one* Servername and ServerAlias settings as set for
 this vhost, whereas check_host_alias scans over *multiple* server_recs
 with *multiple* Servername and ServerAlias settings from different vhosts.

Okay, good point. Then the first patch should indeed be sufficient.

What is needed to get it applied to trunk?

Regards,
Micha

CVE-2011-3192 and mod_proxy

[Micha Lenk]

Hi,

I've just installed the recent security update (package apache2
2.2.16-6+squeeze2) that Debian just released for fixing CVE-2011-3192. I
made a short test request for a test document with exactly 10 bytes
length and with a header

Range: bytes=0-4,5-9,0-9

to test whether the patch works as expected.

For content that is not proxied, the security fix seems to fix the
issue, i.e. the full document is delivered unchunked as a whole.

But for content that is proxied via mod_proxy_http, the request
including the bad Range: header hits the backend server. So, if the
backend server is also an Apache, which is still vulnerable to
CVE-2011-3192, it would receive malicious Range headers unfiltered.

Is this intended behavior? Couldn't we filter out bad ranges on proxy
request too?

If this is not intended to be fixed, it is maybe a good idea to include
an explicit hint about this behavior in the security advisory.

However, the workaround from earlier announcements using following lines
in the configuration does also work for a proxied request:

# Drop the (Request-)Range header if more than 5 ranges (CVE-2011-3192)
SetEnvIf Range (,.*?){5} bad-range=1
RequestHeader unset Range env=bad-range
SetEnvIf Request-Range (,.*?){5} bad-request-range=1
RequestHeader unset Request-Range env=bad-request-range

Regards,
Micha

Re: With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

Hi,

On 08/22/2011 07:05 PM CEST +02:00, Eric Covener wrote:
 Do you agree that this is something that needs to be fixed?
 If yes I could start to work on a patch...
 
 I was skeptical but it certainly looks busted for non-wildcard NVH'es
 (that can match the strcmp with the VH addr) like you've explained.
 
 I suggest opening a bugzilla ticket to track.

I've just opened ticket #51709.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51709

Please find attached my patch for check_hostalias(). I tried to stick to
the idea to do the ServerName and ServerAlias check only once for each
server. Also for this reason the result is in the end merely a rewrite
of this function. I hope though that it is clear enough how it is
intended to work.

However, I believe the fix is yet incomplete. The function
ap_matches_request_vhost() used by modules like mod_proxy seems to
implement the virtual host check also in the wrong order. From reading
the comments within that function, apparently someone else wondered
about my issue in the past too:

/* search all the VirtualHost values */
/* XXX: If this is a NameVirtualHost then we may not be doing the
 *  Right Thing. consider:
 *
 * NameVirtualHost 10.1.1.1
 * VirtualHost 10.1.1.1
 * ServerName v1
 * /VirtualHost
 * VirtualHost 10.1.1.1
 * ServerName v2
 * /VirtualHost
 *
 * Suppose r-server is v2, and we're asked to match 10.1.1.1.
 *  We'll say yup it's v2, when really it isn't...
 * if a request came in for 10.1.1.1 it would really go to v1.
 */

I'll follow up with a patch for this function later.

Regards,
Micha
diff -ur httpd-2.2.19.orig//server/vhost.c httpd-2.2.19.patched//server/vhost.c
--- httpd-2.2.19.orig//server/vhost.c	2010-08-10 21:11:40.0 +0200
+++ httpd-2.2.19.patched//server/vhost.c	2011-08-23 10:31:49.827506728 +0200
@@ -872,9 +872,11 @@
 const char *host = r-hostname;
 apr_port_t port;
 server_rec *s;
+server_rec *virthost_s;
 server_rec *last_s;
 name_chain *src;
 
+virthost_s = NULL;
 last_s = NULL;
 
 port = r-connection-local_addr-port;
@@ -901,23 +903,34 @@
 
 s = src-server;
 
-/* does it match the virthost from the sar? */
-if (!strcasecmp(host, sar-virthost)) {
-goto found;
-}
-
-if (s == last_s) {
-/* we've already done ServerName and ServerAlias checks for this
- * vhost
- */
-continue;
+/* If we still need to do ServerName and ServerAlias checks for this
+ * server, do them now.
+ */
+if (s != last_s) {
+/* does it match any ServerName or ServerAlias directive? */
+if (matches_aliases(s, host)) {
+goto found;
+}
 }
 last_s = s;
 
-if (matches_aliases(s, host)) {
-goto found;
+/* Fallback: does it match the virthost from the sar? */
+if (!strcasecmp(host, sar-virthost)) {
+/* only the first match is used */
+if (virthost_s == NULL) {
+virthost_s = s;
+}
 }
 }
+
+/* If ServerName and ServerAlias check failed, we end up here.  If it
+ * matches a VirtualHost, virthost_s is set. Use that as fallback
+ */
+if (virthost_s) {
+s = virthost_s;
+goto found;
+}
+
 return;
 
 found:

Re: With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

Hi,

On 08/23/2011 10:42 AM CEST +02:00, Micha Lenk wrote:
 However, I believe the fix is yet incomplete. The function
 ap_matches_request_vhost() used by modules like mod_proxy seems to
 implement the virtual host check also in the wrong order. [...]
 
 I'll follow up with a patch for this function later.

Here you go. Attached is the suggested patch for the function
ap_matches_request_vhost().

Anything else missing?

Regards,
Micha
diff -u4 -r httpd-2.2.19.orig//server/vhost.c httpd-2.2.19.patched//server/vhost.c
--- httpd-2.2.19.orig//server/vhost.c	2010-08-10 21:11:40.0 +0200
+++ httpd-2.2.19.patched//server/vhost.c	2011-08-23 12:01:04.247092813 +0200
@@ -820,38 +820,22 @@
 server_addr_rec *sar;
 
 s = r-server;
 
-/* search all the VirtualHost values */
-/* XXX: If this is a NameVirtualHost then we may not be doing the Right Thing
- * consider:
- *
- * NameVirtualHost 10.1.1.1
- * VirtualHost 10.1.1.1
- * ServerName v1
- * /VirtualHost
- * VirtualHost 10.1.1.1
- * ServerName v2
- * /VirtualHost
- *
- * Suppose r-server is v2, and we're asked to match 10.1.1.1.  We'll say
- * yup it's v2, when really it isn't... if a request came in for 10.1.1.1
- * it would really go to v1.
- */
+/* search the ServerName and all the ServerAlias values */
+if ((port == s-port)  matches_aliases(s, host)) {
+return 1;
+}
+
+/* Fallback: search all the VirtualHost values */
 for (sar = s-addrs; sar; sar = sar-next) {
 if ((sar-host_port == 0 || port == sar-host_port)
  !strcasecmp(host, sar-virthost)) {
 return 1;
 }
 }
 
-/* the Port has to match now, because the rest don't have ports associated
- * with them. */
-if (port != s-port) {
-return 0;
-}
-
-return matches_aliases(s, host);
+return 0;
 }
 
 
 static void check_hostalias(request_rec *r)

Re: [PATCH 51489] ProxyPassReverse issue + patch

[Micha Lenk]

Hi Ruediger,

sorry, things piled up recently...

On 07/13/2011 06:36 PM CEST +02:00, Ruediger Pluem wrote:
 Try
 
 ProxyPassReverse balancer://196f045aca6adc82a0b6eea93ed286a1
 
 instead.

This results in an Internal Server Error:

[warn] proxy: No protocol handler was valid for the URL request URL.
If you are using a DSO version of mod_proxy, make sure the proxy
submodules are included in the configuration using LoadModule.

Doesn't seem to be a solution... :(

Regards,
Micha

Re: [PATCH 51489] ProxyPassReverse issue + patch

[Micha Lenk]

Hi Ruediger,

On 08/22/2011 02:40 PM CEST +02:00, Plüm, Rüdiger, VF-Group wrote:
 On 07/13/2011 06:36 PM CEST +02:00, Ruediger Pluem wrote:
 Try

 ProxyPassReverse balancer://196f045aca6adc82a0b6eea93ed286a1

 instead.

 This results in an Internal Server Error:

 [warn] proxy: No protocol handler was valid for the URL request URL.
 If you are using a DSO version of mod_proxy, make sure the proxy
 submodules are included in the configuration using LoadModule.

 Doesn't seem to be a solution... :(
 
 This means that your config is wrong and you have not loaded the required 
 protocol
 handlers like mod_proxy_http or mod_proxy_ajp.
 Or your config does not load mod_proxy_balancer.

I do have the protocol handler mod_proxy_http loaded and
mod_proxy_balancer is loaded as well.

However, I guess the 'your config is wrong' is nevertheless true. I
found out that I have no issue at all if the config consistently uses a
slash after the load balancer id.

Proxy balancer://196f045aca6adc82a0b6eea93ed286a1/
BalancerMember http://server-1.local/ status=-SE
BalancerMember http://server-2.local/ status=-SE
/Proxy
VirtualHost 10.8.16.33:80
ServerName frontend.local

Location /
ProxyPass balancer://196f045aca6adc82a0b6eea93ed286a1/
ProxyPassReverse balancer://196f045aca6adc82a0b6eea93ed286a1/
/Location
/VirtualHost

So, please consider the case as solved and the patch as void.

Regards,
Micha

With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

Hi,

I have configuration with two virtual hosts v1 and v2, both listening on
the same IP address. The configuration for the virtual hosts basically
looks like this:

Listen: 10.0.0.1:80 http
NameVirtualHost 10.0.0.1:80
VirtualHost 10.0.0.1:80
ServerName v1
DocumentRoot /srv/v1
/VirtualHost
VirtualHost 10.0.0.1:80
ServerName v2
ServerAlias 10.0.0.1
DocumentRoot /srv/v2
/VirtualHost

For HTTP requests with the request header 'Host: 10.0.0.1:80', the
request always ends up on virtual host v1, delivering the content from
/srv/v1. This seems to be wrong, but at least it is unexpected.

I want to avoid using fully qualified host names as parameters to
VirtualHost, because for some reason I can't rely on the host names
being present in DNS, which in the worst case would cause the virtual
host to be skipped with the following error:

Name or service not known: Could not resolve host name v1 -- ignoring!

I can re-order the configuration to setup the virtual host v2 with the
IP literal as ServerAlias first. This will then work, but I wonder how
reliable this would be.

From reading the code (version 2.2.19) I discovered, that in the
function check_hostalias() defined in server/vhost.c the host header is
matched against the parameter from the VirtualHost container first,
before it is then matched against any ServerName or ServerAlias
directive. And as soon as the first VirtualHost seems to match, no
ServerName or ServerAliases are checked. So, essentially any name or IP
literal specified as parameter to VirtualHost seems to have precedence
over all ServerName or ServerAlias directives.

I've already consulted the SVN history. Apparently this code flow didn't
change since the NameVirtualHost directive was added (SVN Rev. 79345)
back in October 1997, and maybe is even older. Does anybody know a
reason for this code flow?

Do you agree that this is something that needs to be fixed?
If yes I could start to work on a patch...

Regards,
Micha

Re: With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

Hi Ruediger,

On 08/22/2011 06:16 PM CEST +02:00, Plüm, Rüdiger, VF-Group wrote:
 No, this works as designed and documented for ages. I guess your question is 
 better
 suited for the us...@httpd.apache.org list.

Sorry, I disagree. The documentation reads:

  Now when a request arrives, the server will first check if it is
  using an IP address that matches the NameVirtualHost. If it is, then
  it will look at each VirtualHost section with a matching IP address
  and try to find one where the ServerName or ServerAlias matches the
  requested hostname. If it finds one, then it uses the configuration
  for that server. If no matching virtual host is found, then the first
  listed virtual host that matches the IP address will be used.

  Source: http://httpd.apache.org/docs/2.2/en/vhosts/name-based.html

In the case I described earlier, *only* the virtual server v2 has a
ServerName that matches the requested hostname. The last sentence from
the cited documentation doesn't apply here.

Regards,
Micha

Re: With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

Hi Rüdiger,

On 08/22/2011 06:39 PM CEST +02:00, Plüm, Rüdiger, VF-Group wrote:
 Sorry, I missed the ServerAlias for the IP in the second virtual host.
 So, yes in general the second virtual host should be hit.
 But using IP addresses as Serveralias is quite unusual and in this case
 the solution is to just reverse the definition of the virtual hosts.

Yes, an IP address in the HTTP Host header is quite unusual, granted!
But this is only how I discovered the issue...

The same problem exists for the following virtual host configuration,
assuming that v2.test.local has a proper DNS record that resolves to a
local IP address:

Listen v2.test.local:80 http
NameVirtualHost v2.test.local:80
VirtualHost v2.test.local:80
ServerName v1.test.local
DocumentRoot /srv/v1
/VirtualHost
VirtualHost v2.test.local:80
ServerName v2.test.local
DocumentRoot /srv/v2
/VirtualHost

In this case a HTTP/1.1 request for http://v2.test.local/ is served the
content from /srv/v1. Again, the problem is, that, if it matches, the
parameter from VirtualHost seems to override any ServerName or
ServerAlias directive.

Regards,
Micha

Re: With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

Hi Nick,

On 08/22/2011 06:45 PM CEST +02:00, Nick Kew wrote:
 On Mon, 22 Aug 2011 18:27:10 +0200
 Micha Lenk mi...@lenk.info wrote:
 
 In the case I described earlier, *only* the virtual server v2 has a
 ServerName that matches the requested hostname.
 
 No it doesn't.  ServerName or ServerAlias arguments are just strings,
 so the :80 makes it a non-match.

Which ':80' do you mean? The port of the Host: header is stripped from
the request_rec.hostname by calling fix_hostname(r) in
ap_update_vhost_from_headers() before doing any string comparisons...

Regards,
Micha

Re: With IP address in Host: header ServerName/ServerAlias doesn't work

[Micha Lenk]

On 08/22/2011 06:57 PM CEST +02:00, Reindl Harald wrote:
 but why the fuck are you using hostnames there especially for Listen?

I don't. This was just an example to show that the problem doesn't
depend on the usage of IP addresses. Wheter I use a hostname or an IP
address for 'Listen' is irrelevant to this problem. So, sorry for adding
the confusion...

Regards,
Micha

Re: [PATCH 51489] ProxyPassReverse issue + patch

[Micha Lenk]

Hi Jim,

Am 10.07.2011 16:57, schrieb Jim Jagielski:

Try:

Proxy balancer://196f045aca6adc82a0b6eea93ed286a1/


Thank you for your hint. I tried that, but it doesn't change anything. :(

Micha

Re: [PATCH 51489] ProxyPassReverse issue + patch

[Micha Lenk]

Hi Nick,

On 9 Jul 2011, at 11:16, Nick Kew wrote:

On 8 Jul 2011, at 17:29, Micha Lenk wrote:


I'm using Apache as a reverse proxy in a simple load balancer setup.
I use ProxyPassReverse in order to re-write the backend server name in HTTP 
redirections (ie. in the Location header of the HTTP response).
My configuration for the virtual host essentially looks like this:

Proxy balancer://196f045aca6adc82a0b6eea93ed286a1
BalancerMember http://server-1.local status=-SE
BalancerMember http://server-2.local status=-SE
/Proxy


Is the absence of slashes after local there intentional?  You have a
slash in your ProxyPass[Reverse] directives.


Yes, the absence of slashes after local is intentional. If I added them, 
the double slash in the Location: header of a redirect response would 
indeed be gone, but it would in turn cause a double slash to be added to 
*every* backend request. In case of a thttpd as backend, this would 
cause the backend to refuse all requests entirely. :(


This seems to be different issue, but maybe I should try to fix this 
instead...



What I bother about is the additional slash before '/foo/', so
I  digged into the source code and found the following lines in
modules/proxy/proxy_util.c:


That's not easy to follow: your comments don't cross-reference to the code
until I've opened a lot more windows than I have screen space for.
A link to your bugzilla report would've helped, and your reference to
source is ambiguous - I guessed /trunk/.


Correct.


When digging in source for this kind of thing, the annotate feature of svn
is your best tool:
http://svn.eu.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?annotate=1138627
The lines you're unhappy about were introduced in r771587:
http://svn.eu.apache.org/viewvc?view=revisionrevision=771587
which has quite an informative commit message.  Maybe that might help
to figure out the purpose?


Thank you for the hints. I will take a closer look on the diff once time 
permits. I should have done that before...


Cheers,
Micha

[PATCH 51489] ProxyPassReverse issue + patch

[Micha Lenk]

Hi Apache developers,

I'm using Apache as a reverse proxy in a simple load balancer setup.
I use ProxyPassReverse in order to re-write the backend server name in 
HTTP redirections (ie. in the Location header of the HTTP response).

My configuration for the virtual host essentially looks like this:

Proxy balancer://196f045aca6adc82a0b6eea93ed286a1
BalancerMember http://server-1.local status=-SE
BalancerMember http://server-2.local status=-SE
/Proxy
VirtualHost 10.8.16.33:80
ServerName frontend.local

Location /
ProxyPass balancer://196f045aca6adc82a0b6eea93ed286a1/
ProxyPassReverse balancer://196f045aca6adc82a0b6eea93ed286a1/
/Location
/VirtualHost

Now, I was wondering why redirects get an additional slash between the 
server name and the path. Example:


1. The backend server redirects the URL http://server-1.local/foo to
   the URL http://server-1.local/foo/ because this is actually a
   directory (so far no issue)

2. With the configuration above the reverse proxy redirects the URL
   http://frontend.local/foo to http://frontend.local//foo/

What I bother about is the additional slash before '/foo/', so I digged 
into the source code and found the following lines in 
modules/proxy/proxy_util.c:


PROXY_DECLARE(const char *) ap_proxy_location_reverse_map(request_rec *r,
  proxy_dir_conf *conf, const char *url)
{
[...]
l1 = strlen(url);
[...]
for (i = 0; i  conf-raliases-nelts; i++) {
if (ap_proxy_valid_balancer_name((char *)real, 0) 
(balancer = ap_proxy_get_balancer(r-pool, sconf, real))) {
int n, l3 = 0;
proxy_worker **worker = (proxy_worker 
**)balancer-workers-elts;

const char *urlpart = ap_strchr_c(real, '/');
if (urlpart) {
if (!urlpart[1])
urlpart = NULL;
else
l3 = strlen(urlpart);
}
for (n = 0; n  balancer-workers-nelts; n++) {
l2 = strlen((*worker)-s-name);
if (urlpart) {
/* urlpart (l3) assuredly starts with its own '/' */
if ((*worker)-s-name[l2 - 1] == '/')
--l2;
if (l1 = l2 + l3
 strncasecmp((*worker)-s-name, url, l2) 
== 0

 strncmp(urlpart, url + l2, l3) == 0) {
u = apr_pstrcat(r-pool, ent[i].fake, url[l2 + 
l3],

NULL);
return ap_construct_url(r-pool, u, r);
}
}
else if (l1 = l2  strncasecmp((*worker)-s-name, 
url, l2) == 0) {

u = apr_pstrcat(r-pool, ent[i].fake, url[l2], NULL);
return ap_construct_url(r-pool, u, r);
}
worker++;
}
[...]

Right now I don't really understand the reason for the special casing of 
urlpart == / in modules/proxy/proxy_util.c, lines 1126 to 1129 (SVN 
rev. 1144374). If urlpart == /, then the code in lines 1151 and 1152 
gets executed, which seems to add the slash.


I tried to remove the special casing (see submitted patch), and 
apparently the removal fixes the issue.


Does anybody know the reason for the special casing mentioned above?
If not I want to suggest to commit my patch.

Regards,
Micha