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

[Graham Leggett]

Hi all,

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

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

Regards,
Graham
--

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

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

[Graham Leggett]

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

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

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

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

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

Regards,
Graham
—

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

[Graham Leggett]

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

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

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

Regards,
Graham
—

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

[Graham Leggett]

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

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

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

Regards,
Graham
—



httpd-register-poll2.patch
Description: Binary data

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

[Yann Ylavic]

On Mon, Mar 14, 2016 at 8:51 PM, Graham Leggett  wrote:
> On 14 Mar 2016, at 10:32 AM, Yann Ylavic  wrote:
>
>> Since apr_pollfd_t is not opaque (unlike apr_socket_t), maybe we could
>> remove the indirection here (and in the code below) with somthing like
>> (apr_pollfd_t *pfds, size_t npfds, ...).
>> That would allow a single allocation (all pfds in once) and possibly
>> make things easier for the caller.
>
> Having looked at this in more detail this isn’t as simple.
>
> The sticking point is the cleanup, which needs to be passed a single struct 
> to do it’s work. To pass both the pfds and the npdfs these two need to be 
> wrapped in a structure of some kind, which the user has to feed in from the 
> outside common to both register and unregister. This structure probably 
> should be an apr_array_header_t.

Hmm right, either this or, say, the ending pfd is the one with
->reqevents=0, or ->p=NULL, or probably better
->desc_type=APR_NO_DESC.
Up to the caller to ensure this, it could possibly be documented.

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

That would still require some "complex" initialization, so I'd prefer
the above if it doesn't sound to hacky...

Regards,
Yann.

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

[Graham Leggett]

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

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

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

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

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

Regards,
Graham
—

Re: apache-swat project - need feedback!

[Alexey Melezhik]

I have just update some docs on swatpm.org to help beginners start
with swat on more gentle way:

- http://swatpm.org/ - hello world example to start with
- http://swatpm.org/faq - FAQ page with answers to possible common
questions about swat

Regards

PS anyway, if anybody succeeded in starting writing tests with swat?
Please let me know  Thanks


2016-02-04 14:07 GMT+03:00 Alexey Melezhik :
> Hi!
>
> Recently I have started apache-swat project
> https://github.com/melezhik/apache-swat , thanks to Bill
> for supporting my first attempt -
> http://mail-archives.apache.org/mod_mbox/httpd-dev/201601.mbox/%3CCACsi253b6=vufvyrw3j0ixyhmgruzlyy1+ufrenaxfkd+re...@mail.gmail.com%3E
>
> As being newbie in apache project I'd be glad to get a feedback from
> apache developers
>
> * is it clear what apache-swat is?
> * is it useful / helpful ?
> * what things could be improved
>
> About apache-swat in few words.
>
> * apache-swat is test suites base to be run on local developer's environment
> * test suites is on per issues basis, issues taken by me ( in almost )
> random  ) mode from https://bz.apache.org/bugzilla/ and get converted
> into test scenarios
> * everyone is encourage create new tests for existed or new issues
> * apache-swat  tries to be as irrelevant as possible to your local
> environment so allow tests suite installation / configuration process
> with minimal efforts , a minimal conventions on your local environment
> are:
>
>  - you have perl  ( almost any version )
>  - you have cpan or cpanm ( to install swat cpan module )
>  - you have git to checkout tests suite from
> https://github.com/melezhik/apache-swat
> -  you have vanilla apache installed according
> https://httpd.apache.org/docs/2.2/en/install.html or
> https://httpd.apache.org/docs/2.4/en/install.html
>  - and then you follow a simple instruction to bootstrap apache-swat tests 
> suite
>
> * tests sites could be run in whole chunk ( all tests ), just say:
>
>   $ swat
>
> * or you may run a certain test of your interest , mostly this is per
> issue tests, for example:
>
>  $ swat  -t 58854 # run tests for issue
> https://bz.apache.org/bugzilla/show_bug.cgi?id=58854
>
>
> Contributing to apache-swat:
>
> * git fork
> * write new tests for issue - t foo-bar-baz
> * make github MR
>
>
> Existed issues list covered by apache-swat
>
> 44221
> 46751
> 58777
> 58789
> 58854
>
> I have posted an information about apache-swat tests suite existence
> to  proper Bugzilla issue pages.
>
>
> Thanks in advance
>
>
> Alexey Melezhik

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

[Jeff Trawick]

Feedback requested below on a couple of issues...

On Mon, Mar 14, 2016 at 11:42 AM,  wrote:

> Author: trawick
> Date: Mon Mar 14 15:42:45 2016
> New Revision: 1734947
>
> URL: http://svn.apache.org/viewvc?rev=1734947=rev
> Log:
> Add CGIVar directive for configuring REQUEST_URI behavior
>
> The goal is to use this one directive to handle any configurable
> CGI variable behavior; only one CGI variable is supported initially.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/manual/mod/core.xml
> httpd/httpd/trunk/include/http_core.h
> httpd/httpd/trunk/server/core.c
> httpd/httpd/trunk/server/util_script.c
>
> ...


>
> Modified: httpd/httpd/trunk/include/http_core.h
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1734947=1734946=1734947=diff
>
> ==
> --- httpd/httpd/trunk/include/http_core.h (original)
> +++ httpd/httpd/trunk/include/http_core.h Mon Mar 14 15:42:45 2016
> @@ -673,6 +673,9 @@ typedef struct {
>
>  unsigned int qualify_redirect_url :2;
>  ap_expr_info_t  *expr_handler; /* forced with SetHandler */
> +
> +/** Table of rules for building CGI variables, NULL if none
> configured */
> +apr_hash_t *cgi_var_rules;
>

Any concerns with forcing the module/core/whatever to have to check if the
table has been created before seeing if it has a rule for a particular
variable?


>  } core_dir_config;
>
>  /* macro to implement off by default behaviour */
>
> Modified: httpd/httpd/trunk/server/core.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1734947=1734946=1734947=diff
>
> ==
> --- httpd/httpd/trunk/server/core.c (original)
> +++ httpd/httpd/trunk/server/core.c Mon Mar 14 15:42:45 2016
> @@ -420,6 +420,15 @@ static void *merge_core_dir_configs(apr_
>
>  conf->cgi_pass_auth = new->cgi_pass_auth != AP_CGI_PASS_AUTH_UNSET ?
> new->cgi_pass_auth : base->cgi_pass_auth;
>
> +if (new->cgi_var_rules) {
> +if (!conf->cgi_var_rules) {
> +conf->cgi_var_rules = new->cgi_var_rules;
> +}
> +else {
> +conf->cgi_var_rules = apr_hash_overlay(a, new->cgi_var_rules,
> conf->cgi_var_rules);
> +}
> +}
> +
>  AP_CORE_MERGE_FLAG(qualify_redirect_url, conf, base, new);
>
>  return (void*)conf;
> @@ -1872,6 +1881,31 @@ static const char *set_cgi_pass_auth(cmd
>  return NULL;
>  }
>
> +static const char *set_cgi_var(cmd_parms *cmd, void *d_,
> +   const char *var, const char *rule_)
> +{
> +core_dir_config *d = d_;
> +char *rule = apr_pstrdup(cmd->pool, rule_);
> +
> +ap_str_tolower(rule);
> +
> +if (!strcmp(var, "REQUEST_URI")) {
> +if (strcmp(rule, "current-uri") && strcmp(rule, "original-uri")) {
> +return "Valid rules for REQUEST_URI are 'current-uri' and
> 'original-uri'";
> +}
> +}
> +else {
> +return apr_pstrcat(cmd->pool, "Unrecognized CGI variable: \"",
> +   var, "\"", NULL);
>

I can see letting third-party modules use this directive for their own
purposes, which requires ignoring variables/rules we don't know about and
simply storing them in the table.  For the time being this refuses any
unexpected variables/rules; that could be loosened up later if desired.
But the possibility has some effect on the choice of rule representation --
flexible character string (current) vs. some enumerated value (more
efficient).  (Aside from letting third-party modules play without custom
code in httpd, it also simplifies the httpd implementation to pass through
anything.  That's what happens essentially with "SetEnvIf Request_URI .
proxy-fcgi-pathinfo".)

Any thoughts?



> +}
> +
> +if (!d->cgi_var_rules) {
> +d->cgi_var_rules = apr_hash_make(cmd->pool);
> +}
> +apr_hash_set(d->cgi_var_rules, var, APR_HASH_KEY_STRING, rule);
> +return NULL;
> +}
> +
>  static const char *set_qualify_redirect_url(cmd_parms *cmd, void *d_, int
> flag)
>  {
>  core_dir_config *d = d_;
> @@ -4565,6 +4599,8 @@ AP_INIT_TAKE12("LimitInternalRecursion",
>  AP_INIT_FLAG("CGIPassAuth", set_cgi_pass_auth, NULL, OR_AUTHCFG,
>   "Controls whether HTTP authorization headers, normally
> hidden, will "
>   "be passed to scripts"),
> +AP_INIT_TAKE2("CGIVar", set_cgi_var, NULL, OR_FILEINFO,
> +  "Controls how some CGI variables are set"),
>  AP_INIT_FLAG("QualifyRedirectURL", set_qualify_redirect_url, NULL,
> OR_FILEINFO,
>   "Controls whether the REDIRECT_URL environment variable is
> fully "
>   "qualified"),
>
> Modified: httpd/httpd/trunk/server/util_script.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_script.c?rev=1734947=1734946=1734947=diff
>
> 

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

[Jim Jagielski]

They are also proxy-timeout related it seems:

# testing : QUERY_STRING passed OK
# expected: qr/QUERY_STRING = horse=trigger\n/s
# received: '
# 
# 504 Proxy Error
# 
# Proxy Error
# The gateway did not receive a timely response
# from the upstream server or application.
# '
not ok 27
# Failed test 27 in t/modules/rewrite.t at line 85

> On Mar 14, 2016, at 10:03 AM, Ruediger Pluem  wrote:
> 
> 
> 
> On 03/14/2016 02:55 PM, Jim Jagielski wrote:
>> 
>>> On Mar 14, 2016, at 4:48 AM, Ruediger Pluem  wrote:
>>> 
>>> 
>>> This seems to cause frequent (no always) failures with test 8 of 
>>> t/ssl/proxy.t.
>>> The request times out with a 504 status. So it looks like the "backend" in 
>>> this request does not respond.
>>> Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
>>> closeset guess would be that the changes to
>>> checkpipeline cause this.
>>> 
>>> 
>> 
>> Getting similar behavior on OSX as well. Note sure if you are also
>> seeing occasional errors here as well:
>>   t/modules/rewrite.t . 28/29 # Failed test 28 in 
>> t/modules/rewrite.t at line 88
>>   # Failed test 29 in t/modules/rewrite.t at line 90
>> 
>> Also:
>>   t/modules/proxy.t ... 5/18 # Failed test 5 in 
>> t/modules/proxy.t at line 28
>>   # Failed test 6 in t/modules/proxy.t at line 29
>> 
>> 
>> 
>> 
> 
> I have not noticed these here.
> 
> Regards
> 
> Rüdiger
> 

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

[Ruediger Pluem]

On 03/14/2016 02:55 PM, Jim Jagielski wrote:
> 
>> On Mar 14, 2016, at 4:48 AM, Ruediger Pluem  wrote:
>>
>>
>> This seems to cause frequent (no always) failures with test 8 of 
>> t/ssl/proxy.t.
>> The request times out with a 504 status. So it looks like the "backend" in 
>> this request does not respond.
>> Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
>> closeset guess would be that the changes to
>> checkpipeline cause this.
>>
>>
> 
> Getting similar behavior on OSX as well. Note sure if you are also
> seeing occasional errors here as well:
>t/modules/rewrite.t . 28/29 # Failed test 28 in 
> t/modules/rewrite.t at line 88
># Failed test 29 in t/modules/rewrite.t at line 90
> 
> Also:
>t/modules/proxy.t ... 5/18 # Failed test 5 in 
> t/modules/proxy.t at line 28
># Failed test 6 in t/modules/proxy.t at line 29
> 
> 
> 
> 

I have not noticed these here.

Regards

Rüdiger

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

[Jim Jagielski]

> On Mar 14, 2016, at 7:07 AM, Graham Leggett  wrote:
> 
> On 14 Mar 2016, at 10:32 AM, Yann Ylavic  wrote:
> 
>> Since apr_pollfd_t is not opaque (unlike apr_socket_t), maybe we could
>> remove the indirection here (and in the code below) with somthing like
>> (apr_pollfd_t *pfds, size_t npfds, ...).
>> That would allow a single allocation (all pfds in once) and possibly
>> make things easier for the caller.
> 
> This definitely makes sense.
> 
> I originally wondered whether we could pass an apr_array_header_t into it, 
> but it felt like overkill. Doing it your way means that we could use an array 
> if we wanted to, or we could just pass structures on the stack, which would 
> be much more flexible.
> 

+1 for stack approach.

Also, +1 on the whole concept :)

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

[Jim Jagielski]

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

Getting similar behavior on OSX as well. Note sure if you are also
seeing occasional errors here as well:
   t/modules/rewrite.t . 28/29 # Failed test 28 in 
t/modules/rewrite.t at line 88
   # Failed test 29 in t/modules/rewrite.t at line 90

Also:
   t/modules/proxy.t ... 5/18 # Failed test 5 in 
t/modules/proxy.t at line 28
   # Failed test 6 in t/modules/proxy.t at line 29

Re: HTTP/2 in massive file transfers

[Stefan Eissing]

Just to give some numbers from my laptop, testing against localhost:

httpd mpm worker, OS X, client h2load, transfer rates in GByte/s

Version  #requests2.4.18  2.4.x
1 GB resource
h2c clear   1*1 0.017   1.0
8*1 2.2
h2  tls 1*1 0.430   0.425
8*1 0.75
10 MB resource   
h2c clear  1*1001.0 0.94
   8*1002.4 2.4
h2  tls1*1000.425   0.418
   8*1000.8 0.8

There is a clear bug in 2.4.18 for the cleartext case where client/server are 
stalling and 0% CPU is used. With TLS or with smaller resources, this is not 
observable. This has been fixed in the upcoming 2.4.19 release.

-Stefan


> Am 11.03.2016 um 12:40 schrieb Molina :
> 
> Hello again,
> 
> I installed the newest version now (2.4.18), but I still get the same results.
> 
> Answering by parts:
> 1. I do not use SSL
> 2. I have tried now with h2o and nghttpd. Obtained results are explained below
> 3. Increasing or decreasing the number of concurrent streams does not 
> significantly modify the transfer time as long as it remains in a decent 
> number of files (say, minimum 100)
> 4. Varying the size of each stream did not significantly modify the transfer 
> time
> 
> I will now explain the results I’ve observed when trying with other 
> server/clients:
> 
> nghttpd
> I started to suspect this issue is related to the window size, so I run 
> nghttp with a window size of 2^16 (64KB, the default one). I got the 
> following sample output in the client when downloading a 1GB-file:
> 
> [ 12.087] recv DATA frame 

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

[Graham Leggett]

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

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

This definitely makes sense.

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

Regards,
Graham
—

Re: [Patch] mod_tcp / mod_proxy_tcp / mod_ssl_tcp

[Graham Leggett]

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

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

Hmmm - good point.

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

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

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

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

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

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

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

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

Regards,
Graham
—

Re: r1619483

[Yann Ylavic]

Hi Christophe,

On Sun, Mar 13, 2016 at 10:21 PM, Christophe JAILLET
 wrote:
> Hi,
>
> while looking at potential backport to synch and trunk, I arrived on
> r1619483.
>
> This is a follow up of r1619444 which is a follow up of r1619383.
> These 2 have been bakcported (see r1669555)
>
> So I was wondering if r1619483 should also have been backported at the same
> time?

It's been a long time, but it seems that indeed this follow up did not
make it in r1669555 :/

Recollecting, this commit uses the correct way to account for inflated
bytes should there be data already available before the inflate() call
(otherwise some bytes may be counted twice).

I think we should merge it now, better late than never :)

Regards,
Yann.

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

[Stefan Eissing]

> Am 14.03.2016 um 09:32 schrieb Yann Ylavic :
> 
> On Mon, Mar 14, 2016 at 12:41 AM, Graham Leggett  wrote:
>> On 13 Mar 2016, at 10:55 PM, Eric Covener  wrote:
>> 
>>> I also meant the original feature never made it, so we can whatever we
>>> want to it.
>> 
>> What do you think of this?
> 
> Looks good and indeed very valuable to me, s/socket/pollfd/ is a great idea.

Like the direction this is going as well.

Do we need a MPM Query for detecting support before one actually has a handle
for registration?

> Maybe a little nit-picking below...
> 
>> 
>> Index: include/ap_mpm.h
>> ===
>> --- include/ap_mpm.h(revision 1734657)
>> +++ include/ap_mpm.h(working copy)
>> @@ -207,50 +207,48 @@
>>void *baton);
>> 
>> /**
>> - * Register a callback on the readability or writability on a group of 
>> sockets
>> - * @param s Null-terminated list of sockets
>> + * Register a callback on the readability or writability on a group of
>> + * sockets/pipes.
>> + * @param pds Null-terminated list of apr_pollfd_t
>>  * @param p pool for use between registration and callback
>> - * @param for_read Whether the sockets are monitored for read or writability
>>  * @param cbfn The callback function
>>  * @param baton userdata for the callback function
>> - * @return APR_SUCCESS if all sockets could be added to a pollset,
>> + * @return APR_SUCCESS if all sockets/pipes could be added to a pollset,
>>  * APR_ENOTIMPL if no asynch support, or an apr_pollset_add error.
>> - * @remark When activity is found on any 1 socket in the list, all are 
>> removed
>> + * @remark When activity is found on any 1 socket/pipe in the list, all are 
>> removed
>>  * from the pollset and only 1 callback is issued.
>>  */
>> 
>> -AP_DECLARE(apr_status_t) ap_mpm_register_socket_callback(apr_socket_t **s,
>> - apr_pool_t *p,
>> - int for_read,
>> - 
>> ap_mpm_callback_fn_t *cbfn,
>> - void *baton);
>> +AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(apr_pollfd_t **pds,
>> +   apr_pool_t *p,
>> +   ap_mpm_callback_fn_t 
>> *cbfn,
>> +   void *baton);
> 
> Since apr_pollfd_t is not opaque (unlike apr_socket_t), maybe we could
> remove the indirection here (and in the code below) with somthing like
> (apr_pollfd_t *pfds, size_t npfds, ...).
> That would allow a single allocation (all pfds in once) and possibly
> make things easier for the caller.
> 
> Regards,
> Yann.

Re: [Patch] mod_tcp / mod_proxy_tcp / mod_ssl_tcp

[Yann Ylavic]

On Sat, Mar 12, 2016 at 4:46 PM, Graham Leggett  wrote:
>
> The following patch provides support for TCP proxying to httpd.
>
> It consists of the following three parts:
>
> - mod_tcp: Allows the frontend to receive pure TCP connections

It looks like this module is only needed to remove HTTP filters from the chain.
Is the goal to have this core module instead of mod_http and make the
latter dynamic?

> - mod_proxy_tcp: Allows the proxy to make pure tcp or tls connections to a 
> backend

Thanks, this will be very useful.

> - mod_ssl_tcp: Allows the proxy to route incoming connections based on the 
> SNI header (tlsext)

Hmm, isn't mod_ssl (underlying-)protocol agnostic?
Why couldn't it be used as-is (or adapted), and avoid code duplication?

>
> In the following example config, incoming TCP connections are routed based on 
> their SNI (the tlsext protocol) to given backend servers, which then complete 
> the SSL connections as raw tunnels.
>
> This allows you to use client certificates through the httpd proxy balancer 
> all the way to the backend server without the proxy terminating any SSL along 
> the way.
>
> 
>   Protocol tlsext

Maybe "tcps"? I agree that SNI extension is needed, but "tlsext" looks
confusing.

I'll look at the patch in more details, I may have missed things...

Regards,
Yann.

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

[Ruediger Pluem]

On 03/12/2016 01:43 AM, minf...@apache.org wrote:
> Author: minfrin
> Date: Sat Mar 12 00:43:58 2016
> New Revision: 1734656
> 
> URL: http://svn.apache.org/viewvc?rev=1734656=rev
> Log:
> core: Extend support for setting aside data from the network input filter
> to any connection or request input filter.
> 
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/httpd.h
> httpd/httpd/trunk/include/mpm_common.h
> httpd/httpd/trunk/include/util_filter.h
> httpd/httpd/trunk/modules/http/http_core.c
> httpd/httpd/trunk/modules/http/http_request.c
> httpd/httpd/trunk/server/core.c
> httpd/httpd/trunk/server/core_filters.c
> httpd/httpd/trunk/server/mpm/event/event.c
> httpd/httpd/trunk/server/mpm/motorz/motorz.c
> httpd/httpd/trunk/server/mpm/simple/simple_io.c
> httpd/httpd/trunk/server/mpm_common.c
> httpd/httpd/trunk/server/util_filter.c
> 


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


Regards

Rüdiger

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

[Yann Ylavic]

On Mon, Mar 14, 2016 at 12:41 AM, Graham Leggett  wrote:
> On 13 Mar 2016, at 10:55 PM, Eric Covener  wrote:
>
>> I also meant the original feature never made it, so we can whatever we
>> want to it.
>
> What do you think of this?

Looks good and indeed very valuable to me, s/socket/pollfd/ is a great idea.

Maybe a little nit-picking below...

>
> Index: include/ap_mpm.h
> ===
> --- include/ap_mpm.h(revision 1734657)
> +++ include/ap_mpm.h(working copy)
> @@ -207,50 +207,48 @@
> void *baton);
>
>  /**
> - * Register a callback on the readability or writability on a group of 
> sockets
> - * @param s Null-terminated list of sockets
> + * Register a callback on the readability or writability on a group of
> + * sockets/pipes.
> + * @param pds Null-terminated list of apr_pollfd_t
>   * @param p pool for use between registration and callback
> - * @param for_read Whether the sockets are monitored for read or writability
>   * @param cbfn The callback function
>   * @param baton userdata for the callback function
> - * @return APR_SUCCESS if all sockets could be added to a pollset,
> + * @return APR_SUCCESS if all sockets/pipes could be added to a pollset,
>   * APR_ENOTIMPL if no asynch support, or an apr_pollset_add error.
> - * @remark When activity is found on any 1 socket in the list, all are 
> removed
> + * @remark When activity is found on any 1 socket/pipe in the list, all are 
> removed
>   * from the pollset and only 1 callback is issued.
>   */
>
> -AP_DECLARE(apr_status_t) ap_mpm_register_socket_callback(apr_socket_t **s,
> - apr_pool_t *p,
> - int for_read,
> - 
> ap_mpm_callback_fn_t *cbfn,
> - void *baton);
> +AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(apr_pollfd_t **pds,
> +   apr_pool_t *p,
> +   ap_mpm_callback_fn_t 
> *cbfn,
> +   void *baton);

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

Regards,
Yann.