Re: svn commit: r1748461 - in /httpd/httpd/branches/2.2.x: ./ CHANGES support/ab.c

2016-06-14 Thread William A Rowe Jr
Two quick observations...

Every time OpenSSL was compiled against a different CRT, whether is was a
release build of OpenSSL and a debug build of httpd/ab, or whether OpenSSL
was built under Visual Studio 2012 and httpd/ab was built against VS 2015,
this issue would occur without this patch.

Visual Studio 2015 only magnified or brought this quirk to light.

I'm curious, having seen reports of a Windows specific defect in local CRL
validation, whether this is also required in httpd.

Sadly, OpenSSL only looks for this symbol in the currently executing binary
(recall Win32 is two-level namespace)... we bind libssl/libcrypto to
libaprutil.dll and mod_ssl.so.  Exporting this from httpd.exe isn't
practical.

If that is the root of the other CRL failure observation, I'm not sure that
OpenSSL's hack is going to resolve it for our custom BIO implementation.

Gregg, if you would vet the patch as applied to trunk/2.4/2.2, I'd
appreciate it.
On Jun 14, 2016 3:37 PM,  wrote:

Author: wrowe
Date: Tue Jun 14 20:37:52 2016
New Revision: 1748461

URL: http://svn.apache.org/viewvc?rev=1748461=rev
Log:
abs: Include OPENSSL_Applink when compiling on Windows, to resolve
failures under Visual Studio 2015 and other mismatched MSVCRT flavors.

PR: 59630
Submitted by: Jan Ehrhardt 


Modified:
httpd/httpd/branches/2.2.x/   (props changed)
httpd/httpd/branches/2.2.x/CHANGES
httpd/httpd/branches/2.2.x/support/ab.c

Propchange: httpd/httpd/branches/2.2.x/
--
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Jun 14 20:37:52 2016
@@ -1,2 +1,2 @@
 
/httpd/httpd/branches/2.4.x:138,159,1648845,1649003,1681034,1682929,1682939,1707123,1722573,1726087
-/httpd/httpd/trunk:290940,395552,417988,451572,501364,583817,583830,611483,630858,639005,639010,647395,657354,657459,660461,660566,664330,678761,680082,681190,682369,683626,685112,686805,686809,687099,687754,693120,693392,693727-693728,696006,697093,706318,707163,708902,711421,713575,719357,720250,729316-729317,729586,732414,732504,732816,732832,733127,733134,733218-733219,734710,743589,755190,756671,756675,756678,756683,757741,761329,763394,764239,768535,769809,771587,771610,776325,777042,777091,778438-778439,778531,778942,780648,780655,780692,780697,780699,785457,785661,790587,803704,819480,823536,823563,834378,835046,891282,900022,932791,942209,952823,953311,955966,979120,981084,992625,1026743,1031551,1040304,1040373,1058192,1070096,1082189,1082196,1090645,1172732,1200040,1200372,1200374,1213380,1222335,1223048,1231446,1244211,1294306,1299738,1300171,130,1308862,1327036,1327080,1328133,1328325-1328326,1345319,1348656,1349905,1352912,1363183,1363186,1366344,1367778,1368131,136
 
8396,1369568,1395225,1398066,1400700,1408402,1410681,1413732,1414094,1416889,1418752,1422234,1422253,1435178,1447426,1470940,1475878,1476604,1476621,1476642,1476644-1476645,1477530,1484852,1485409,1485668,1490994,1493330,1496429,1500323,1504276,1506714,1509872,1509875,1514215,1524192,1524770,1526168,1526189,1527291,1527295,1527925,1528718,1529559,1529988,1529991,1531505,1532816,1551685,1551714,1552227,1553204,1554276,1554281,1555240,155,1556428,1563420,1572092,1572198,1572543,1572611,1572630,1572655,1572663,1572668-1572671,1572896,1572911,1572967,1573224,1573229,1575400,1585090,1586745,1587594,1587639,1588851,1590509,1603156,1604353,1610207,1610311,1610491,1610501,1611165,1611169,1620932,1621453,1643537,1643543,1648840,1649001,1649043,1650310,1650320,1652929,1653997,1657897,1658765,1663647,1664205,1665215,1665218,1665625,1665721,1666363,1674056,1675533,1676654,1677462,1679182,1679470,1680895,1680900,1680942,1681037,1682923,1682937,1684513,1685345,1685347,1685349-1685350,1688274,
 1688536,1688538,1706989,1722572,1726086
+/httpd/httpd/trunk:290940,395552,417988,451572,501364,583817,583830,611483,630858,639005,639010,647395,657354,657459,660461,660566,664330,678761,680082,681190,682369,683626,685112,686805,686809,687099,687754,693120,693392,693727-693728,696006,697093,706318,707163,708902,711421,713575,719357,720250,729316-729317,729586,732414,732504,732816,732832,733127,733134,733218-733219,734710,743589,755190,756671,756675,756678,756683,757741,761329,763394,764239,768535,769809,771587,771610,776325,777042,777091,778438-778439,778531,778942,780648,780655,780692,780697,780699,785457,785661,790587,803704,819480,823536,823563,834378,835046,891282,900022,932791,942209,952823,953311,955966,979120,981084,992625,1026743,1031551,1040304,1040373,1058192,1070096,1082189,1082196,1090645,1172732,1200040,1200372,1200374,1213380,1222335,1223048,1231446,1244211,1294306,1299738,1300171,130,1308862,1327036,1327080,1328133,1328325-1328326,1345319,1348656,1349905,1352912,1363183,1363186,1366344,1367778,1368131,136
 

Re: h2_proxy_util.c, is this going make 2.4.21?

2016-06-13 Thread William A Rowe Jr
On Mon, Jun 13, 2016 at 1:59 PM, Gregg Smith  wrote:

> I have the to connect this module in the traditional windows build but as
> of right now it's using h2_util.c which Bill had an objection to. See his
> comments http://marc.info/?l=apache-httpd-dev=146543811201820=2
>
> So to me that seems to be a -1 to mod_proxy_http2, at minimum on Windows.
> In trunk Stefan seems to have chosen option 3 in Bill's list and that is
> h2_proxy_util.c.
>
> If I knew that was going to be backported I would add the rest of the bits
> needed to use h2_proxy_util.c. If it is not going to make it, then I will
> not commit anything and there will be no mod_proxy_http2 in 2.4.21 on
> Windows. If this gets in overnight and you tag in the morning, I may not be
> out of bed yet due to the time difference.
>
> That's where my concern is. Make sense?


Shouldn't be a concern. I'm mildly concerned about the single-level
namespace
collisions on Unix, but because the .so object is pre-linked to its own
functions
before anything is imported/exported, mod_http2.so should be using
h2_utils.o
and mod_proxy_http2.so should be using h2_proxy_utils.o, even without any
additional namespace protection. A third module trying to use the functions
of
those two modules could cause headaches, but that can be addressed later.

Windows has two-level namespaces, so there is no ambiguity between symbols
in one .so (.dll) and a second, unless you are simultaneously linking a
module
to both of these modules.

I accept Stefan's proposed fix for the time being, and we can certainly make
this simpler on trunk in the future.

Cheers,

Bill


Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)

2016-06-09 Thread William A Rowe Jr
On Thu, Jun 9, 2016 at 3:52 PM, Michael Kaufmann 
wrote:

>
> Note that "Suppress 'h2' announcements in Upgrade: header" has been
> inserted at the top of the STATUS file; it should probably be moved to the
> bottom of the 'Patches proposed' section.
>

Yea, that would be a problem, it will get no review, since we process our
status
reviews top down, paying extra attention to the LIFO queue (things we had
not
yet seen).  It says so point blank two lines above.

The submitter should fix their submission for maximum feedback.


T 2.2.32 next week...

2016-06-09 Thread William A Rowe Jr
On Tue, Sep 1, 2015 at 2:31 AM,  wrote:

> Author: rpluem
> Date: Tue Sep  1 07:31:11 2015
> New Revision: 1700408
>
> URL: http://svn.apache.org/r1700408
> Log:
> * Fix a regression with 2.2.31 that caused inherited workers to
>   use a different scoreboard slot then the original one.
>   This has no trunk revision since this a 2.2.x issue only and trunk
>   code is different.
>
> PR: 58267
>

Since it has been 11 mos since we shipped a 2.2.x candidate, and the
regression in 2.2.31 suggests we aught to share that fix, I'm prepared
to tag and roll one of our last 2.2.x releases next week, alongside the
2.4.x release Jim offers to T  (And certainly push out our message
of our EOL concensus with both release announcements, to get our
slowest moving users toward 2.4.x with all due haste.)

There is a group of other backports out there looking for review (and
one with commentary that didn't seem to be addressed by an updated
patch).  If folks want to review and backport some of these bigger
defect fixes, that would be great.  But the list is rather short, so it
should take minimal effort to get it ready for T

Cheers,

Bill


h2_utils duplicity

2016-06-08 Thread William A Rowe Jr
On Wed, Jun 8, 2016 at 7:19 PM,  wrote:

> Author: wrowe
> Date: Thu Jun  9 00:19:24 2016
> New Revision: 1747470
>
> URL: http://svn.apache.org/viewvc?rev=1747470=rev
> Log:
> The answer to the question appears to be in 2.4.21, drop h2_casecmpstr fork
>
> Modified:
> httpd/httpd/trunk/modules/http2/NWGNUmod_http2
> httpd/httpd/trunk/modules/http2/h2_util.c
> httpd/httpd/trunk/modules/http2/h2_util.h
> httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
>
> Modified: httpd/httpd/trunk/modules/http2/h2_util.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_util.c?rev=1747470=1747469=1747470=diff
>
> ==
> --- httpd/httpd/trunk/modules/http2/h2_util.c (original)
> +++ httpd/httpd/trunk/modules/http2/h2_util.c Thu Jun  9 00:19:24 2016
> @@ -1578,123 +1578,3 @@ void h2_push_policy_determine(struct h2_
>  req->push_policy = policy;
>  }
>
>
> -/***
> - * ap_cstr_casecmp, when will it be backported?
> -
> **/
> -#if !APR_CHARSET_EBCDIC
> -/*
> - * Provide our own known-fast implementation of str[n]casecmp()
> - * NOTE: Only ASCII alpha characters 41-5A are folded to 61-7A,
> - * other 8-bit latin alphabetics are never case-folded!
> - */
> -static const unsigned char ucharmap[] = {
> -0x0,  0x1,  0x2,  0x3,  0x4,  0x5,  0x6,  0x7,
> -0x8,  0x9,  0xa,  0xb,  0xc,  0xd,  0xe,  0xf,
> -0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
> -0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
> -0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27,
> -0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f,
> -0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
> -0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f,
> -0x40,  'a',  'b',  'c',  'd',  'e',  'f',  'g',
> - 'h',  'i',  'j',  'k',  'l',  'm',  'n',  'o',
> - 'p',  'q',  'r',  's',  't',  'u',  'v',  'w',
> - 'x',  'y',  'z', 0x5b, 0x5c, 0x5d, 0x5e, 0x5f,
> -0x60,  'a',  'b',  'c',  'd',  'e',  'f',  'g',
> - 'h',  'i',  'j',  'k',  'l',  'm',  'n',  'o',
> - 'p',  'q',  'r',  's',  't',  'u',  'v',  'w',
> - 'x',  'y',  'z', 0x7b, 0x7c, 0x7d, 0x7e, 0x7f,
> -0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87,
> -0x88, 0x89, 0x8a, 0x8b, 0x8c, 0x8d, 0x8e, 0x8f,
> -0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97,
> -0x98, 0x99, 0x9a, 0x9b, 0x9c, 0x9d, 0x9e, 0x9f,
> -0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7,
> -0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf,
> -0xb0, 0xb1, 0xb2, 0xb3, 0xb4, 0xb5, 0xb6, 0xb7,
> -0xb8, 0xb9, 0xba, 0xbb, 0xbc, 0xbd, 0xbe, 0xbf,
> -0xc0, 0xc1, 0xc2, 0xc3, 0xc4, 0xc5, 0xc6, 0xc7,
> -0xc8, 0xc9, 0xca, 0xcb, 0xcc, 0xcd, 0xce, 0xcf,
> -0xd0, 0xd1, 0xd2, 0xd3, 0xd4, 0xd5, 0xd6, 0xd7,
> -0xd8, 0xd9, 0xda, 0xdb, 0xdc, 0xdd, 0xde, 0xdf,
> -0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xe7,
> -0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef,
> -0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,
> -0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff
> -};
> -
> -#else /* APR_CHARSET_EBCDIC */
> -/* Derived from apr-iconv/ccs/cp037.c for EBCDIC case comparison,
> -   provides unique identity of every char value (strict ISO-646
> -   conformance, arbitrary election of an ISO-8859-1 ordering, and
> -   very arbitrary control code assignments into C1 to achieve
> -   identity and a reversible mapping of code points),
> -   then folding the equivalences of ASCII 41-5A into 61-7A,
> -   presenting comparison results in a somewhat ISO/IEC 10646
> -   (ASCII-like) order, depending on the EBCDIC code page in use.
> - */
> -static const unsigned char ucharmap[] = {
> -   0x00, 0x01, 0x02, 0x03, 0x9C, 0x09, 0x86, 0x7F,
> -   0x97, 0x8D, 0x8E, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
> -   0x10, 0x11, 0x12, 0x13, 0x9D, 0x85, 0x08, 0x87,
> -   0x18, 0x19, 0x92, 0x8F, 0x1C, 0x1D, 0x1E, 0x1F,
> -   0x80, 0x81, 0x82, 0x83, 0x84, 0x0A, 0x17, 0x1B,
> -   0x88, 0x89, 0x8A, 0x8B, 0x8C, 0x05, 0x06, 0x07,
> -   0x90, 0x91, 0x16, 0x93, 0x94, 0x95, 0x96, 0x04,
> -   0x98, 0x99, 0x9A, 0x9B, 0x14, 0x15, 0x9E, 0x1A,
> -   0x20, 0xA0, 0xE2, 0xE4, 0xE0, 0xE1, 0xE3, 0xE5,
> -   0xE7, 0xF1, 0xA2, 0x2E, 0x3C, 0x28, 0x2B, 0x7C,
> -   0x26, 0xE9, 0xEA, 0xEB, 0xE8, 0xED, 0xEE, 0xEF,
> -   0xEC, 0xDF, 0x21, 0x24, 0x2A, 0x29, 0x3B, 0xAC,
> -   0x2D, 0x2F, 0xC2, 0xC4, 0xC0, 0xC1, 0xC3, 0xC5,
> -   0xC7, 0xD1, 0xA6, 0x2C, 0x25, 0x5F, 0x3E, 0x3F,
> -   0xF8, 0xC9, 0xCA, 0xCB, 0xC8, 0xCD, 0xCE, 0xCF,
> -   0xCC, 0x60, 0x3A, 0x23, 0x40, 0x27, 0x3D, 0x22,
> -   0xD8, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67,
> -   0x68, 0x69, 0xAB, 0xBB, 0xF0, 0xFD, 0xFE, 0xB1,
> -   0xB0, 0x6A, 0x6B, 0x6C, 0x6D, 0x6E, 0x6F, 0x70,
> -   0x71, 0x72, 0xAA, 0xBA, 

Re: svn commit: r1747469 [1/2] - in /httpd/httpd/trunk: include/ modules/aaa/ modules/cache/ modules/dav/main/ modules/filters/ modules/generators/ modules/http/ modules/http2/ modules/loggers/ module

2016-06-08 Thread William A Rowe Jr
Some of these items are dis-optimized by redundant logic that
we seriously aught to clean up, if anyone is looking for some
low hanging fruit.


On Wed, Jun 8, 2016 at 7:06 PM,  wrote:

> Author: wrowe
> Date: Thu Jun  9 00:06:42 2016
> New Revision: 1747469
>
> URL: http://svn.apache.org/viewvc?rev=1747469=rev
> Log:
> Rename ap_casecmpstr[n]() to ap_cstr_casecmp[n](), update with APR doxygen
>

A few pre-existing examples of this dis-optimization...


> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_storage.c?rev=1747469=1747468=1747469=diff
>
> ==
> --- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
> +++ httpd/httpd/trunk/modules/cache/cache_storage.c Thu Jun  9 00:06:42
> 2016
> @@ -115,7 +115,7 @@ int cache_create_entity(cache_request_re
>
>  static int filter_header_do(void *v, const char *key, const char *val)
>  {
> -if ((*key == 'W' || *key == 'w') && !ap_casecmpstr(key, "Warning")
> +if ((*key == 'W' || *key == 'w') && !ap_cstr_casecmp(key, "Warning")
>  && *val == '1') {
>  /* any stored Warning headers with warn-code 1xx (see section
>   * 14.46) MUST be deleted from the cache entry and the forwarded
> @@ -129,7 +129,7 @@ static int filter_header_do(void *v, con
>  }
>  static int remove_header_do(void *v, const char *key, const char *val)
>  {
> -if ((*key == 'W' || *key == 'w') && !ap_casecmpstr(key, "Warning")) {
> +if ((*key == 'W' || *key == 'w') && !ap_cstr_casecmp(key, "Warning"))
> {
>  /* any stored Warning headers with warn-code 2xx MUST be retained
>   * in the cache entry and the forwarded response.
>   */
>



> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1747469=1747468=1747469=diff
>
> ==
> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Thu Jun  9 00:06:42
> 2016
> @@ -526,7 +526,7 @@ static unsigned is_absolute_uri(char *ur
>  switch (*uri++) {
>  case 'a':
>  case 'A':
> -if (!ap_casecmpstrn(uri, "jp://", 5)) {/* ajp://*/
> +if (!ap_cstr_casecmpn(uri, "jp://", 5)) {/* ajp://*/
>*sqs = 1;
>return 6;
>  }
> @@ -534,7 +534,7 @@ static unsigned is_absolute_uri(char *ur
>
>  case 'b':
>  case 'B':
> -if (!ap_casecmpstrn(uri, "alancer://", 10)) {   /* balancer:// */
> +if (!ap_cstr_casecmpn(uri, "alancer://", 10)) {   /* balancer://
> */
>*sqs = 1;
>return 11;
>  }
> @@ -542,10 +542,10 @@ static unsigned is_absolute_uri(char *ur
>
>  case 'f':
>  case 'F':
> -if (!ap_casecmpstrn(uri, "tp://", 5)) {/* ftp://*/
> +if (!ap_cstr_casecmpn(uri, "tp://", 5)) {/* ftp://*/
>  return 6;
>  }
> -if (!ap_casecmpstrn(uri, "cgi://", 6)) {   /* fcgi://   */
> +if (!ap_cstr_casecmpn(uri, "cgi://", 6)) {   /* fcgi://   */
>  *sqs = 1;
>  return 7;
>  }
> @@ -553,26 +553,26 @@ static unsigned is_absolute_uri(char *ur
>
>  case 'g':
>  case 'G':
> -if (!ap_casecmpstrn(uri, "opher://", 8)) { /* gopher:// */
> +if (!ap_cstr_casecmpn(uri, "opher://", 8)) { /* gopher:// */
>  return 9;
>  }
>  break;
>
>  case 'h':
>  case 'H':
> -if (!ap_casecmpstrn(uri, "ttp://", 6)) {   /* http://   */
> +if (!ap_cstr_casecmpn(uri, "ttp://", 6)) {   /* http://   */
>  *sqs = 1;
>  return 7;
>  }
> -else if (!ap_casecmpstrn(uri, "ttps://", 7)) { /* https://  */
> +else if (!ap_cstr_casecmpn(uri, "ttps://", 7)) { /* https://  */
>  *sqs = 1;
>  return 8;
>  }
> -else if (!ap_casecmpstrn(uri, "2://", 4)) {/* h2:// */
> +else if (!ap_cstr_casecmpn(uri, "2://", 4)) {/* h2:// */
>  *sqs = 1;
>  return 5;
>  }
> -else if (!ap_casecmpstrn(uri, "2c://", 5)) {   /* h2c://*/
> +else if (!ap_cstr_casecmpn(uri, "2c://", 5)) {   /* h2c://*/
>  *sqs = 1;
>  return 6;
>  }
> @@ -580,14 +580,14 @@ static unsigned is_absolute_uri(char *ur
>
>  case 'l':
>  case 'L':
> -if (!ap_casecmpstrn(uri, "dap://", 6)) {   /* ldap://   */
> +if (!ap_cstr_casecmpn(uri, "dap://", 6)) {   /* ldap://   */
>  return 7;
>  }
>  break;
>
>  case 'm':
>  case 'M':
> -if (!ap_casecmpstrn(uri, "ailto:", 6)) {   /* mailto:   */
> +if (!ap_cstr_casecmpn(uri, "ailto:", 6)) {   /* mailto:   */
>  *sqs = 1;
>  return 7;

Re: strncasecmp

2016-06-08 Thread William A Rowe Jr
So, finally looping back to the httpd vs. apr implementations of these
functions...

apr was testing null against equality and both *str1 null and *str2 null
characters - that's obviously wrong since str2 of null. It also performed
final unnecessary increments of str1 and str1, which I've corrected on
apr-2 trunk.  And I'm using a short [] rather than unsigned char []
which solves some sign conversion, presuming most architectures
are able to more efficiently grab an aligned short and promote to int,
as opposed to an unaligned byte (testing backed this up on intel,
and I measure no net benefit when aligning to int instead).

With these changes, apr slightly outperforms httpd's implementation
in most instances. The one small penalty is looking ahead to a null
that turns out to be there, but we should only be scanning one of the
two strings for a null, and the difference between XOR ax, ax vs.
loading both values into ax, cx and subtracting to obtain a zero value
is very nominal, once per string comparison.  The penalty is only
in the case that str1 is equal to str2 for the full null terminated length
and is a net win in all other cases.

Just synch'ed to apr 2.0 trunk, and now will mass-rename so that
the backported functions all correspond to their trunk counterparts
using an ap_cstr_ decorated name corresponding to our namespace,
to apr's function family group ("C"/POSIX string function).

I think we can use that straight back to httpd 2.4 and then worry
about migrating trunk/2.6 consumers to the apr 1.6/2.0 names with
some helpful #define's.

On Mon, Nov 23, 2015 at 11:10 PM, Mikhail T. <mi+t...@aldan.algebra.com>
wrote:

> On 23.11.2015 23:14, William A Rowe Jr wrote:
>
> L1 cache and other direct effects of cpu internal optimization.
>
> Just what I was thinking. Attached is the same program with one more pair
> of functions added (and an easy way to add more "candidates" to the
> main-driver). I changed the FOR-loop define to obtain repeatable results:
>
> # Test 1 -- equal strings:
> foreach m ( 0 1 2 )
> foreach? ./strncasecmp $m 1 a A 7
> foreach? end
> string.h (nb=1, len=7)
> time = 6.975845 : res = 0
> optimized (nb=1, len=7)
> time = 1.492197 : res = 0
> 'A' - 'a' (nb=1, len=7)
> time = 1.787807 : res = 0
>
> # Test 2 -- immediately-different strings
> foreach m ( 0 1 2 )
> foreach? ./strncasecmp $m 1 a x 7
> foreach? end
> string.h (nb=1, len=7)
> time = 2.527727 : res = -23
> optimized (nb=1, len=7)
> time = 0.406867 : res = -23
> 'A' - 'a' (nb=1, len=7)
> time = 0.440320 : res = -23
>
> # Test 3 -- strings different at the very end
> foreach m ( 0 1 2 )
> foreach? ./strncasecmp $m 1 a x 0
> foreach? end
> string.h (nb=1, len=0)
> time = 9.629660 : res = -23
> optimized (nb=1, len=0)
> time = 1.387208 : res = -23
> 'A' - 'a' (nb=1, len=0)
> time = 1.754683 : res = -23
>
> The new pair (method 2) does not use the static table, which is likely to
> benefit from CPU-cache unfairly in repetitive benchmarks.  It is slower
> than the table-using method 1 functions. But the two pairs might be
> comparable -- or even faster -- in real life.
>
> -mi
>
>


Re: svn commit: r1747027 - /httpd/httpd/branches/2.4.x/STATUS

2016-06-08 Thread William A Rowe Jr
Go back through my order of events.  If we conditionally compile in
ap_foo() when apr 1.5 is installed, and provide no stub when apr 1.6 is
installed, every module using ap_foo breaks after the user rebuilds httpd
against 1.6.

We promise our users we won't break the binary compatibility of their third
party modules until we ship httpd 2.6 :(

That was the basis of my veto, it can be fixed.  My newest question is what
we promise users if the compile their 3rd party mod with httpd 2.4 and apr
1.6... should they expect it to run on the same httpd 2.4 with apr 1.5
installed?
On Jun 8, 2016 1:54 PM, "Ruediger Pluem" <rpl...@apache.org> wrote:

>
>
> On 06/08/2016 05:31 PM, William A Rowe Jr wrote:
>
> >
> >   jailletc36: v2: update as per discussion on dev@. Do not
> mix ap_ and apr_
> >   namespaces + tweak DOXYGEN comments.
> > - +1: jailletc36, icing
> > + +1: jailletc36, icing
> > + -0: wrowe [Prefer that we pre-@deprecate this API and
> encourage users
> > +to adopt the apr_ convention, reversing the
> #defines, being
> > +ready for the httpd 2.next minor release.]
> > + -1: wrowe [Notes this patch creates binary incompatible
> versions of httpd
> > +with this change; there must be #else stubs in
> util.c sources
> > +compiled against APR 1.6+ for binary
> compatibility, even if
> > +these are simply;
> > +AP_DECLARE(int) ap_cstr_casecmpn(const char *s1,
> > + const char
> *s2, apr_size_t n)
> > +{
> > +return apr_cstr_casecmpn(s1, s2, n);
> > +}
> > +   ]
> >
> >
> > You cannot have httpd build in such a way that when one exigent
> circumstance
> > changes,  the resulting httpd binary is now a different binary with
> different
> > exported symbols.
> >
> > E.g.
> >
> >   1. Install apr-1.5
> >   2. Build httpd 2.4.recent
> >   3. Build thirdparty mod_foo, referencing ap_cstr_casecmp()
> >   4. Pick up, build and install apr-1.6
> >   5. Pick up httpd 2.4.latest, build and install over 2.4.recent
> >   6. Start httpd.  LoadModule mod_foo fails, ap_cstr_casecmp()
> unresolved
> >   7. httpd fails to start.
> >
> > We have a contract with third party modules that, once built against
> 2.4.x,
> > the will continue to work unmodified when loaded in 2.4.x+1.
> >
> >
> > Here's a counter-question I'd like to raise...
> >
> > Right now we are *not* demanding users pick up apr-1.6, we see no reason
> to force them to during the lifespan of 2.4.x.
> > In httpd 2.6 or 3.0, released after apr 1.6 (or 2.0) has been released,
> that becomes a completely reasonable requirement.
> >
> > If we stub ap_cstr_casecmp() as apr_cstr_casecmp() when apr 1.6 is
> detected (keeping an ap_cstr_casecmp() stub for
> > binary compatibility with modules built against apr 1.5)... do we have
> any concerns that the module compiled against apr
> > 1.6 would then be loaded into an apr 1.5-based build of httpd?
>
> Maybe I am confused now, but I understood that the implementation of
> ap_cstr_casecmp() depends on what apr version the
> httpd binary was compiled against. If the httpd binary is compiled against
> 1.5, then we supply our own implementation in
> httpd, if compiled against 1.6 we call what we have in apr. How does it
> matter which apr version was used to compile the
> modules? Or do you want to check the used apr version during runtime and
> decide based on this?
>
> Regards
>
> Rüdiger
>
>


Re: svn commit: r1747027 - /httpd/httpd/branches/2.4.x/STATUS

2016-06-08 Thread William A Rowe Jr
On Tue, Jun 7, 2016 at 12:15 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> It seems the most critical point of my earlier post was missed...
>
> On Mon, Jun 6, 2016 at 10:08 AM, <wr...@apache.org> wrote:
>
>> Author: wrowe
>> Date: Mon Jun  6 15:08:01 2016
>> New Revision: 1747027
>>
>> URL: http://svn.apache.org/viewvc?rev=1747027=rev
>> Log:
>> Showstopper to patch adoption, along with a more general objection
>>
>> Modified:
>> httpd/httpd/branches/2.4.x/STATUS
>>
>> Modified: httpd/httpd/branches/2.4.x/STATUS
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1747027=1747026=1747027=diff
>>
>> ==
>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>> +++ httpd/httpd/branches/2.4.x/STATUS Mon Jun  6 15:08:01 2016
>> @@ -203,7 +203,20 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>   2.4.x patch:
>> http://home.apache.org/~jailletc36/apr_cstr_casecmp_v2.diff
>
>
Note that this is not the apr_cstr_ implementation, so the entire proposal
seems a little borked to me.  If you are going to borrow the name, why
not the implementation?

I'm going back to the earlier comparisons to benchmark these two against
one another before proposing an alternate patch.

  jailletc36: v2: update as per discussion on dev@. Do not mix ap_ and
>> apr_
>>   namespaces + tweak DOXYGEN comments.
>> - +1: jailletc36, icing
>> + +1: jailletc36, icing
>> + -0: wrowe [Prefer that we pre-@deprecate this API and encourage
>> users
>> +to adopt the apr_ convention, reversing the #defines,
>> being
>> +ready for the httpd 2.next minor release.]
>> + -1: wrowe [Notes this patch creates binary incompatible versions of
>> httpd
>> +with this change; there must be #else stubs in util.c
>> sources
>> +compiled against APR 1.6+ for binary compatibility, even
>> if
>> +these are simply;
>> +AP_DECLARE(int) ap_cstr_casecmpn(const char *s1,
>> + const char *s2,
>> apr_size_t n)
>> +{
>> +return apr_cstr_casecmpn(s1, s2, n);
>> +}
>> +   ]
>>
>
> You cannot have httpd build in such a way that when one exigent
> circumstance
> changes,  the resulting httpd binary is now a different binary with
> different
> exported symbols.
>
> E.g.
>
>   1. Install apr-1.5
>   2. Build httpd 2.4.recent
>   3. Build thirdparty mod_foo, referencing ap_cstr_casecmp()
>   4. Pick up, build and install apr-1.6
>   5. Pick up httpd 2.4.latest, build and install over 2.4.recent
>   6. Start httpd.  LoadModule mod_foo fails, ap_cstr_casecmp() unresolved
>   7. httpd fails to start.
>
> We have a contract with third party modules that, once built against 2.4.x,
> the will continue to work unmodified when loaded in 2.4.x+1.
>

Here's a counter-question I'd like to raise...

Right now we are *not* demanding users pick up apr-1.6, we see no reason to
force them to during the lifespan of 2.4.x.  In httpd 2.6 or 3.0, released
after apr 1.6 (or 2.0) has been released, that becomes a completely
reasonable requirement.

If we stub ap_cstr_casecmp() as apr_cstr_casecmp() when apr 1.6 is detected
(keeping an ap_cstr_casecmp() stub for binary compatibility with modules
built against apr 1.5)... do we have any concerns that the module compiled
against apr 1.6 would then be loaded into an apr 1.5-based build of httpd?

Based on the consensus answer to that question, the final patch is
straightforward.

Bill


Re: svn commit: r1747056 - in /httpd/httpd/branches/2.4.x: ./ STATUS modules/mappers/mod_rewrite.c

2016-06-07 Thread William A Rowe Jr
On Tue, Jun 7, 2016 at 1:23 AM, Marion & Christophe JAILLET <
christophe.jail...@wanadoo.fr> wrote:

> Le 07/06/2016 à 06:59, William A Rowe Jr a écrit :
>
> On Mon, Jun 6, 2016 at 3:10 PM, Marion & Christophe JAILLET <
> <christophe.jail...@wanadoo.fr>christophe.jail...@wanadoo.fr> wrote:
>
>> ap_casecmpstr breaks 2.4.x build.
>> Keep strncasecmp for now? (waiting for ap_cstr_casecmp[n])
>>
>> Le 06/06/2016 à 21:11, j...@apache.org a écrit :
>>
>>> Author: jim
>>> Date: Mon Jun  6 19:11:20 2016
>>> New Revision: 1747056
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1747056=rev
>>> Log:
>>> Merge r1744206 from trunk:
>>>
>>> mod_rewrite: adding h2:// and h2c:// proxy schemes to absolute uri
>>> detection, patch by Evgeny Kotkov
>>> Submitted by: icing
>>> Reviewed/backported by: jim
>>>
>>> Modified:
>>>  httpd/httpd/branches/2.4.x/   (props changed)
>>>  httpd/httpd/branches/2.4.x/STATUS
>>>  httpd/httpd/branches/2.4.x/modules/mappers/mod_rewrite.c
>>>
>>>
>>> Modified: httpd/httpd/branches/2.4.x/STATUS
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1747056=1747055=1747056=diff
>>>
>>> ==
>>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>>> +++ httpd/httpd/branches/2.4.x/STATUS Mon Jun  6 19:11:20 2016
>>> @@ -115,12 +115,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
>>> [ start all new proposals below, under PATCHES PROPOSED. ]
>>> - *) mod_rewrite: support new h2:// and h2c:// proxy url schemes in
>>> detection
>>> - of absolute urls:
>>> - trunk patch: http://svn.apache.org/r1744206
>>> - 2.4.x patch: trunk works
>>> - +1: icing, jim, covener
>>>
>>
> Well, contrary to the commit message that failed to credit to the two
> other
> reviewers, it did have 3 +1's... we are pretty specific on backport policy.
>
> Is that a vote against, jallettc? If not, the backport remains valid.
>
> No, it's not a vote against.
> It is just a remark that we should move forward and either backport
> ap_cstr_casecmp[n] or decide to delay it to another release.
>
> We are building 2.4.21, so breaking compilation for a few days is, IMHO,
> acceptable.
>

Agreed, I don't like a broken trunk or branch state, but it isn't
a major crisis.


> I will have limited time until end of the week to update my
> ap_cstr_casecmp[n] backport proposal.
> So if anyone wants to update it before, do not hesitate.
>

I'm happy to do so, but my preference is to favor the apr_cstr_casecmp[n]
as I detailed in my (too many times edited) original reply to the proposal.

I'll work this up in the coming hour, no worries about your schedule.

Cheers,

Bill


Re: svn commit: r1747027 - /httpd/httpd/branches/2.4.x/STATUS

2016-06-06 Thread William A Rowe Jr
It seems the most critical point of my earlier post was missed...

On Mon, Jun 6, 2016 at 10:08 AM,  wrote:

> Author: wrowe
> Date: Mon Jun  6 15:08:01 2016
> New Revision: 1747027
>
> URL: http://svn.apache.org/viewvc?rev=1747027=rev
> Log:
> Showstopper to patch adoption, along with a more general objection
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1747027=1747026=1747027=diff
>
> ==
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Mon Jun  6 15:08:01 2016
> @@ -203,7 +203,20 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>   2.4.x patch:
> http://home.apache.org/~jailletc36/apr_cstr_casecmp_v2.diff
>   jailletc36: v2: update as per discussion on dev@. Do not mix ap_
> and apr_
>   namespaces + tweak DOXYGEN comments.
> - +1: jailletc36, icing
> + +1: jailletc36, icing
> + -0: wrowe [Prefer that we pre-@deprecate this API and encourage
> users
> +to adopt the apr_ convention, reversing the #defines,
> being
> +ready for the httpd 2.next minor release.]
> + -1: wrowe [Notes this patch creates binary incompatible versions of
> httpd
> +with this change; there must be #else stubs in util.c
> sources
> +compiled against APR 1.6+ for binary compatibility, even
> if
> +these are simply;
> +AP_DECLARE(int) ap_cstr_casecmpn(const char *s1,
> + const char *s2,
> apr_size_t n)
> +{
> +return apr_cstr_casecmpn(s1, s2, n);
> +}
> +   ]
>

You cannot have httpd build in such a way that when one exigent circumstance
changes,  the resulting httpd binary is now a different binary with
different
exported symbols.

E.g.

  1. Install apr-1.5
  2. Build httpd 2.4.recent
  3. Build thirdparty mod_foo, referencing ap_cstr_casecmp()
  4. Pick up, build and install apr-1.6
  5. Pick up httpd 2.4.latest, build and install over 2.4.recent
  6. Start httpd.  LoadModule mod_foo fails, ap_cstr_casecmp() unresolved
  7. httpd fails to start.

We have a contract with third party modules that, once built against 2.4.x,
the will continue to work unmodified when loaded in 2.4.x+1.

It's a lesson from Compatibility 101.


Re: svn commit: r1747056 - in /httpd/httpd/branches/2.4.x: ./ STATUS modules/mappers/mod_rewrite.c

2016-06-06 Thread William A Rowe Jr
On Mon, Jun 6, 2016 at 3:10 PM, Marion & Christophe JAILLET <
christophe.jail...@wanadoo.fr> wrote:

> ap_casecmpstr breaks 2.4.x build.
> Keep strncasecmp for now? (waiting for ap_cstr_casecmp[n])
>
> Le 06/06/2016 à 21:11, j...@apache.org a écrit :
>
>> Author: jim
>> Date: Mon Jun  6 19:11:20 2016
>> New Revision: 1747056
>>
>> URL: http://svn.apache.org/viewvc?rev=1747056=rev
>> Log:
>> Merge r1744206 from trunk:
>>
>> mod_rewrite: adding h2:// and h2c:// proxy schemes to absolute uri
>> detection, patch by Evgeny Kotkov
>> Submitted by: icing
>> Reviewed/backported by: jim
>>
>> Modified:
>>  httpd/httpd/branches/2.4.x/   (props changed)
>>  httpd/httpd/branches/2.4.x/STATUS
>>  httpd/httpd/branches/2.4.x/modules/mappers/mod_rewrite.c
>>
>>
>> Modified: httpd/httpd/branches/2.4.x/STATUS
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1747056=1747055=1747056=diff
>>
>> ==
>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>> +++ httpd/httpd/branches/2.4.x/STATUS Mon Jun  6 19:11:20 2016
>> @@ -115,12 +115,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
>> [ start all new proposals below, under PATCHES PROPOSED. ]
>> - *) mod_rewrite: support new h2:// and h2c:// proxy url schemes in
>> detection
>> - of absolute urls:
>> - trunk patch: http://svn.apache.org/r1744206
>> - 2.4.x patch: trunk works
>> - +1: icing, jim, covener
>>
>
Well, contrary to the commit message that failed to credit to the two other
reviewers, it did have 3 +1's... we are pretty specific on backport policy.

Is that a vote against, jallettc? If not, the backport remains valid.


Re: Confusion about SSLProxyCheckPeerName/CN

2016-06-02 Thread William A Rowe Jr
This looks like the resulting patch.  Wordsmithing the docs changes today...

On Wed, Jun 1, 2016 at 1:50 PM, Ruediger Pluem <rpl...@apache.org> wrote:

>
> On 06/01/2016 05:45 PM, William A Rowe Jr wrote:
> >
> >   CheckPeerName  CheckPeerCN
> >on {ignored}CheckPeerName verification
> >unset unset CheckPeerName verification
> >unset onCheckPeerName verification?
> >unset off   no verification
> >off   on*CheckPeerCN* verification
> >off   unset | off   no verification
> >
> > Because CheckPeerName is a superset of the CheckPeerCN functionality,
> > I don't think there is any harm is using CheckPeerName in this case.
> >
>
> I think CheckPeerName is ok in this case.
>
> Regards
>
> Rüdiger
>

 Index: ssl_engine_io.c
===
--- ssl_engine_io.c (revision 1746587)
+++ ssl_engine_io.c (working copy)
@@ -1189,6 +1189,8 @@
 }
 }
 if ((sc->proxy_ssl_check_peer_name != SSL_ENABLED_FALSE) &&
+((sc->proxy_ssl_check_peer_cn != SSL_ENABLED_FALSE) ||
+ (sc->proxy_ssl_check_peer_name == SSL_ENABLED_TRUE)) &&
 hostname_note) {
 apr_table_unset(c->notes, "proxy-request-hostname");
 if (!cert
@@ -1200,7 +1202,7 @@
   "for hostname %s", hostname_note);
 }
 }
-else if ((sc->proxy_ssl_check_peer_cn != SSL_ENABLED_FALSE) &&
+else if ((sc->proxy_ssl_check_peer_cn == SSL_ENABLED_TRUE) &&
 hostname_note) {
 const char *hostname;
 int match = 0;


[RESULTS] [POLL] Commitment to 2.2.x lifecycle? (Was: End of the road of 2.2.x maintenance?)

2016-06-01 Thread William A Rowe Jr
On Wed, May 25, 2016 at 10:11 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

>
> So let's try this... would 2.2.x maintainers and PMC folks please answer
> this
> poll -if- you have an intention to help throughout the wind-down of 2.2.x,
> since
> this is all predicated on having committed-committers to participate.
> This isn't
> to say folks refuse to participate after the time period you offer, but
> for how long
> you are personally prepared to participate.  [If you aren't a 2.2.x legacy
> branch
> participant, testing RCs or applying backports, then no response is
> needed.]
>
>  *) I intend to help maintain/test 2.2.x releases over the next [] mos
>

Thanks for all your feedback... if my tally is correct;

6 mos 12 mos 18 mos
covener covener covener
wrowe
rpluem
druggeri
rjung
jorton wrowe
rpluem
druggeri
rjung
jorton
gsmith


>
>  *) I intend to backport/review 2.2.x security patches over the next
> [] mos
>


6 mos 12 mos 18 mos
drugerri
covener
rjung
yann drugerri
covener
rjung
yann drugerri
covener
rjung
yann
wrowe
rpluem
jaillet
jorton wrowe
rpluem
jailletc36
jorton
gls

It seems we can safely call out the following in the 2.4.21 and future 2.2.x
releases;

"The Apache Web Server Project will continue to provide maintenance
releases of the 2.2.x flavor through June of 2017, and will provide some
security patches beyond this date through at least December of 2017.
Minimal maintenance patches are expected throughout this period,
and users are strongly encouraged to complete their transitions to
the 2.4.x flavor of httpd on an expedited basis to benefit from a larger
assortment of bug fixes, software robustness and new features."

Or something along those lines.


Re: Confusion about SSLProxyCheckPeerName/CN

2016-06-01 Thread William A Rowe Jr
On Wed, Jun 1, 2016 at 9:46 AM, Ruediger Pluem <rpl...@apache.org> wrote:

>
>
> On 06/01/2016 04:19 PM, William A Rowe Jr wrote:
> > Correcting one typo, below...
> >
> > On Wed, Jun 1, 2016 at 9:19 AM, William A Rowe Jr <wr...@rowe-clan.net
> <mailto:wr...@rowe-clan.net>> wrote:
> >
> >
> > Proposal...
> >
> > CheckPeerName  CheckPeerCN
> >  unset | onunset | onCheckPeerName verification
> >  off   on*CheckPeerCN* verification
> >  off   unset | off   no verification
> >  unset | off   off   no verification
> >
> > WDYT?
> >
> >
> >
>
> In general yes plus
>
> CheckPeerName  CheckPeerCN
>   onunset | offCheckPeerName verification
>

What about one more exceptional case... where the

CheckPeerCN On

is the only directive?  Do we still want to enable CheckPeerName by default?

  CheckPeerName  CheckPeerCN
   on {ignored}CheckPeerName verification
   unset unset CheckPeerName verification
   unset onCheckPeerName or CheckPeerCN verification?
   unset off   no verification
   off   on*CheckPeerCN* verification
   off   unset | off   no verification

Because CheckPeerName is a superset of the CheckPeerCN functionality,
I don't think there is any harm is using CheckPeerName in this case.


Re: Confusion about SSLProxyCheckPeerName/CN

2016-06-01 Thread William A Rowe Jr
Correcting one typo, below...

On Wed, Jun 1, 2016 at 9:19 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

>
> Proposal...
>
> CheckPeerName  CheckPeerCN
>  unset | onunset | onCheckPeerName verification
>  off   on*CheckPeerCN* verification
>  off   unset | off   no verification
>  unset | off   off   no verification
>
> WDYT?
>
>
>


Re: Confusion about SSLProxyCheckPeerName/CN

2016-06-01 Thread William A Rowe Jr
On Tue, May 31, 2016 at 1:15 PM, Ruediger Pluem <rpl...@apache.org> wrote:

>
> On 05/31/2016 06:37 PM, William A Rowe Jr wrote:
> > It seems the behavior introduced in 2.4.5 is causing a lot
> > of confusion for users attempting to disable peer checking.
> >
> > Right now, nothing needs to be done to do deep inspection
> > (altsubjectname plus common name).  Neither directive is
> > required, both default to on.
> >
> > Disabling checking is a pain in the ass, and the docs seem
> > to be very misleading;
> >
> >
> > SSLProxyCheckPeerName Directive
> >
> > Default: <
> http://httpd.apache.org/docs/2.4/mod/directive-dict.html#Default>
>  |SSLProxyCheckPeerName on
> > |
> >
> > This directive configures host name checking for server certificates
> when mod_ssl is acting as an SSL client. The check
> > will succeed if the host name from the request URI is found in either
> the subjectAltName extension or (one of) the CN
> > attribute(s) in the certificate's subject. If the check fails, the SSL
> request is aborted and a 502 status code (Bad
> > Gateway) is returned. The directive supersedes |SSLProxyCheckPeerCN
> > <http://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslproxycheckpeercn>|,
> which only checks for the expected host name
> > in the first CN attribute.
> >
> >
> > SSLProxyCheckPeerCN Directive
> >
> > Default: <
> http://httpd.apache.org/docs/2.4/mod/directive-dict.html#Default>
>  |SSLProxyCheckPeerCN on|
> >
> > This directive sets whether the remote server certificate's CN field is
> compared against the hostname of the request
> > URL. If both are not equal a 502 status code (Bad Gateway) is sent.
> >
> > In 2.4.5 and later, SSLProxyCheckPeerCN has been superseded by
> |SSLProxyCheckPeerName
> > <http://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslproxycheckpeername>|,
> and its setting is only taken into account
> > when |SSLProxyCheckPeerName off| is specified at the same time.
> >
> >
> > So under CheckPeerName, we *claim* that the directive *supersedes* the
> CheckPeerCN - but taking the only action you can
> > with CheckPeerName (turning it off) falls right back into the trap of
> CheckPeerCN, which *must* be *seperately*
> > disabled. This behavior sucks.
> >
> > I would suggest that CheckPeerCN should NOT default to "on" any longer.
> The only valid use case is for the user to
> > actively disable CheckPeerName (off), and has still wishes to actively
> enable CheckPeerCN (on).
> >
> > But we will need to improve this horrible CheckPeerName documentation
> for users of 2.4.5 through 2.4.20, even if we
> > change the behavior.
> >
> > Thoughts?
> >
> >
>
> As I felt into this trap a couple of days ago on my own a wholeheartedly
> +1.
>
> Regards
>
> Rüdiger


Thinking about this further, I don't know that this fix is correct.

A user on 2.4.3 (there are many) will be broken moving to 2.4.21 if their
config
simply reads;

SSLProxyCheckPeerCN off

Their intent was pretty clear, turn off this test. The fact that we added
the
alt subject names and wildcard domains doesn't address the fact that the
user simply wanted this turned *off* and the user is *not* supposed to ever
need to change their configuration from one subversion release to another.
We are *not* supposed to change the behavior of the server in unexpected
ways from one subversion release to another.

Proposal...

CheckPeerName  CheckPeerCN
 unset | onunset | onCheckPeerName verification
 off   onCheckPeerName verification
 off   unset | off   no verification
 unset | off   off   no verification

WDYT?


Re: Confusion about SSLProxyCheckPeerName/CN

2016-05-31 Thread William A Rowe Jr
On Tue, May 31, 2016 at 11:37 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> It seems the behavior introduced in 2.4.5 is causing a lot
> of confusion for users attempting to disable peer checking.
>
> I would suggest that CheckPeerCN should NOT default to "on" any longer.
> The only valid use case is for the user to actively disable CheckPeerName
> (off), and has still wishes to actively enable CheckPeerCN (on).
>
> But we will need to improve this horrible CheckPeerName documentation for
> users of 2.4.5 through 2.4.20, even if we change the behavior
>

--- ssl_engine_io.c (revision 1746297)
+++ ssl_engine_io.c (working copy)
@@ -1200,7 +1200,7 @@
   "for hostname %s", hostname_note);
 }
 }
-else if ((sc->proxy_ssl_check_peer_cn != SSL_ENABLED_FALSE) &&
+else if ((sc->proxy_ssl_check_peer_cn == SSL_ENABLED_TRUE) &&
 hostname_note) {
 const char *hostname;
 int match = 0;


Seems to be the entire patch, no?

Bill


Confusion about SSLProxyCheckPeerName/CN

2016-05-31 Thread William A Rowe Jr
It seems the behavior introduced in 2.4.5 is causing a lot
of confusion for users attempting to disable peer checking.

Right now, nothing needs to be done to do deep inspection
(altsubjectname plus common name).  Neither directive is
required, both default to on.

Disabling checking is a pain in the ass, and the docs seem
to be very misleading;

SSLProxyCheckPeerName Directive
Default: 
SSLProxyCheckPeerName
on
This directive configures host name checking for server certificates when
mod_ssl is acting as an SSL client. The check will succeed if the host name
from the request URI is found in either the subjectAltName extension or
(one of) the CN attribute(s) in the certificate's subject. If the check
fails, the SSL request is aborted and a 502 status code (Bad Gateway) is
returned. The directive supersedes SSLProxyCheckPeerCN
,
which only checks for the expected host name in the first CN attribute.

SSLProxyCheckPeerCN Directive
Default: 
SSLProxyCheckPeerCN
on
This directive sets whether the remote server certificate's CN field is
compared against the hostname of the request URL. If both are not equal a
502 status code (Bad Gateway) is sent.

In 2.4.5 and later, SSLProxyCheckPeerCN has been superseded by
SSLProxyCheckPeerName
,
and its setting is only taken into account when SSLProxyCheckPeerName off is
specified at the same time.


So under CheckPeerName, we *claim* that the directive *supersedes* the
CheckPeerCN - but taking the only action you can with CheckPeerName
(turning it off) falls right back into the trap of CheckPeerCN, which
*must* be *seperately* disabled. This behavior sucks.

I would suggest that CheckPeerCN should NOT default to "on" any longer. The
only valid use case is for the user to actively disable CheckPeerName
(off), and has still wishes to actively enable CheckPeerCN (on).

But we will need to improve this horrible CheckPeerName documentation for
users of 2.4.5 through 2.4.20, even if we change the behavior.

Thoughts?


Re: New segfault with 2.4.20 with mod_perl

2016-05-31 Thread William A Rowe Jr
On Tue, May 31, 2016 at 7:35 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On May 29, 2016 01:02, "Jie Gao" <j@sydney.edu.au> wrote:
> >
> > Hi All
> >
> > I wonder if anybody is looking at this issue. At the moment, the build
> cores even at the end of generating a Makefile.
> >
> > If not, I would like to get my hands dirty in an attmpt to get the ball
> rolling. Any help on how to get a handle on the "ip comparisons"
> recommended by W. Rowe Jr would be much appreciated.
>
> Hi Jie,
>
> At the moment, I'm still on the fence of whether this is a supportable use
> case (in the faux-handler for the modperl alt protocol example.)
>
> If it is not, the fix is to drop the test from modperl.
>

Alternately, the test could be fixed, but that is non-trivial...

A non-request oriented alt protocol simply never examines the req_rec,
and must not use request oriented modules like auth.  That should be
pretty obvious.

To create a pseudo -request- handler, you would have your connection
hook handler populate the appropriate fields and insert the appropriate
protocol-specific input and output filters below the request/body filters
and run the request through the rest of the phases, performing the
auth validation at the appropriate phase of the request (post_read
would be one obvious choice.)

Otherwise, the fix is also straightforward, I can provide hints, but you
> can cause the fault by changing the domain name in the test case config
> from example 'hostname' to example 'ip addr'.  The offending code is in the
> backtrace.  Both ip and host lookups would test if the useragent_addr is
> null, and use the corresponding lookups from the conn_rec.
>
> I sort of expect the modperl test to continue to be broken because the
> req_rec simply has not been fully initialized... it is not until the entire
> read_req hook phase is complete that all these req_rec field members have
> meaningful values.
>
> But if enough folks agree we can fix the lookup to refer to the conn_rec
> values until useragent_addr is initialized.
>


Re: [POLL] Commitment to 2.2.x lifecycle? (Was: End of the road of 2.2.x maintenance?)

2016-05-27 Thread William A Rowe Jr
On Wed, May 25, 2016 at 10:11 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

>
> So let's try this... would 2.2.x maintainers and PMC folks please answer
> this
> poll -if- you have an intention to help throughout the wind-down of 2.2.x,
> since
> this is all predicated on having committed-committers to participate.
> This isn't
> to say folks refuse to participate after the time period you offer, but
> for how long
> you are personally prepared to participate.  [If you aren't a 2.2.x legacy
> branch
> participant, testing RCs or applying backports, then no response is
> needed.]
>
>  *) I intend to help maintain/test 2.2.x releases over the next [] mos
>
>  *) I intend to backport/review 2.2.x security patches over the next
> [] mos
>

With the holiday weekend here in the US, I'll let the poll run a full week,
and cross-tabulate the results for us this coming Wednesday.


Re: Module Development - Advice Needed

2016-05-26 Thread William A Rowe Jr
On May 26, 2016 5:44 PM, "Van Ulden, Joost (NRCan/RNCan)" <
joost.vanul...@canada.ca> wrote:
>
> We want to develop an httpd module, “mod_mapml” which will allow the
admin to configure MapML responses over a map tile cache, using one or more
URI templates as configuration parameters.  A Java servlet prototype for
this functionality is available: https://github.com/Maps4HTML/MapMLServer.
(For the servlet, the parameters are passed via web.xml).

> We would like some advice on how to proceed with the development of a
module. For instance should we pursue this as a third-party module? Can we
tap into the community for development assistance if we don’t have the
required skills in-house? Any advice would be appreciated.

The users list is the best place to ask for clarification and guidance,
users are not exclusively admins, they include module authors,
troubleshooters, even a testing community.

I'd suggest starting with a clearly licensed github repo of your efforts
and encourage others to help fom the start in a public way.

Httpd project here has two paths, we have the core httpd package
distribution, but we have also championed add in modules.  An example would
be mod_proxy_fcgi, which is in the core bundle, and mod_fcgid, which is a
separate download.

It is probably easier to ask dev to consider your module for project
adoption here once there is a proof of concept/first draft of the working
module.  There are hundreds of third party modules, and we adopt relatively
few of these, but you should be able to create a community around your
efforts whether it is at this project, or external.

Yet another option is incubator.apache.org, and launch an independent
project as the add in module, under the Apache License 2.0 and the Apache
governance model.  The gene...@incubator.apache.org list is the starting
point for all such efforts.  If you were targeting httpd plus other server
frameworks, it is probably the best fit at the ASF.

Cheers

Bill


Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

2016-05-26 Thread William A Rowe Jr
On Thu, May 26, 2016 at 8:24 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

>
> Excellent, but one big issue, namespace collision.
>>
>> [...]
>>
> would be the proper doxygen, to dissuade users from directly consuming
> ap_ flavor.  However we would not drop the ap_flavor until 2.6.x so that
> any
> later updates of 2.4.x would still retain this function.
>
> When the user updates to apr 1.6.x without altering httpd 2.4.21, we don't
> want the symbol collisions between the httpd binary and libapr-1.so
> library.
> The effects vary between OS architectures, from inconvenient to fatal to
> entirely benign (win32 or os2 2-level namespaces).
>
> WDYT?
>

I overlooked something, we can simplify... in the header...

+#if !APR_VERSION_AT_LEAST(1,6,0)
+/**
+ * Known-fast version of strcasecmp(): ASCII case-folding, POSIX compliant
+ * @param s1 The 1st string to compare
+ * @param s2 The 2nd string to compare
+ * @return 0 if s1 is lexicographically equal to s2 ignoring case;
+ * non-0 otherwise.
+ */
+#define apr_cstr_casecmp(s1, s2) ap_cstr_casecmp(s1, s2)
+#endif
+
+/**
+ * HTTPD Internal function, do not use. @see apr_cstr_casecmp instead
+ * @deprecated Internal function will be absent from httpd 2.6 / 3.0.
+ */
+AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2);

Note the function declaration is a constant, above, not conditional.

In the C sources...

+#if APR_VERSION_AT_LEAST(1,6,0)
+int ap_cstr_casecmp(const char *s1, const char *s2)
+{
+return apr_cstr_casecmp(const char *s1, const char *s2);
+}
+#else
+int ap_cstr_casecmp(const char *s1, const char *s2)
+{
... full implementation.

This ensures any earlier module code linked to ap_cstr... will still
resolve that symbol, while transitioning to a call out to apr_cstr
flavor once httpd is recompiled with the newer apr.

This seems like the best transitional approach.


Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

2016-05-26 Thread William A Rowe Jr
Premature ctrl+enter... sorry about that...

+#if !APR_VERSION_AT_LEAST(1,6,0)
> +/**
> + * Known-fast version of strcasecmp(): ASCII case-folding, POSIX compliant
> + * @param s1 The 1st string to compare
> + * @param s2 The 2nd string to compare
> + * @return 0 if s1 is lexicographically equal to s2 ignoring case;
> + * non-0 otherwise.
> + */
> +#define apr_cstr_casecmp(s1, s2) ap_cstr_casecmp(s1, s2)
> ++/** * Known-fast version of strcasecmp(): ASCII case-folding, POSIX 
> compliant
> +AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2);
>
> On Thu, May 26, 2016 at 3:52 AM,  wrote:
>
>> Author: jailletc36
>> Date: Thu May 26 08:52:09 2016
>> New Revision: 1745582
>>
>> URL: http://svn.apache.org/viewvc?rev=1745582=rev
>> Log:
>> Proposal
>>
>> Modified:
>> httpd/httpd/branches/2.4.x/STATUS
>>
>> Modified: httpd/httpd/branches/2.4.x/STATUS
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1745582=1745581=1745582=diff
>>
>> ==
>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>> +++ httpd/httpd/branches/2.4.x/STATUS Thu May 26 08:52:09 2016
>> @@ -188,6 +188,21 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>   2.4.x patch: trunk works
>>   +1: jailletc36,
>>
>> +  *) core: ASCII string comparison functions optimized speed.
>> + This proposal includes and renames ap_casecmpstr[n] functions
>> available
>> + in trunk.
>> + The proposed names are the ones used in APR for the same kind of
>> functions.
>> + In order to avoid some new ap_ functions (which are just copies of
>> what is
>> + available in APR 1.6.0+), I propose to use exactly the same names
>> and only
>> + declare and define the functions in httpd if not available in APR.
>> + The same approach has already been used for apr_time_from_msec()
>> for example.
>> + Also note that the implementation in APR and in httpd are slighly
>> different.
>> + If/when aggreed on this backport and function names in httpd, then
>> trunk should
>> + be upgraded accordingly. Uses of the functions could then be
>> backported.
>> + trunk patch: ?
>> + 2.4.x patch:
>> http://home.apache.org/~jailletc36/apr_cstr_casecmp.diff
>> + +1: jailletc36,
>> +
>>  PATCHES/ISSUES THAT ARE BEING WORKED
>>
>>*) http: Don't remove the Content-Length of zero from a HEAD response
>> if
>>
>>
>>
>


Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

2016-05-26 Thread William A Rowe Jr
This will teach me to go back to pine... gui web clients... grrr...

Final draft...

On Thu, May 26, 2016 at 8:16 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> Excellent, but one big issue, namespace collision.
>
> What about...
>

+#if !APR_VERSION_AT_LEAST(1,6,0)
+/**
+ * Known-fast version of strcasecmp(): ASCII case-folding, POSIX compliant
+ * @param s1 The 1st string to compare
+ * @param s2 The 2nd string to compare
+ * @return 0 if s1 is lexicographically equal to s2 ignoring case;
+ * non-0 otherwise.
+ */
+#define apr_cstr_casecmp(s1, s2) ap_cstr_casecmp(s1, s2)
+
+/**
+ * HTTPD Internal function, do not use. @see apr_cstr_casecmp instead
+ */
+AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2);

would be the proper doxygen, to dissuade users from directly consuming
ap_ flavor.  However we would not drop the ap_flavor until 2.6.x so that any
later updates of 2.4.x would still retain this function.

When the user updates to apr 1.6.x without altering httpd 2.4.21, we don't
want the symbol collisions between the httpd binary and libapr-1.so library.
The effects vary between OS architectures, from inconvenient to fatal to
entirely benign (win32 or os2 2-level namespaces).

WDYT?

On Thu, May 26, 2016 at 3:52 AM, <jaillet...@apache.org> wrote:
>
>> Author: jailletc36
>> Date: Thu May 26 08:52:09 2016
>> New Revision: 1745582
>>
>> URL: http://svn.apache.org/viewvc?rev=1745582=rev
>> Log:
>> Proposal
>>
>> Modified:
>> httpd/httpd/branches/2.4.x/STATUS
>>
>> Modified: httpd/httpd/branches/2.4.x/STATUS
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1745582=1745581=1745582=diff
>>
>> ==
>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>> +++ httpd/httpd/branches/2.4.x/STATUS Thu May 26 08:52:09 2016
>> @@ -188,6 +188,21 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>   2.4.x patch: trunk works
>>   +1: jailletc36,
>>
>> +  *) core: ASCII string comparison functions optimized speed.
>> + This proposal includes and renames ap_casecmpstr[n] functions
>> available
>> + in trunk.
>> + The proposed names are the ones used in APR for the same kind of
>> functions.
>> + In order to avoid some new ap_ functions (which are just copies of
>> what is
>> + available in APR 1.6.0+), I propose to use exactly the same names
>> and only
>> + declare and define the functions in httpd if not available in APR.
>> + The same approach has already been used for apr_time_from_msec()
>> for example.
>> + Also note that the implementation in APR and in httpd are slighly
>> different.
>> + If/when aggreed on this backport and function names in httpd, then
>> trunk should
>> + be upgraded accordingly. Uses of the functions could then be
>> backported.
>> + trunk patch: ?
>> + 2.4.x patch:
>> http://home.apache.org/~jailletc36/apr_cstr_casecmp.diff
>> + +1: jailletc36,
>> +
>>  PATCHES/ISSUES THAT ARE BEING WORKED
>>
>>*) http: Don't remove the Content-Length of zero from a HEAD response
>> if
>>
>>
>>
>


Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

2016-05-26 Thread William A Rowe Jr
>
> On Thu, May 26, 2016 at 3:52 AM,  wrote:
>>
>>> Author: jailletc36
>>> Date: Thu May 26 08:52:09 2016
>>> New Revision: 1745582
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1745582=rev
>>> Log:
>>> Proposal
>>>
>>> Modified:
>>> httpd/httpd/branches/2.4.x/STATUS
>>>
>>> Modified: httpd/httpd/branches/2.4.x/STATUS
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1745582=1745581=1745582=diff
>>>
>>> ==
>>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>>> +++ httpd/httpd/branches/2.4.x/STATUS Thu May 26 08:52:09 2016
>>> @@ -188,6 +188,21 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>>   2.4.x patch: trunk works
>>>   +1: jailletc36,
>>>
>>> +  *) core: ASCII string comparison functions optimized speed.
>>> + This proposal includes and renames ap_casecmpstr[n] functions
>>> available
>>> + in trunk.
>>> + The proposed names are the ones used in APR for the same kind of
>>> functions.
>>> + In order to avoid some new ap_ functions (which are just copies of
>>> what is
>>> + available in APR 1.6.0+), I propose to use exactly the same names
>>> and only
>>> + declare and define the functions in httpd if not available in APR.
>>> + The same approach has already been used for apr_time_from_msec()
>>> for example.
>>> + Also note that the implementation in APR and in httpd are slighly
>>> different.
>>> + If/when aggreed on this backport and function names in httpd, then
>>> trunk should
>>> + be upgraded accordingly. Uses of the functions could then be
>>> backported.
>>> + trunk patch: ?
>>> + 2.4.x patch:
>>> http://home.apache.org/~jailletc36/apr_cstr_casecmp.diff
>>> + +1: jailletc36,
>>> +
>>>  PATCHES/ISSUES THAT ARE BEING WORKED
>>>
>>>*) http: Don't remove the Content-Length of zero from a HEAD response
>>> if
>>>
>>>
>>>
>>
>


Re: svn commit: r1745582 - /httpd/httpd/branches/2.4.x/STATUS

2016-05-26 Thread William A Rowe Jr
Excellent, but one big issue, namespace collision.

What about...

+#if !APR_VERSION_AT_LEAST(1,6,0)
+/**
+ * Known-fast version of strcasecmp(): ASCII case-folding, POSIX compliant
+ * @param s1 The 1st string to compare
+ * @param s2 The 2nd string to compare
+ * @return 0 if s1 is lexicographically equal to s2 ignoring case;
+ * non-0 otherwise.
+ */
+#define apr_cstr_casecmp(s1, s2) ap_cstr_casecmp(s1, s2)
++/**+ * Known-fast version of strcasecmp(): ASCII case-folding, POSIX compliant
+AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2);


On Thu, May 26, 2016 at 3:52 AM,  wrote:

> Author: jailletc36
> Date: Thu May 26 08:52:09 2016
> New Revision: 1745582
>
> URL: http://svn.apache.org/viewvc?rev=1745582=rev
> Log:
> Proposal
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1745582=1745581=1745582=diff
>
> ==
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Thu May 26 08:52:09 2016
> @@ -188,6 +188,21 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>   2.4.x patch: trunk works
>   +1: jailletc36,
>
> +  *) core: ASCII string comparison functions optimized speed.
> + This proposal includes and renames ap_casecmpstr[n] functions
> available
> + in trunk.
> + The proposed names are the ones used in APR for the same kind of
> functions.
> + In order to avoid some new ap_ functions (which are just copies of
> what is
> + available in APR 1.6.0+), I propose to use exactly the same names
> and only
> + declare and define the functions in httpd if not available in APR.
> + The same approach has already been used for apr_time_from_msec() for
> example.
> + Also note that the implementation in APR and in httpd are slighly
> different.
> + If/when aggreed on this backport and function names in httpd, then
> trunk should
> + be upgraded accordingly. Uses of the functions could then be
> backported.
> + trunk patch: ?
> + 2.4.x patch:
> http://home.apache.org/~jailletc36/apr_cstr_casecmp.diff
> + +1: jailletc36,
> +
>  PATCHES/ISSUES THAT ARE BEING WORKED
>
>*) http: Don't remove the Content-Length of zero from a HEAD response if
>
>
>


Re: svn commit: r1745517 - /httpd/httpd/branches/2.4.x/support/win32/ApacheMonitor.c

2016-05-25 Thread William A Rowe Jr
On Wed, May 25, 2016 at 1:14 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> Your comment doesn't match the code, and I think you have the condition
> inverted, _setargv() worked for decades, and only was broken in the more
> recent MSVC's.
>

Correction, the code looks right (sort of, I believe you might have broken
the
command line invocation to halt ApacheMonitor.exe to allow for it to be
upgraded
or replaced, and perhaps to re-invoke itself as Admin.)  The commit log
message
seems inverted.


Re: svn commit: r1745517 - /httpd/httpd/branches/2.4.x/support/win32/ApacheMonitor.c

2016-05-25 Thread William A Rowe Jr
Your comment doesn't match the code, and I think you have the condition
inverted, _setargv() worked for decades, and only was broken in the more
recent MSVC's.

My thought is to unilaterally change this to the unicode implementation,
because 1. ANSI-only are dead Windows OS's, and 2. Getting the utf-8
thing right in this app is becoming a big headache.

Thoughts?



On Wed, May 25, 2016 at 11:29 AM,  wrote:

> Author: gsmith
> Date: Wed May 25 16:29:59 2016
> New Revision: 1745517
>
> URL: http://svn.apache.org/viewvc?rev=1745517=rev
> Log:
> backport r1745516
> _setargv will not compile on _MSC_VER < 1700
> MS documentation's example simply does not work.
> Disabe for now, Apachemonitor still works.
>
> Modified:
> httpd/httpd/branches/2.4.x/support/win32/ApacheMonitor.c
>
> Modified: httpd/httpd/branches/2.4.x/support/win32/ApacheMonitor.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/support/win32/ApacheMonitor.c?rev=1745517=1745516=1745517=diff
>
> ==
> --- httpd/httpd/branches/2.4.x/support/win32/ApacheMonitor.c (original)
> +++ httpd/httpd/branches/2.4.x/support/win32/ApacheMonitor.c Wed May 25
> 16:29:59 2016
> @@ -1586,8 +1586,10 @@ int WINAPI WinMain(HINSTANCE hInstance,
>  #ifdef UNICODE
>  __wargv = CommandLineToArgvW(GetCommandLineW(), &__argc);
>  #else
> +#if defined(_MSC_VER) && _MSC_VER < 1800
>  _setargv();
>  #endif
> +#endif
>
>  if ((__argc == 2) && (_tcscmp(__targv[1], _T("--kill")) == 0))
>  {
>
>
>


Re: [POLL] Commitment to 2.2.x lifecycle? (Was: End of the road of 2.2.x maintenance?)

2016-05-25 Thread William A Rowe Jr
 *) I intend to help maintain/test 2.2.x releases over the next 12 mos

 *) I intend to backport/review 2.2.x security patches over the next 12 mos


[POLL] Commitment to 2.2.x lifecycle? (Was: End of the road of 2.2.x maintenance?)

2016-05-25 Thread William A Rowe Jr
On Mon, May 16, 2016 at 5:07 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Mon, May 16, 2016 at 1:26 PM, Gregg Smith <g...@gknw.net> wrote:
>
>> On 5/16/2016 11:03 AM, Eric Covener wrote:
>>
>>> One shortcoming of a 12 month countdown is that some folks will not be
>>> able to plan/pitch/budget/execute a migration in 12 calendar months
>>> because of bad timing.
>>>
>>> As long as we're not committing to releases in this window, I'd
>>> personally be okay with pushing out to 18 months re: the later
>>> feedback in this thread. I don't feel too strongly about>  12 months
>>> notice, but I will still have a 2.2-derivative in support beyond that
>>> window anyway.
>>>
>>
>> We should just pick a date right now (say 31/12/2017) and begin
>> announcing (website/download page/mail lists) it now. Include that one last
>> release will come out before the end of the year and it will be patches
>> only after final release and until EOL date. That is 18 and a half months
>> to EOL with a final release within 6 and a half.
>>
>
> To square this circle... I had expect that the 'final release' for httpd
> 2.2 would
> be based on the need for a security fix release, and expected that this may
> happen as many times over that year long period as were necessary.
>

So let's try this... would 2.2.x maintainers and PMC folks please answer
this
poll -if- you have an intention to help throughout the wind-down of 2.2.x,
since
this is all predicated on having committed-committers to participate.  This
isn't
to say folks refuse to participate after the time period you offer, but for
how long
you are personally prepared to participate.  [If you aren't a 2.2.x legacy
branch
participant, testing RCs or applying backports, then no response is needed.]

 *) I intend to help maintain/test 2.2.x releases over the next [] mos

 *) I intend to backport/review 2.2.x security patches over the next []
mos


Re: svn commit: r1733537 - /httpd/httpd/trunk/support/ab.c

2016-05-24 Thread William A Rowe Jr
On Thu, Mar 3, 2016 at 6:10 PM,  wrote:

> Author: ylavic
> Date: Fri Mar  4 00:10:10 2016
> New Revision: 1733537
>
> URL: http://svn.apache.org/viewvc?rev=1733537=rev
> Log:
> ab: Use caseless matching for HTTP tokens (e.g. content-length).
> PR 59111.
>
> Modified:
> httpd/httpd/trunk/support/ab.c
>
> Modified: httpd/httpd/trunk/support/ab.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/support/ab.c?rev=1733537=1733536=1733537=diff
>
> ==
> --- httpd/httpd/trunk/support/ab.c (original)
> +++ httpd/httpd/trunk/support/ab.c Fri Mar  4 00:10:10 2016
> @@ -426,6 +426,41 @@ static char *xstrdup(const char *s)
>  return ret;
>  }
>
> +/*
> + * Similar to standard strstr() but we ignore case in this version.
> + * Copied from ap_strcasestr().
> + */
> +static char *xstrcasestr(const char *s1, const char *s2)
> +{
> +char *p1, *p2;
> +if (*s2 == '\0') {
> +/* an empty s2 */
> +return((char *)s1);
> +}
> +while(1) {
> +for ( ; (*s1 != '\0') && (apr_tolower(*s1) != apr_tolower(*s2));
> s1++);
> +if (*s1 == '\0') {
> +return(NULL);
> +}
> +/* found first character of s2, see if the rest matches */
> +p1 = (char *)s1;
> +p2 = (char *)s2;
> +for (++p1, ++p2; apr_tolower(*p1) == apr_tolower(*p2); ++p1,
> ++p2) {
> +if (*p1 == '\0') {
> +/* both strings ended together */
> +return((char *)s1);
> +}
> +}
> +if (*p2 == '\0') {
> +/* second string ended, a match */
> +break;
> +}
> +/* didn't find a match here, try starting at next character in s1
> */
> +s1++;
> +}
> +return((char *)s1);
> +}
> +
>  /* pool abort function */
>  static int abort_on_oom(int retcode)
>  {
>

Two thoughts.  It seems this is a good candidate for APR 1.6
apr_cstr_casestr?

Second thought - it seems like the implement locally -> push to apr model
is good for as far as it gets us, but we don't want our httpd users to be in
a rat race to chase new apr[-util] behavior unless they are chasing a new
httpd optional module or feature.

What if we were to refactor some of server/util*.c into a localized
ap-httpd.so
library that could be consumed by httpd core, as well as our support
utilities
and third parties who want our very http-specific functions that are simply
not
good fits for apr?


Re: New segfault with 2.4.20 with mod_perl

2016-05-19 Thread William A Rowe Jr
Re-sending to include the correct perl.a.o dev list.

On Thu, Apr 14, 2016 at 1:25 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> The defect appears to be in t/protocol/TestProtocol/pseudo_http.pm...
>
> First, the handler is registered using
>
>   PerlProcessConnectionHandler TestProtocol::pseudo_http
>
> so its activities are outside of the request handling phase.
>
> Note that this logic has been broken, for a long time;
>
>2.4.1>
>   
>   Order Deny,Allow
>   Allow from @servername@
>   
>   
>
> Where @servername@ is a hostname, this module defect was
> identified in version 2.4.20 when we began using the per-req
> hostname in comparison (based on r->useragent_addr, which
> is obviously is null during part of the read_request phase).
>
> But this module using mod_access_compat during the connection
> phase has been broken for much longer, since Allow from {ip-addr}
> would already have failed since 2.4.1 was released, due to the
> same null r->useragent_addr.
>
> Effectively, mod_access_compat.c never supported per-connection
> IP addresses since it was added.  The fact that it supported
> per-connection hostname comparison was a quirk, and that the
> pseudo_http tests only looked at hostname and not ip comparisons
> was an oversight.
>
> But the module will fail in other manners if attempting to use
> http request_rec processing since that record is never fleshed
> out with the proper read/post_read request hook phases.
>
> My thought is to simply decouple access_compat from this
> module test... opinions?
>
> See also; https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=820824;msg=5
>
>
> On Thu, Apr 14, 2016 at 11:55 AM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
>
>> We can be more vigilant about unexpectedly null values, however...
>>
>> how are you running request processing in the connection callback
>> of mod_perl?  That makes no sense, and probably signals a deeper
>> logic error.
>>
>> The access checker is configured per-dir, so until the request rec
>> is completely initialized during read_request, this doesn't make
>> much sense to me (full backtrace .. including frames #6-#10, for
>> those who are curious...)
>>
>> Either the callback list registered for modperl_callback_connection,
>> or the Perl_runops_standard, or the Perl_pp_entersub invoking the
>> run_access_checker hook seem the most suspect here.
>>
>> #0  apr_getnameinfo (hostname=hostname@entry=0x7fd4461ee368, sockaddr=0x0, 
>> flags=flags@entry=0)
>> at /tmp/buildd/apr-1.5.2/network_io/unix/sockaddr.c:663
>> #1  0x55feaf0f513a in ap_get_useragent_host (r=r@entry=0x7fd4461ee0a0, 
>> type=type@entry=3,
>> str_is_ip=str_is_ip@entry=0x7fd44740c9c4) at core.c:990
>> #2  0x7fd4519d7212 in find_allowdeny (r=r@entry=0x7fd4461ee0a0, 
>> method=method@entry=0, a=,
>> a=) at mod_access_compat.c:279
>> #3  0x7fd4519d74b2 in check_dir_access (r=0x7fd4461ee0a0) at 
>> mod_access_compat.c:332
>> #4  0x55feaf0f8f30 in ap_run_access_checker (r=r@entry=0x7fd4461ee0a0) 
>> at request.c:87
>> #5  0x7fd448a6f7dd in XS_Apache2__RequestRec_run_access_checker 
>> (my_perl=0x55feb2964a20, cv=)
>> at HookRun.c:235
>> #6  0x7fd44f5f7e6a in Perl_pp_entersub () from 
>> /usr/lib/x86_64-linux-gnu/libperl.so.5.22
>> #7  0x7fd44f5f0ca6 in Perl_runops_standard () from 
>> /usr/lib/x86_64-linux-gnu/libperl.so.5.22
>> #8  0x7fd44f575f06 in Perl_call_sv () from 
>> /usr/lib/x86_64-linux-gnu/libperl.so.5.22
>> #9  0x7fd44f91ec28 in modperl_callback 
>> (my_perl=my_perl@entry=0x55feb2964a20, handler=0x7fd4461f2750,
>> p=p@entry=0x7fd4461f2028, r=r@entry=0x0, s=s@entry=0x7fd453ddc628, 
>> args=0x55feb3beebd0)
>> at modperl_callback.c:100
>> #10 0x7fd44f91f576 in modperl_callback_run_handlers (idx=0, 
>> type=type@entry=1, r=r@entry=0x0,
>> c=, s=0x7fd453ddc628, pconf=pconf@entry=0x0, plog=0x0, 
>> ptemp=0x0, run_mode=MP_HOOK_RUN_FIRST)
>> at modperl_callback.c:236
>> #11 0x7fd44f91fd4f in modperl_callback_connection (idx=, 
>> c=,
>> run_mode=) at modperl_callback.c:359
>> #12 0x55feaf10cdf0 in ap_run_process_connection 
>> (c=c@entry=0x7fd4461f22b8) at connection.c:42
>> #13 0x55feaf10d340 in ap_process_connection (c=c@entry=0x7fd4461f22b8, 
>> csd=csd@entry=0x7fd4461f20a0)
>> at connection.c:226
>> #14 0x7fd4523f3e6b in process_socket (bucket_alloc=0x7fd4461f0028, 
>> my_thread_num=1, my_ch

Re: mod_proxy_http2 windows build

2016-05-19 Thread William A Rowe Jr
On May 18, 2016 6:08 PM, "Jan Ehrhardt" <php...@ehrhardt.nl> wrote:
>
> William A Rowe Jr in gmane.comp.apache.devel (Wed, 18 May 2016 14:54:41
> -0500):
> >The .dsp files become irrelevant in this day and age, the legacy
environment
> >it maps to is entirely dead and beyond availability (snip)...
>
> Yet they are still the preferred way of building Apache by the people at
> Apachelounge. And the .mak files are preferred by Gregg at Apachehaus.

Although I've tweaked them for the past 10 years for separate APR vs httpd
builds, my headless build environments all had relied on .mak files.

CMake is essentially there, but I would never have figured out what httpd
was doing if I couldn't easily bring in httpd project files into Visual
Studio '98 a long time ago,, instrument it for browser source code
references, and toggle JIT debugging and step through within a few hours of
first digging into httpd 1.3.10.

I hope we keep things that simple, zero-intervention httpd builds, and
GUI/guided build environments such as visual studio, eclipse, etc


Re: mod_proxy_http2 windows build

2016-05-18 Thread William A Rowe Jr
On Wed, May 18, 2016 at 10:58 AM, Michal Karm <michal.baba...@gmail.com>
wrote:

> On 05/18/2016 05:24 PM, William A Rowe Jr wrote:
>
>> ... and .mak/.dep files in 2.4 branch, I'm on it today.
>>
>>
>> On Wed, May 18, 2016 at 10:09 AM, Stefan Eissing <
>> stefan.eiss...@greenbytes.de <mailto:stefan.eiss...@greenbytes.de>>
>> wrote:
>>
>> Reaching out to the knowledgable and always helpful Windows people:
>> do we
>> need a mod_proxy_http2.dsp in trunk/modules/http2 (and 2.4.x branch
>> for
>> next release)?
>>
>> Or perhaps to seize the opportunity to purge .dep/.mak and have a proper
> CMakeLists.txt instead? :)
>

As Stefan points out, that was already solved for the CMakeLists.txt...
if you consider it an 'improper' cmake implementation, I'm sure you can't
wait to post your patches to correct that.

Purging .dsp/.mak/.dep arrives with httpd 2.6/3.0.

We try not to be a$$es to our users as they upgrade from 2.2.31 to 2.2.32,
or from 2.4.20 to 2.4.21, so we ensure that the package remains buildable
from one subversion release to another... particularly because users often
pick up the latest subversion release to obtain security fixes, and we want
that transition from one to the next subversion release to involve as little
disruption as possible. This is across our users who are on windows, or
or linux, or any of a number of other architectures.

The .dsp files become irrelevant in this day and age, the legacy environment
it maps to is entirely dead and beyond availability, but our next chance to
fully
transition to cmake comes with the next major.minor version bump.


Re: mod_proxy_http2 windows build

2016-05-18 Thread William A Rowe Jr
... and .mak/.dep files in 2.4 branch, I'm on it today.


On Wed, May 18, 2016 at 10:09 AM, Stefan Eissing <
stefan.eiss...@greenbytes.de> wrote:

> Reaching out to the knowledgable and always helpful Windows people: do we
> need a mod_proxy_http2.dsp in trunk/modules/http2 (and 2.4.x branch for
> next release)?
>
> -Stefan


Re: RFC 7230..7235 Parsing Conformance?

2016-05-17 Thread William A Rowe Jr
Based on Jason's question...

On Tue, May 17, 2016 at 1:31 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Tue, May 17, 2016 at 1:00 PM, Julian Reschke <julian.resc...@gmx.de>
> wrote:
>
>> On 2016-05-17 19:01, Graham Leggett wrote:
>>
>>> On 17 May 2016, at 6:43 PM, William A Rowe Jr <wr...@rowe-clan.net>
>>> wrote:
>>>
>>> Wondering what other contributors are thinking on this topic.
>>>>
>>>> We have a number of changes in the ABNF grammar between
>>>> RFC2616 and RFC7230..7235. Do we want trunk 2.6/3.0 to be
>>>> an entirely RFC723x generation server, and drop all support for
>>>> RFC2616?
>>>>
>>>> Do we want to backport these changes to 2.4.x? If so, what
>>>> mechanism do we want to toggle the behavior of the server
>>>> between 2616 and 7230..7235?
>>>>
>>>> We can presume a small performance hit in any conditional
>>>> operation, especially when those decisions apply to tight parsing
>>>> loop. Toggling between two different parser implementations would
>>>> probably be a bit more efficient than conditionals within a parser
>>>> itself.
>>>>
>>>
>>> Can you give some examples to get a sense of the extent of this?
>>>
>> +1 to the question; I'd like to see examples as well...
>>
>> I believe we only changed the ABNF when we came to the conclusion that
>> the old one was incorrect, or did not reflect what implementations do in
>> practice.
>>
>
> One of the more significant is the change to token,
> https://tools.ietf.org/html/rfc2616#section-2.2
>
>  token  = 1*
>
>
   separators = "(" | ")" | "<" | ">" | "@"
  | "," | ";" | ":" | "\" | <">
  | "/" | "[" | "]" | "?" | "="
  | "{" | "}" | SP | HT


(Note that HT is a CTL, right, so it appears to be doubly excluded, no?)
CHAR is US-ASCII 0-127.


> vs https://tools.ietf.org/html/rfc7230#section-3.2.6
>
>  token  = 1*tchar
>
>  tchar  = "!" / "#" / "$" / "%" / "&" / "'" / "*"
> / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
> / DIGIT / ALPHA
> ; any VCHAR, except delimiters
>
>
  "Delimiters are chosen
   from the set of US-ASCII visual characters not allowed in a token
   (DQUOTE and "(),/:;<=>?@[\]{}")."


The characters missing above from tchar are '"', '(', ')', ',', '/', ':',
';', '<', '=', '>', '?', '@', '[', '\', ']', '{', '}' which corresponds to
this delimiter list, and to the RFC2616 list. VCHAR is clearly US-ASCII
20-7E, possibly includes tab. (Tabs are visible spacing.)

So my concerns may have been unfounded, but reviewing the new spec against
implementation still seems prudent. As I come across specifics we can
discuss those, sorry for my confusion.


Re: RFC 7230..7235 Parsing Conformance?

2016-05-17 Thread William A Rowe Jr
On Tue, May 17, 2016 at 1:00 PM, Julian Reschke <julian.resc...@gmx.de>
wrote:

> On 2016-05-17 19:01, Graham Leggett wrote:
>
>> On 17 May 2016, at 6:43 PM, William A Rowe Jr <wr...@rowe-clan.net>
>> wrote:
>>
>> Wondering what other contributors are thinking on this topic.
>>>
>>> We have a number of changes in the ABNF grammar between
>>> RFC2616 and RFC7230..7235. Do we want trunk 2.6/3.0 to be
>>> an entirely RFC723x generation server, and drop all support for
>>> RFC2616?
>>>
>>> Do we want to backport these changes to 2.4.x? If so, what
>>> mechanism do we want to toggle the behavior of the server
>>> between 2616 and 7230..7235?
>>>
>>> We can presume a small performance hit in any conditional
>>> operation, especially when those decisions apply to tight parsing
>>> loop. Toggling between two different parser implementations would
>>> probably be a bit more efficient than conditionals within a parser
>>> itself.
>>>
>>
>> Can you give some examples to get a sense of the extent of this?
>>
> +1 to the question; I'd like to see examples as well...
>
> I believe we only changed the ABNF when we came to the conclusion that the
> old one was incorrect, or did not reflect what implementations do in
> practice.
>

One of the more significant is the change to token,
https://tools.ietf.org/html/rfc2616#section-2.2

 token  = 1*


vs https://tools.ietf.org/html/rfc7230#section-3.2.6

 token  = 1*tchar

 tchar  = "!" / "#" / "$" / "%" / "&" / "'" / "*"
/ "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
/ DIGIT / ALPHA
; any VCHAR, except delimiters


This has a lot of tangential effects. My plan was to begin auditing
the code against spec and begin assembling patches. My question
in the note above is how we will generally handle the shift from one
spec to the next, and what we will promise our users (use 2.next/3.0
for conformance? Use 2.4 to retain 2616 behavior?)


RFC 7230..7235 Parsing Conformance?

2016-05-17 Thread William A Rowe Jr
Wondering what other contributors are thinking on this topic.

We have a number of changes in the ABNF grammar between
RFC2616 and RFC7230..7235. Do we want trunk 2.6/3.0 to be
an entirely RFC723x generation server, and drop all support for
RFC2616?

Do we want to backport these changes to 2.4.x? If so, what
mechanism do we want to toggle the behavior of the server
between 2616 and 7230..7235?

We can presume a small performance hit in any conditional
operation, especially when those decisions apply to tight parsing
loop. Toggling between two different parser implementations would
probably be a bit more efficient than conditionals within a parser
itself.


Re: End of the road of 2.2.x maintenance?

2016-05-16 Thread William A Rowe Jr
On Mon, May 16, 2016 at 1:26 PM, Gregg Smith <g...@gknw.net> wrote:

> On 5/16/2016 11:03 AM, Eric Covener wrote:
>
>> On Tue, May 10, 2016 at 1:38 PM, William A Rowe Jr<wr...@rowe-clan.net>
>> wrote:
>>
>>> Are we ready to start the 12 month countdown as of the next/final bug
>>> fix release of 2.2, and highlight this in both the 2.2 and 2.4 announce
>>> broadcasts?
>>>
>> One shortcoming of a 12 month countdown is that some folks will not be
>> able to plan/pitch/budget/execute a migration in 12 calendar months
>> because of bad timing.
>>
>> As long as we're not committing to releases in this window, I'd
>> personally be okay with pushing out to 18 months re: the later
>> feedback in this thread. I don't feel too strongly about>  12 months
>> notice, but I will still have a 2.2-derivative in support beyond that
>> window anyway.
>>
>
> We should just pick a date right now (say 31/12/2017) and begin announcing
> (website/download page/mail lists) it now. Include that one last release
> will come out before the end of the year and it will be patches only after
> final release and until EOL date. That is 18 and a half months to EOL with
> a final release within 6 and a half. This should leave any admin time to
> plan/pitch/budget/execute a migration and give everyone a firm date to EOL
> to plan around.
>

To square this circle... I had expect that the 'final release' for httpd
2.2 would
be based on the need for a security fix release, and expected that this may
happen as many times over that year long period as were necessary.

So from the 'next release' (using that release as a vehicle to make these
important announcements) throughout a fixed period, they should be able
to get security 'releases' for that period as part of a full tarball.

That's the model that we've observed, that the Tomcat project and other ASF
efforts have observed. It isn't cast in stone, but it seems to have been
workable.
If there were no significant security issues, we would probably have no
further
release candidates tarred and rolled and voted on.

Perhaps what Eric and Gregg are asking for is a longer 'official window' of
our
offering security patches as incidents come up? E.g. after the one year EOL
period of creating security releases, we would commit to offering security
fix
backports to 2.2.x over the following 6 mos? 1 year? These would all be
noted
under http://httpd.apache.org/security/vulnerabilities_22.html - they would
not
be 'releases' - would not require 3 PMC +1's, they could be provided so
long
as a committer wanted to prepare the backported patch, and would be subject
to discussion on security@ before publication, and discussion on dev@ after
they are publicized/disclosed.  Then it becomes a matter of how long we
have a group of committers dedicated to making these happen, and whether
that group can agree upon a timeframe we can communicate to users.

WDYT? I read my proposal, Gregg's and Eric's as trying to state sort
of the same thing in three different ways, and I'd like to offer users a
single
statement that accomplishes these aligned but differently-worded goals.


Re: End of the road of 2.2.x maintenance?

2016-05-16 Thread William A Rowe Jr
On Wed, May 11, 2016 at 2:31 PM, Mark Blackman <m...@exonetric.com> wrote:

>
> On 10 May 2016, at 21:38, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> Are we ready to start the 12 month countdown as of the next/final bug
> fix release of 2.2, and highlight this in both the 2.2 and 2.4 announce
> broadcasts?
>
> Feedback desired
>
> As a big consumer of Apache 2.2 in my day job, where we are obliged to
> track Apache’s policies very closely, I would prefer to delay this a bit.
> When Apache announces the formal end-of-life date of 2.2, we will be
> required to engineer the migration of 6000+ wildly diverse sites to Apache
> 2.4 to meet internal audit policies. I would propose the 12 month countdown
> starts no earlier than Jan 2017 (as a consumer).
>

An underlying issue from my own perspective is that OpenSSL 1.0.1 support
ends this year on December 31st. If there were ever a critical time to
update a deployment stack from end-to-end, the termination of support for
the highest-risk component would be the time to do so.

>From an operational perspective, the historical trend has been a 3 year
rolling deprecation of physical commodity hardware systems (although not
for big iron). It seems within the scope of modern cloud and virtualized
deployments, that window may be growing  shorter - with many apps
provisioned on their own discrete client environments.

The larger issue from our perspective, as an open-source collaboration -
not as a commercial distributor, is that it is hard to support users who
have insisted on deploying new httpd 2.2 instances over these past three
years. 2.4.1 was released literally 4 years ago. Users who insist on
deploying software which is already 3 to 4 years out of date need to seek
some paid commercial mitigation in terms of support, or assume the risk of
supporting the source code themselves (it is open source, when it breaks we
all get to keep all of the pieces and fix them as we like.)

This isn't really an appropriate role for ASF projects and volunteers. The
2.4 release a radically different situation than Gnome 3, for example,
which led to a split of opinion and dedicated communities interested in
continuing the legacy of Gnome 2. The migration path from 2.2 to 2.4 is not
painless, but is really not complex or unfamiliar. Modules are included to
ease the transition, particularly the transition of auth schemas.


> What’s the cost of maintaining (but maybe not updating) Apache 2.2?
>

Finding the volunteers among our committers and PMC members willing to
continue to evaluate and triage security reports, backport fixes, creating
release candidates and at least 3 PMC members willing to review and vote on
a release. And those costs are deducted from the opportunity to further
support the 2.4 branch and advance the development trunk.

Already, a rather large majority of our committers have abandoned the 2.2
maintenance branch and focus their attentions on trunk (2.6/3.0 future
release) and the 2.4 maintenance branch. So this question is largely
directed to the dedicated minority of committers who have continued to
service this branch throughout its 10 years.

If there are 3 clear votes against moving toward EOL at this time from our
PMC, this poll would fail (there is a big enough contingent to keep 2.2
releases alive for an extended period of time). Without 3 PMC members
reviewing release candidates, 2.2 would already be EOL by default.

Note that our choice to end ASF development of the 2.2 maintenance branch
only indirectly impacts the decision by vendors to end or continue
commercial support and security fixes to the software. This is why I'd
suggested that we may continue to collect security patches to the final
2.2.x release, as we have several distributors represented here who have a
lot to gain a lot by collaborating and reviewing the efficacy of security
patches as a community. As an OSS project, we would simply elect not to
craft a "release" of that software following our EOL consensus date. Two
vendor-supported examples, RHEL 6 still ships 2.2.15 and SLES 12 still
ships 2.2.22 - absolutely ancient software that they choose as a business
model to patch and update for key defects. That's a byproduct of their
decisions, not ASF decisions.

Hope this adds clarity to the suggested EOL, you can go back through the
list archives to see how we arrived at the decisions to end maintenance of
the 2.0, and 1.3 releases. It's always better to work as a group to that
end date, than to let an OSS project reach that point unannounced by
attrition of participants.

Yours,

Bill


Re: svn commit: r1742822 - /httpd/httpd/trunk/modules/http/http_core.c

2016-05-13 Thread William A Rowe Jr
On Fri, May 13, 2016 at 11:41 AM, Yann Ylavic <ylavic@gmail.com> wrote:

> On Thu, May 12, 2016 at 4:55 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> >
> > I'd explained in another thread this week why this patch is invalid,
> > and I've gone ahead and reverted.
> >
> > We agreed there is a defect here, what about the attached fix?
>
> Looks good, if something bad happened in ap_read_request() we already
> have responded with ap_send_error_response().
>
> What may be missing is reporting SERVER_BUSY_WRITE (with the
> partial/bad request) in ap_send_error_response(), and then we'd
> probably don't need to pass r for SERVER_BUSY_LOG in the error paths
> of ap_read_request().
>

AIUI, at that point in the code, if there was an error response it was
already
sent, the other paths appear to be simple disconnection states with no
further
logging or socket activity, other than forcing lingering close... perhaps.

In the lingering close phase, we do update the score status, so I don't
think
that needs to be done in these alternate code paths here.


Re: svn commit: r1742822 - /httpd/httpd/trunk/modules/http/http_core.c

2016-05-13 Thread William A Rowe Jr
On Thu, May 12, 2016 at 9:55 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Sun, May 8, 2016 at 8:53 AM, <rj...@apache.org> wrote:
>
>> Author: rjung
>> Date: Sun May  8 13:53:37 2016
>> New Revision: 1742822
>>
>> URL: http://svn.apache.org/viewvc?rev=1742822=rev
>> Log:
>> Fix yet another case where we clobber the
>> server-status request info when a timeout happens.
>>
>> Modified:
>> httpd/httpd/trunk/modules/http/http_core.c
>>
>> Modified: httpd/httpd/trunk/modules/http/http_core.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_core.c?rev=1742822=1742821=1742822=diff
>>
>> ==
>> --- httpd/httpd/trunk/modules/http/http_core.c (original)
>> +++ httpd/httpd/trunk/modules/http/http_core.c Sun May  8 13:53:37 2016
>> @@ -148,7 +148,8 @@ static int ap_process_http_async_connect
>>  c->keepalive = AP_CONN_UNKNOWN;
>>  /* process the request if it was read without error */
>>
>> -ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
>> +ap_update_child_status(c->sbh, SERVER_BUSY_WRITE,
>> +   r->the_request ? r : NULL);
>>  if (r->status == HTTP_OK) {
>>  cs->state = CONN_STATE_HANDLER;
>>  ap_process_async_request(r);
>> @@ -203,7 +204,8 @@ static int ap_process_http_sync_connecti
>>  c->keepalive = AP_CONN_UNKNOWN;
>>  /* process the request if it was read without error */
>>
>> -ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
>> +ap_update_child_status(c->sbh, SERVER_BUSY_WRITE,
>> +   r->the_request ? r : NULL);
>>  if (r->status == HTTP_OK) {
>>  if (cs)
>>  cs->state = CONN_STATE_HANDLER;
>>
>
> I'd explained in another thread this week why this patch is invalid,
> and I've gone ahead and reverted.
>
> We agreed there is a defect here, what about the attached fix?
>

Reposting, since gmail likes to base64 encode attached patches...

--- modules/http/http_core.c (revision 1743511)
+++ modules/http/http_core.c (working copy)
@@ -148,9 +148,9 @@
 c->keepalive = AP_CONN_UNKNOWN;
 /* process the request if it was read without error */

-ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
 if (r->status == HTTP_OK) {
 cs->state = CONN_STATE_HANDLER;
+ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
 ap_process_async_request(r);
 /* After the call to ap_process_request, the
  * request pool may have been deleted.  We set
@@ -203,10 +203,10 @@
 c->keepalive = AP_CONN_UNKNOWN;
 /* process the request if it was read without error */

-ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
 if (r->status == HTTP_OK) {
 if (cs)
 cs->state = CONN_STATE_HANDLER;
+ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
 ap_process_request(r);
 /* After the call to ap_process_request, the
  * request pool will have been deleted.  We set


>
>
>
>


Re: Scoreboard 3.0

2016-05-13 Thread William A Rowe Jr
On Fri, May 13, 2016 at 8:34 AM, Eric Covener <cove...@gmail.com> wrote:

> On Wed, May 11, 2016 at 6:39 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > In the next iteration for trunk I see a value in having both the basic
> > activity flag per worker thread so we can see those states at a glance
> as we
> > can today, as well as a summary table per process of the number of
> > recognized workers and inactive Conn/req per process by state.
>
> I believe we have (most of?) this data in 2.4 server-status for event.
>

Yup, that's certainly a great start to what we want.

My open question is still, whether administrators would want to see
both the per-connection and the per-thread detailed score records
on the same server.

Once we are in agreement on the backports of scoreboard.c to 2.4.x
branch, it would be a good time to introduce a new state between
reading and writing, perhaps called loading or processing, which
reflects the time between the start of the request handler, and the
beginning of writing to the client.  This would reflect both processing
time, and glomming of any input body.


Re: svn commit: r1742822 - /httpd/httpd/trunk/modules/http/http_core.c

2016-05-12 Thread William A Rowe Jr
On Sun, May 8, 2016 at 8:53 AM,  wrote:

> Author: rjung
> Date: Sun May  8 13:53:37 2016
> New Revision: 1742822
>
> URL: http://svn.apache.org/viewvc?rev=1742822=rev
> Log:
> Fix yet another case where we clobber the
> server-status request info when a timeout happens.
>
> Modified:
> httpd/httpd/trunk/modules/http/http_core.c
>
> Modified: httpd/httpd/trunk/modules/http/http_core.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_core.c?rev=1742822=1742821=1742822=diff
>
> ==
> --- httpd/httpd/trunk/modules/http/http_core.c (original)
> +++ httpd/httpd/trunk/modules/http/http_core.c Sun May  8 13:53:37 2016
> @@ -148,7 +148,8 @@ static int ap_process_http_async_connect
>  c->keepalive = AP_CONN_UNKNOWN;
>  /* process the request if it was read without error */
>
> -ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
> +ap_update_child_status(c->sbh, SERVER_BUSY_WRITE,
> +   r->the_request ? r : NULL);
>  if (r->status == HTTP_OK) {
>  cs->state = CONN_STATE_HANDLER;
>  ap_process_async_request(r);
> @@ -203,7 +204,8 @@ static int ap_process_http_sync_connecti
>  c->keepalive = AP_CONN_UNKNOWN;
>  /* process the request if it was read without error */
>
> -ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
> +ap_update_child_status(c->sbh, SERVER_BUSY_WRITE,
> +   r->the_request ? r : NULL);
>  if (r->status == HTTP_OK) {
>  if (cs)
>  cs->state = CONN_STATE_HANDLER;
>

I'd explained in another thread this week why this patch is invalid,
and I've gone ahead and reverted.

We agreed there is a defect here, what about the attached fix?
Index: modules/http/http_core.c
===
--- modules/http/http_core.c	(revision 1743511)
+++ modules/http/http_core.c	(working copy)
@@ -148,9 +148,9 @@
 c->keepalive = AP_CONN_UNKNOWN;
 /* process the request if it was read without error */
 
-ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
 if (r->status == HTTP_OK) {
 cs->state = CONN_STATE_HANDLER;
+ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
 ap_process_async_request(r);
 /* After the call to ap_process_request, the
  * request pool may have been deleted.  We set
@@ -203,10 +203,10 @@
 c->keepalive = AP_CONN_UNKNOWN;
 /* process the request if it was read without error */
 
-ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
 if (r->status == HTTP_OK) {
 if (cs)
 cs->state = CONN_STATE_HANDLER;
+ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
 ap_process_request(r);
 /* After the call to ap_process_request, the
  * request pool will have been deleted.  We set


Re: Scoreboard 3.0

2016-05-11 Thread William A Rowe Jr
On May 11, 2016 4:16 PM, "Eric Covener"  wrote:
>
> On Wed, May 11, 2016 at 8:28 AM, Stefan Eissing
>  wrote:
> > I got some such feedback when h2 connections truly went async and no
longer showed up as occupying a slot. I share your idea that in an event
based server, threads are not the primary dimension but connections are.
>
> Threads (slots) are the primary dimension for what you need to monitor
> to not get overrun.  The state of connections NOT running on a thread
> (keepalive, write completion, lingering close) seems to me to still be
> a distant second for routine work. I see value in listing individual
> write-completion requests, since they could be an issue from the users
> POV but would be de-emphasized in current server-status.

In the next iteration for trunk I see a value in having both the basic
activity flag per worker thread so we can see those states at a glance as
we can today, as well as a summary table per process of the number of
recognized workers and inactive Conn/req per process by state.  Those
should be detailed whether or not an extendedstatus is enabled, or not.

That table should help us understand the nature of any server overload.


Re: Scoreboard 3.0

2016-05-11 Thread William A Rowe Jr
On Wed, May 11, 2016 at 10:28 AM, Stefan Eissing <
stefan.eiss...@greenbytes.de> wrote:

>
> > Am 11.05.2016 um 17:03 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
> >
> > It seems like a lot of the scoreboard confusion lies in the event
> > MPM lies in the work-performed-by-thread model of the thread
> > based scoreboard.
> >
> > Let's call this existing model 'ExtendedStatus threads'
> >
> > What it seems some users and committers are looking for is
> > a different model, let's call this 'ExtendedStatus connections'.
>
> I got some such feedback when h2 connections truly went async and no
> longer showed up as occupying a slot. I share your idea that in an event
> based server, threads are not the primary dimension but connections are.
>

I think they are both representations, I'm curious if there is a use case
for
configuring both in verbose/extended mode at the same time.


> The information which thread handled a connection at which point in time
> seems irrelevant. If a connection occupies a thread for more than x
> seconds, it certainly becomes interesting. Also the waiting time, should
> all threads be occupied, is interesting as well as statistics about how
> many keepalive connections were closed early due to worker exhaustion.


+1 - e.g. track the time at which the connection awoke based on the
actual triggers in the pollset, even while the dispatcher is still trying
to find a slot for each of the now-ready sockets.  But until we poll again
and we won't until we dispatch all of the sockets which just woke us,
we won't be seeing other connections which are potentially now awake.

> If we adopted an ExtendedStatus connections, the scoreboard
> > table would be much larger than the number of event threads,
> > and would report all of the open connections to the server (at
> > least, all those that had passed the httpready threshold of our
> > awareness). It can't be a simple ring, because the slots would
> > expire at different times relative to their injection, so we would
> > probably want to track the oldest request. The most optimal
> > implementation would have score pools per-process so that
> > connection score slot reuse is a simple thread lock, but we
> > could alternately perform this tracking across processes.
> >
> > The first question that comes to mind is whether there is any
> > value in supporting ExtendedStatus threads+connections?
> > Two entire detailed lists residing in two different score shms?
> > Note that the traditional thread status flag scoreboard would
> > certainly be preserved, for a thread summary, even when the
> > user chooses ExtendedStatus connections.
> >
> > Feedback and more ideas?
>
> It seems the information which request had been processed is so
> interesting (which I totally destroyed in my ignorance) that scoreboard
> should make every attempt to keep it until it is replaced by something new.
> This is fine for the "static" HTML view that there is now.
>
> What I would find really cool is a feature that records connection/worker
> changes and lets me play that back in a browser. Maybe even a live feed of
> (a subset of) activities. Something that you see in the background on Star
> Trek, only it would be real.
>

Turning the conventional per-thread score and per-connection
score into two score-notice handlers, and allowing the user to
then load other modules which register against the score notice
hook, would accomplish this...

It could record individual deltas or snapshots at timed intervals. Not sure
> what is more feasible. I once did a status monitor of a system with various
> props that reported green/yellow/red and had that in a horizontal scroll in
> a browser. Customer was not happy as it became apparent how often his
> system was in red though...but that cannot happen to us!
>

I'm thinking ActiveMQ or a similar message broker might be our
friend here for creating such an app.  Not "logging", per say, but
every actual server connection/request state change notifications.

Digging the idea :)


Scoreboard 3.0

2016-05-11 Thread William A Rowe Jr
It seems like a lot of the scoreboard confusion lies in the event
MPM lies in the work-performed-by-thread model of the thread
based scoreboard.

Let's call this existing model 'ExtendedStatus threads'

What it seems some users and committers are looking for is
a different model, let's call this 'ExtendedStatus connections'.

If we adopted an ExtendedStatus connections, the scoreboard
table would be much larger than the number of event threads,
and would report all of the open connections to the server (at
least, all those that had passed the httpready threshold of our
awareness). It can't be a simple ring, because the slots would
expire at different times relative to their injection, so we would
probably want to track the oldest request. The most optimal
implementation would have score pools per-process so that
connection score slot reuse is a simple thread lock, but we
could alternately perform this tracking across processes.

The first question that comes to mind is whether there is any
value in supporting ExtendedStatus threads+connections?
Two entire detailed lists residing in two different score shms?
Note that the traditional thread status flag scoreboard would
certainly be preserved, for a thread summary, even when the
user chooses ExtendedStatus connections.

Feedback and more ideas?

Cheers,

Bill


Re: svn commit: r1742794 - /httpd/httpd/branches/2.4.x/STATUS

2016-05-11 Thread William A Rowe Jr
On Tue, May 10, 2016 at 11:36 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Tue, May 10, 2016 at 7:26 PM, Yann Ylavic <ylavic@gmail.com> wrote:
>
>>
>> The case where this happens is for keepalive/lingering-close handling.
>> Suppose thread t1 handles request r1 on connection c1, and t1 is
>> released after r1 (when c1 is kept alive).
>>
>
So thread t1 has traditionally reported 'K'eepalive state, but that is based
on the fact that t1 is still 'busy' waiting on the next request from c1, in
the classic prefork or worker models.

In the event case, thread t1 should now be reported as Idle, because that
worker no longer has any work assigned to it; however, the score slot
should continue to display t1's last request r1 until a new connection or
request occupies that slot, at which time everything related to r1 must
be zeroed out (replaced again by c1 where appropriate).

What we might want in the score is to track "unaffiliated connections"
with no thread assigned, in the state they were last in, but right now
we have no such reporting mechanism.


> Same for t2 with r2 on c2.
>>
>> Now the following cases (not exhaustive):
>> 1. r3 arrives on c1, t2 handles the event (read).
>> 2. c2 is closed/reset remotely, t1 handles the event (read).
>> 2. c1 is closed/reset remotely, t2 handles the event (read).
>>
>> Without my patch:
>> 1. t2 reports r3 on c1, "losing" r2 on c2 (no more reported by the
>> scoreboard), while t1 still reports r1 on c1 (although "superseded" by
>
> r3).
>
>
That behavior is by design, because t2 isn't performing any further work
on r2 from c2 - it is performing work for c1.  At this moment, c1 should
appear in both score slots, t1 still reports r1, but in an idle state
so that we know that isn't what the thread is doing right now, while
t2 also reports c1, which it picked up, in an active processing state.

The t2 should not report r1 because r1 is "done and finished" and t2
never performed work on r1.

At this time, c2 shouldn't appear in the scoreboard at this time; as I'd
observed above, that scoreboard functionality doesn't exist right now.


> 2. t1 reads/closes c2 and clobbers r1 (blank).
>>
>
That's fine, t1's only work on c2 was to read a request, which it did
not find.  t1 is working on c2.  t1 did not handle the request r1.


> 3. t2 reads/closes c1 and clobbers r3 (blank).
>> => t1 and t2 scoreboard's request/vhost are blank.
>>
>
At the time we read no request, if we have not yet yielded the thread,
the existing score slot for t2 should simply be updated with the status
of closing, no change to the request or vhost because these fields
should not be updated if no request is resolved.

If t2 yields after completing r3, when it resumes it has no request.
This is true whether you have jumped threads, or managed to land
back in the same thread as you illustrate above for c2.


> With my patch:
> 1. t2 reports r3 on c1, "losing" r2 on c2 (no more reported by the
> scoreboard), while t1 still reports r1 on c1 (although "superseded" by
> r3).
>

AIUI the propsoed trunk patch introduces an edge case where t2 will
briefly report r1 while t1 is still reporting r1, until the request is read
complete, and we are toggling to write status... is this correct?


> 2. t1 reads/closes c2 and restores r2.
>

Incorrect behavior, IMO.  t1 performed no work on r2.


> 3. t2 reads/closes c1 and restores r3.
> => t1 and t2 scoreboard's request/vhost contain relevent (last) info.
>

So in bullet 2. above, it isn't the last relevant activity of the thread t1.
You can argue that r3 was the most recent activity performed on t2,
but we would want to perform an identity check that the score slot
has simply resumed the same connection, that it had not jumped
threads, and that there was not intervening activity on that thread.

In httpd 2.4 and prior, the scoreboard is relating what each thread's
action is.  In httpd 3.0, you could certainly propose that the score
is connection oriented, with one slot per connection irrespective of
which thread (if any) is handling the request.  Then the proposed
behavior makes a great deal of sense.

I agree that we should simply not update the scoreboard with a
fresh "W" update when we have no request to write (the client
disconnected and went away).  That's why I cast a -1 on the test
you added for r->the_request, that isn't the right way to know that
there is no request being written.  I need to still dig further but am
really glad you pointed our attention at the offending status update.
I expect we can dodge that update altogether for a no-request state.

Believe that we may be missing an update to the scoreboard at the
moment we wake up an event thread, so I'm looking in that corner
as well.  In h

Re: svn commit: r1742794 - /httpd/httpd/branches/2.4.x/STATUS

2016-05-10 Thread William A Rowe Jr
On Tue, May 10, 2016 at 7:26 PM, Yann Ylavic <ylavic@gmail.com> wrote:

> On Tue, May 10, 2016 at 5:23 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > On Sun, May 8, 2016 at 7:29 AM, Yann Ylavic <ylavic@gmail.com>
> wrote:
> >>
> >> ISTM that the request line has already been clobbered (blanked) by the
> >> previous call to ap_update_child_status_from_conn(SERVER_BUSY_READ) in
> >> ap_process_http_[a]sync_connection(), where we already lost the
> >> previous request's line.
> >
> > That's FINE.  In the previous call we are already on to another thread,
> > this thread did not perform work on the previous request (or if it had,
> > it had surrendered the thread and accidentally ended back up there.)
>
> Right, that's why/when the new thread clobbers the request-line.
> We are talking about MPM event only here, with sync MPMs we do
> SERVER_BUSY_READ for the first request on the connection only, and my
> goal is to have consistent reporting for all MPMs.
>

But that's not what the third suggested backport does, it toggles the
request
state to "W"rite when there is actually no request received (either it was
actually not ready, or had spuriously disconnected)


> > The scoreboard reports what work this *thread* has performed.  So
> > far, if it is a wakeup to read a new request, on the event MPM in 2.4
> > this is not request-based work.
>
> The scoreboard reports that the thread is now working on a new
> connection (possibly kept alive), but clears half of the data
> associated with that connection (i.e. the last request/vhost)
> altogether.
>

Of course it should, because the old request is still parked on the old
worker thread until it is reclaimed, and the newly activated thread for
this connection never processed work for the old thread...


> >> That's why I proposed to save the previous request line and host in
> >> the conn_rec, and use them in the scoreboard until a new (real)
> >> request is available (it didn't work as expected because the
> >> scoreboard was clobbered in multiple places before Bill's changes, but
> >> it should work better/easily now).
> >
> >
> > It isn't workable because every time we hack around this issue, we
> > mask the actual errors in the calls to the scoreboard API.  In 2.4,
> > the API design is DSS.
>
> DSS?


Dirt simple stupid.


> > Complicating it is a really bad idea, and
> > misleads, if not actually breaking existing consumers of the scoreboard.
>
> My proposed patch (not that complicated IMHO) shouldn't break anything
> (if working as expected), it is meant for consistency between event
> and other MPMs (now that Rainer fixed the blanks there due to
> keepalive/close), see below.
>

I still need to further review, but you toggle a writing state for a request
with no inbound request, which is clearly wrong.


> >> That would be: when no r is available but c is, use the saved
> >> request-line/host from c; when r is available, use the ones from r and
> >> update them in c.
> >>
> >> That way there would be no blank at all (IOW, the scoreboard entry
> >> would be left with the latest request handled on the connection, not a
> >> blank which inevitably ends every connection after keepalive timeout
> >> or close, and shouldn't be taken into account as an empty request
> >> IMHO).
> >
> > Which is all wrong IMHO. If the request completed, that thread still
> > reports the previous request in an idle or keepalive state until that
> thread
> > is woken up to do new work.
>
> Right, for MPM event we can indeed consider that the reporting of the
> previous request for a thread is dead once this thread is given
> another connection, but when yet another thread will take care of the
> former connection, the previous request could show up in its reporting
> (that's how my patch works, faithfully reporting how each MPM works,
> without clobbering data unnecessarily).
>
> The case where this happens is for keepalive/lingering-close handling.
> Suppose thread t1 handles request r1 on connection c1, and t1 is
> released after r1 (when c1 is kept alive).
> Same for t2 with r2 on c2.
>
> Now the following cases (not exhaustive):
> 1. r3 arrives on c1, t2 handles the event (read).
> 2. c2 is closed/reset remotely, t1 handles the event (read).
> 2. c1 is closed/reset remotely, t2 handles the event (read).
>
> Without my patch:
> 1. t2 reports r3 on c1, "losing" r2 on c2 (no more reported by the
> scoreboard), while t1 still reports r1 on c1 (although "superseded" by
> r3).
> 2. t1 reads/cl

End of the road of 2.2.x maintenance?

2016-05-10 Thread William A Rowe Jr
It's been a year, and seems to be a good time to revisit this topic
while those folks who are present at ApacheCon can discuss f2f
the merits of bringing the 2.2.x chapter to a close, and share their
thoughts back here on-list.

According to http://w3techs.com/technologies/history_details/ws-apache/2
the inflection point of a majority of 2.4 instances over 2.2 appears
to occur about 9 months from now.

OpenSUSE 13.1 adopted 2.4 way back in Nov of '13.

Ubuntu - 14.04 LTS, and Debian 8 (Jessie) switched to 2.4 in April '14.

RHEL / CentOS 7 are well over a year old, adopted 2.4 in June '14.
Fedora 18 shipped 2.4 way back in Jan '13.

E.g. every user of the broadly distributed Linux releases will have had
three full years to adopt 2.4 by June of 2017.  I expect the BSD world
looks similar (modulo any Apache License 2.0 stupidity we are all
too familiar with.)  If someone in the BSD, Solaris and other spheres
wants to chime in here with those milestones, that would be great.

I am prepared to RM a final bug-fix release of 2.2 in conjunction with
the next 2.4 release effort, to gather in any final requests for fixes
before we move to a 12-month, security-fixes-only window on that branch.
Once those 12 months expire, as we've done with 1.3 and 2.0, there's
the possibility that relatively few committers would collect some critical
patches/apply-to-2.2.xx final security fixes, but no further releases would
occur.

Are we ready to start the 12 month countdown as of the next/final bug
fix release of 2.2, and highlight this in both the 2.2 and 2.4 announce
broadcasts?

I'm hoping we conclude some fixes of 2.4 scoreboard regressions and
get to the point of releasing 2.4 with mod_proxy_http2 sometime within
the next month or so, and that we can reach a consensus about how
we will proceed on the 2.2 branch, before we get to that release.

Feedback desired,

Cheers,

Bill


Re: svn commit: r1742794 - /httpd/httpd/branches/2.4.x/STATUS

2016-05-10 Thread William A Rowe Jr
I believe this is the right patch (or close to correct) for 2.4 branch, and
perhaps
even trunk with additional refactoring.

In the case of resuming a thread, it should be up to the event/advanced MPM
to reflect the correct no-info | [connection-info [request-info]] state of
the conn
at the time the thread is resumed with different work to perform.

On Sun, May 8, 2016 at 5:30 AM,  wrote:

> Author: rjung
> Date: Sun May  8 10:30:38 2016
> New Revision: 1742794
>
> URL: http://svn.apache.org/viewvc?rev=1742794=rev
> Log:
> Propose another (small) scoreboard/mod_status fix.
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1742794=1742793=1742794=diff
>
> ==
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Sun May  8 10:30:38 2016
> @@ -181,6 +181,20 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>   2.4.x patch: trunk works
>   +1: wrowe, rpluem
>
> +   * Don't globber scoreboard request info if read_request_line() fails
> with
> + a timeout. In that case there's not yet any new useful request info
> + available.
> + Noticed via server-status showing request "NULL" after keep-alive
> + connections timed out.
> + No CHANGES entry needed, because there's already one for PR 59333.
> + Sorry for the two patches. The second is better. Both change the same
> + line, so technically only the second is needed, but merging both
> keeps
> + mergeinfo more complete.
> + trunk patch: http://svn.apache.org/r1742791
> +  http://svn.apache.org/r1742792
> + 2.4.x patch: trunk works
> + +1: rjung
> +
>  PATCHES/ISSUES THAT ARE BEING WORKED
>
>*) http: Don't remove the Content-Length of zero from a HEAD response if
>
>
>


Re: svn commit: r1742794 - /httpd/httpd/branches/2.4.x/STATUS

2016-05-10 Thread William A Rowe Jr
On Sun, May 8, 2016 at 7:29 AM, Yann Ylavic  wrote:

>
> ISTM that the request line has already been clobbered (blanked) by the
> previous call to ap_update_child_status_from_conn(SERVER_BUSY_READ) in
> ap_process_http_[a]sync_connection(), where we already lost the
> previous request's line.
>

That's FINE.  In the previous call we are already on to another thread,
this thread did not perform work on the previous request (or if it had,
it had surrendered the thread and accidentally ended back up there.)

The scoreboard reports what work this *thread* has performed.  So
far, if it is a wakeup to read a new request, on the event MPM in 2.4
this is not request-based work.


That's why I proposed to save the previous request line and host in
> the conn_rec, and use them in the scoreboard until a new (real)
> request is available (it didn't work as expected because the
> scoreboard was clobbered in multiple places before Bill's changes, but
> it should work better/easily now).
>

It isn't workable because every time we hack around this issue, we
mask the actual errors in the calls to the scoreboard API.  In 2.4,
the API design is DSS.  Complicating it is a really bad idea, and
misleads, if not actually breaking existing consumers of the scoreboard.


> That would be: when no r is available but c is, use the saved
> request-line/host from c; when r is available, use the ones from r and
> update them in c.

That way there would be no blank at all (IOW, the scoreboard entry
> would be left with the latest request handled on the connection, not a
> blank which inevitably ends every connection after keepalive timeout
> or close, and shouldn't be taken into account as an empty request
> IMHO).
>

Which is all wrong IMHO. If the request completed, that thread still
reports the previous request in an idle or keepalive state until that thread
is woken up to do new work.


> Also, that would be consistent in both MPMs worker and event, whereas
> currently event is more likely to show this behaviour, thanks to
> queuing (no worker thread used for keepalive handling and hence no
> BUSY_READ during that time, no clobbering either).
> Thus for mpm_event, we'd need to use
> ap_update_child_status_from_conn(SERVER_CLOSING, c) so that the saved
> request-line/vhost from the connection get reused when/if a worker
> thread handles the lingering close.
>
> How about the attached patch?
>

This is the right direction for  - we will be waking up threads
to resume work on an already in-flight request, correct?  That request
needs to be displayed because that is the work this thread is performing.

On trunk and even with your patch as-is, IIUC you are filling in the score
request field for a thread that never performed work on a given request,
which is invalid data.

If we refine this idea somewhat to null out the request once the request
has been entirely fulfilled on a given thread, such that the thread still
reflects what last request work it performed, but the resumed connection
does not report a request it didn't satisfy, I think we will have a solid
fix
for trunk.

For 2.4 - I'm strongly -1 until convinced we can't navigate without it,
given that 2.4 does not allow cross-threaded request_rec lifespans.


Re: Detecting client aborts and stream resets

2016-05-04 Thread William A Rowe Jr
On Wed, May 4, 2016 at 3:44 PM, Michael Kaufmann 
wrote:

> William is right, this is not a good idea. The ->aborted flag should serve
>> this purpose of telling anyone interested that this connection is not
>> longer delivering. I will make a github release soon where that is working
>> and you can test.
>>
>>
> Thank you Stefan! It is now working for stream resets, but it's not yet
> working if the client closes the whole TCP connection.
>

As expected... this is why I pointed out in my first reply that you don't
want a single-protocol solution to this puzzle.

See my later reply about detecting connection tear-down, patches welcome.


Re: Detecting client aborts and stream resets

2016-05-04 Thread William A Rowe Jr
On Tue, May 3, 2016 at 2:05 PM, Michael Kaufmann <m...@michael-kaufmann.ch>
wrote:

> Zitat von William A Rowe Jr <wr...@rowe-clan.net>:
>
>>
>> Nope - an optional function in mod_http2 is too special case, generators
>> must remain protocol (socket or other transport) agnostic.
>>
>
> Sure, official Apache modules should not call protocol-dependent hooks,
> but it could be a solution for 3rd party modules.


Nonsense, third party module designers should be equally aware - when you
start meddling in unexpected edge cases, binary ABI won't be maintained
in those edge cases. You are always better off relying on the intended
design
of httpd, unless you want to chase a moving target.

Extending this detection with a protocol-agnostic API would be interesting,
however an optional function has only a single registered provider, so it
would
be the wrong interface. You want an interface that would test/report "gone
away"
irrespective of http1 vs http2 vs future protocols.


> In the case of mod_cache'd content, the generator can't quit, it is already
>> populating a cache, which means you'll generate an invalid cached object.
>>
>> But if you knew that the cache module wasn't collecting info,
>> r->c->aborted
>> tells you if anyone is still listening, right?
>>
>
> Unfortunately not. While the content generator is running (just preparing
> the response, it is not reading from the input filters anymore and not
> writing to the output filters yet), Apache does not check the connection's
> state.
>

Good point... where/how would we address this for CPU intensive generators?
We certainly don't bother in proxy, because proxy is much lighter-weight.

The obvious answer to me in a worker or event MPM would be for the user
to make a 'notify close?' request to the MPM, which would add the socket
with for the POLLHUP event alone after reading the input brigade (the read
will signal r->c->aborted already), with the single goal of setting the flag
r->c->aborted upon socket close. Very lightweight, but that presumes that
a second poll on the same fd for different events (e.g. poll on read or
write)
elsewhere won't give the kernel conniptions.

mod_http2 has enough going on the master connection that detecting the
stream close should occur fairly promptly.


Re: svn commit: r1741310 - in /httpd/httpd/trunk: modules/http2/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/ server/mpm/winnt/ server/mpm/worker/

2016-05-03 Thread William A Rowe Jr
On Tue, May 3, 2016 at 10:51 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

>
> If we want to be more robust, have a look at the current 2.4.x
> implementation of ap_get_useragent_host() in core.c...
>
> if (r->useragent_addr == conn->client_addr) {
> return ap_get_remote_host(conn, r->per_dir_config, type,
> str_is_ip);
> }
>
> Perhaps this would be better...
>
> if (!r->useragent_addr || (r->useragent_addr == conn->client_addr)) {
> return ap_get_remote_host(conn, r->per_dir_config, type,
> str_is_ip);
> }
>

Committed and proposed for backport.


Re: svn commit: r1741310 - in /httpd/httpd/trunk: modules/http2/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/ server/mpm/winnt/ server/mpm/worker/

2016-05-03 Thread William A Rowe Jr
On Mon, May 2, 2016 at 3:09 AM, Ruediger Pluem  wrote:

>
> On 04/28/2016 10:19 AM, Ruediger Pluem wrote:
> >
> > On 04/27/2016 08:41 PM, wr...@apache.org wrote:
> >> Author: wrowe
> >> Date: Wed Apr 27 18:41:49 2016
> >> New Revision: 1741310
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1741310=rev
> >> Log:
> >>
> >>   Ensure the useragent_ip is only used in the case
> >>   where it has been initialized, fall back on the
> >>   connection's remote_ip if the status is accidently
> >>   updated from an uninitialized request_rec.
> >>
> >> --- httpd/httpd/trunk/server/scoreboard.c (original)
> >> +++ httpd/httpd/trunk/server/scoreboard.c Wed Apr 27 18:41:49 2016
> >> @@ -501,7 +501,7 @@ static int update_child_status_internal(
> >>  copy_request(ws->request, sizeof(ws->request), r);
> >>  }
> >>
> >> -if (r) {
> >> +if (r && r->useragent_ip) {
> >>  if (!(val = ap_get_useragent_host(r, REMOTE_NOLOOKUP,
> NULL)))
> >>  apr_cpystrn(ws->client, r->useragent_ip,
> sizeof(ws->client));
> >
> > Hm, wouldn't it be better to just encapsulate the above line in an if
> (r->useragent_ip)
> > or do we already know that ap_get_useragent_host(r, REMOTE_NOLOOKUP,
> NULL) cannot deliver
> > something meaningful if r->useragent_ip == NULL?
>

It *could* deliver; but if r->useragent_ip is unset, this is
a premature call to set up the score entry, the request
read hook hasn't completed and will fail to deliver.  But
I don't know what you mean by "the above line" - you
mean the apr_cpystrn?  As you point out, the call is
fairly useless without a completed request rec and
known useragent_ip in the first place (you want to
look up the hostname of null address - but then
guard against copying a NULL string once it has
failed ? :-)

By falling back on the conn-based lookup, in the code you
snipped... we are populating the conn-based hostname,
which may be useful datum elsewhere...

[...]
else if (c) {
if (!(val = ap_get_remote_host(c,
c->base_server->lookup_defaults,
   REMOTE_NOLOOKUP, NULL)))
apr_cpystrn(ws->client, c->client_ip, sizeof(ws->client));
else
apr_cpystrn(ws->client, val, sizeof(ws->client));

In all cases we should have the value for (c) where (r) is passed,
whether r was fully populated yet or not.

If we want to be more robust, have a look at the current 2.4.x
implementation of ap_get_useragent_host() in core.c...

if (r->useragent_addr == conn->client_addr) {
return ap_get_remote_host(conn, r->per_dir_config, type, str_is_ip);
}

Perhaps this would be better...

if (!r->useragent_addr || (r->useragent_addr == conn->client_addr)) {
return ap_get_remote_host(conn, r->per_dir_config, type, str_is_ip);
}


Re: [PATCH 54255] mod_deflate adjusts the headers "too late"

2016-05-03 Thread William A Rowe Jr
Keep in mind that such changes alter the expected behavior that all of the
third party module authors have worked around.  This doesn't seem like
a suitable change for 2.4.x maintenance branch but a good improvement
for trunk, looking forwards.

On Tue, May 3, 2016 at 9:42 AM, Michael Kaufmann 
wrote:

> Hi,
>
> It would be great if somebody could have a look at the proposed patch.
>
> The problem: Request headers are adjusted too late (in the input filter).
> Proposed solution: Adjust the request headers in the filter init function.
>
> https://bz.apache.org/bugzilla/show_bug.cgi?id=54255
>
> Regards,
> Michael
>
>


Re: Detecting client aborts and stream resets

2016-05-03 Thread William A Rowe Jr
On Tue, May 3, 2016 at 9:31 AM, Michael Kaufmann 
wrote:

> Hi all,
>
> a content generator module can detect client aborts and stream resets
> while it reads the request body. But how can it detect this afterwards,
> while the response is being generated?
>
> This is important for HTTP/2, because the client may reset a stream, and
> mod_http2 needs to wait for the content generator to finish. Therefore the
> content generator should stop generating the response when it is no longer
> needed.
>
> Is there any API for this? The "conn_rec->aborted" flag exists, but which
> Apache function sets this flag?
>
> If there is no API, maybe an optional function for mod_http2 would be a
> solution.
>

Nope - an optional function in mod_http2 is too special case, generators
must remain protocol (socket or other transport) agnostic.

In the case of mod_cache'd content, the generator can't quit, it is already
populating a cache, which means you'll generate an invalid cached object.

But if you knew that the cache module wasn't collecting info, r->c->aborted
tells you if anyone is still listening, right?


Re: mod_proxy_hcheck ouch in Windows

2016-04-29 Thread William A Rowe Jr
See the usage of APR_THREAD_FUNC macro.
On Apr 28, 2016 9:53 PM, "Gregg Smith"  wrote:

> This one I cannot figure out
>
> .\mod_proxy_hcheck.c(925) : error C2440: 'function' : cannot convert from
> 'void *(__cdecl *)(apr_thread_t *,void *)' to 'apr_thread_start_t'
>
> - Gregg -
>


Re: svn commit: r1741419 [1/4] - in /httpd/httpd/branches/2.4.x: ./ modules/http2/

2016-04-28 Thread William A Rowe Jr
On Thu, Apr 28, 2016 at 11:57 AM, Gregg Smith  wrote:

> On 4/28/2016 5:43 AM, ic...@apache.org wrote:
>
>> Author: icing
>> Date: Thu Apr 28 12:43:02 2016
>> New Revision: 1741419
>>
>> URL: http://svn.apache.org/viewvc?rev=1741419=rev
>> Log:
>> mod_http2: backport of 1.5.2 to 2.4.x
>>
>> Added:
>>  httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.c
>>- copied, changed from r1739303,
>> httpd/httpd/trunk/modules/http2/h2_bucket_beam.c
>>  httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.h
>>- copied, changed from r1739303,
>> httpd/httpd/trunk/modules/http2/h2_bucket_beam.h
>>
>
> Hi Stefan,
>
> FYI
> h2_bucket_beam.c(338) : warning C4003: not enough actual parameters for
> macro 'APLOGNO'
> Maybe you can recycle one from one of the removed files, not sure about
> that however since they were used in 2.4.20.


You can't recycle a released identifier.


Re: svn commit: r1741310 - in /httpd/httpd/trunk: modules/http2/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/ server/mpm/winnt/ server/mpm/worker/

2016-04-28 Thread William A Rowe Jr
On Thu, Apr 28, 2016 at 4:37 AM, Yann Ylavic <ylavic@gmail.com> wrote:

> On Thu, Apr 28, 2016 at 4:30 AM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > On Wed, Apr 27, 2016 at 6:16 PM, Yann Ylavic <ylavic@gmail.com>
> wrote:
> >>
> >> On Wed, Apr 27, 2016 at 8:41 PM,  <wr...@apache.org> wrote:
> >> >   Ensure http2 follows http in the meaning of
> >> >   status WRITE (meaning 'in the request processing
> >> >   phase' even if still consuming the request body,
> >> >   not literally in a 'now writing' state).
> >>
> >> This is indeed consistent with how we report http1 state currently,
> >> but maybe it could be more intuitive to report READ until the body is
> >> consumed in http1 rather than changing http2?
> >> Unless we want to minimize scoreboard updates, for performance
> reasons...
> >
> >
> > Well, we always want to be considerate of performance.
> >
> > That said, we absolutely must not change the semantic meanings of
> > the server-status results on the maintenance branch.  Change those
> > meanings on trunk for a future 2.6/3.0?  Sure... but 2.4.x needs to
> > behave as 2.4.x has behaved from the start.
>
> Agreed, we can't change it for http1 in 2.4.x, but maybe http2 has
> other considerations WRT reading/writing on the master connection,
> reporting READ or WRITE for http2 is not really a compatibility issue
> IMHO.
>

It is, yes, the server-status has well understood semantics and changing
them on a released branch is never acceptable. The master worker already
has special semantics which are new. No reason to change the slave
workers between http/1 and http/2 as they are doing the same things.


> >> >   Ensure a number of MPMs and the h2 connection io
> >> >   no longer clobber the request status line during
> >> >   state-only changes.  While at it, clean up some
> >> >   very ugly formatting and unnecessary decoration,
> >> >   and avoid the wordy _from_conn() flavor when we
> >> >   are not passing a connection_rec.
> >>
> >> Why ap_update_child_status_from_conn(), given a real or NULL c, would
> >> clobber the request line?
> >
> >
> > Given a real conn_rec record, we have no request line.  Therefore the
> result
> > is NULL.  There is no purpose in modifying the conn_rec related fields
> once
> > correctly recorded.  The next chance they have for being modified in any
> > meaningful way is during an ssl renegotiation or the processing of the
> Host:
> > and X-F-F: headers during the read_request phase.
>
> That's more an optimization (not rewrite existing values with the same
> ones), but driven by how http1 in *2.4.x* works currently (this relies
> on the same worker handling the connection for the lifetime of the
> request).
>



> When SUSPENDED is used (trunk does it already at several places) we
> must overwrite the fields with actual values.
>

SUSPENDED within a request cannot be supported on 2.4.x as modules
are allowed to make thread storage and thread-local storage assumptions.
E.g. it breaks modules in an unexpected manner, and it is our responsibility
to inform users of this by bumping the version major.  This wasn't seen as
a big of an issue when we introduced event and launched 2.2.x, because
far more third party modules are request handlers, and are not connection-
based.  But we still bumped the version minor when we allowed the
connection to float between threads. This wasn't changed in maintained
release branch.

On trunk, it will have to become the responsibility of the MPM to restore
the right score slot state before reentering the processing loop on a given
worker thread, multiple times per connection or request, as that MPM
sends requests across threads.


> >> The request line is untouched unless a non-NULL r is passed to
> >> update_child_status_internal(), and ap_update_child_status_from_conn()
> >> sets r to NULL.
> >
> > That was incorrect - to the extend that you've changed it since 2.4.1,
> > such a change must be reverted.
>
> I meant ap_update_child_status_from_conn() gives a NULL r to
> update_child_status_internal(), so it won't change the existing
> request line (as opposed to setting it to the NULL value in the
> scoreboard).


What I meant is that the request line *MUST* be touched - wiped
clean - for any status_from_conn(c) request.

When we say we preserve the most recent state of the worker in
the scoreboard, that old request line is gone.  Every time we are
doing a conn-based refresh of that worker status, the req-based
data is no longer relevant and needs to go away.

> At the

Re: svn commit: r1741310 - in /httpd/httpd/trunk: modules/http2/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/ server/mpm/winnt/ server/mpm/worker/

2016-04-27 Thread William A Rowe Jr
On Wed, Apr 27, 2016 at 9:30 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Wed, Apr 27, 2016 at 6:16 PM, Yann Ylavic <ylavic@gmail.com> wrote:
>
>>
>> So now, either we choose to not report workers' activity in between
>> requests (there are just multi-purpose workers/request-pollers after
>> all), or we save some "last request" informations in each conn_rec and
>> restore them on the worker (score) handling the connection at each
>> time (this is more reporting the workers' activity than the requests
>> lines, which will pass from one worker to the other, even possibly
>> duplicating requests lines with low activity).
>>
>
> If that slot does other work, it should be recorded.  But while that slot
> sits unused, the last activity for that score slot must be preserved.
>

Just to clarify the event case... our server is not free-threaded.  Once we
are in a request, we are threadbound.  The following request on that same
worker is likely to jump threads, and it's fine, my brief testing today
seems
to confirm, but I presented the patch to the followers of PR 59333 for
further
review and testing.

This will likely change in 3.0 (no, we aren't going to make requests
free-threaded without bumping the version major - that's too drastic
a behavior change.)  Once a *request* is jumping from thread to
thread, as opposed to the underlying connection, we are going to
have to rethink how the scoreboard presents that information and
how it is refreshed.


Re: svn commit: r1741310 - in /httpd/httpd/trunk: modules/http2/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/ server/mpm/winnt/ server/mpm/worker/

2016-04-27 Thread William A Rowe Jr
On Wed, Apr 27, 2016 at 6:16 PM, Yann Ylavic  wrote:

> I was offline today so couldn't comment on the different messages on
> the subject, so I'll try to summarize (here) my understanding, so
> far...
>
> On Wed, Apr 27, 2016 at 8:41 PM,   wrote:
> > Author: wrowe
> > Date: Wed Apr 27 18:41:49 2016
> > New Revision: 1741310
> >
> > URL: http://svn.apache.org/viewvc?rev=1741310=rev
> > Log:
> >
> >   Ensure http2 follows http in the meaning of
> >   status WRITE (meaning 'in the request processing
> >   phase' even if still consuming the request body,
> >   not literally in a 'now writing' state).
>
> This is indeed consistent with how we report http1 state currently,
> but maybe it could be more intuitive to report READ until the body is
> consumed in http1 rather than changing http2?
> Unless we want to minimize scoreboard updates, for performance reasons...
>

Well, we always want to be considerate of performance.

That said, we absolutely must not change the semantic meanings of
the server-status results on the maintenance branch.  Change those
meanings on trunk for a future 2.6/3.0?  Sure... but 2.4.x needs to
behave as 2.4.x has behaved from the start.


> >   Ensure a number of MPMs and the h2 connection io
> >   no longer clobber the request status line during
> >   state-only changes.  While at it, clean up some
> >   very ugly formatting and unnecessary decoration,
> >   and avoid the wordy _from_conn() flavor when we
> >   are not passing a connection_rec.
>
> Why ap_update_child_status_from_conn(), given a real or NULL c, would
> clobber the request line?
>

Given a real conn_rec record, we have no request line.  Therefore the result
is NULL.  There is no purpose in modifying the conn_rec related fields once
correctly recorded.  The next chance they have for being modified in any
meaningful way is during an ssl renegotiation or the processing of the
Host:
and X-F-F: headers during the read_request phase.


> The request line is untouched unless a non-NULL r is passed to
> update_child_status_internal(), and ap_update_child_status_from_conn()
> sets r to NULL.
>

That was incorrect - to the extend that you've changed it since 2.4.1,
such a change must be reverted.

At the time we process a new connection (before, and again after we
have parsed any SNI host handshake) - there is NO REQUEST.  Any
lingering request value must be blanked out at that time.  Once the
request URI is parsed we again invoke update_child_status, this time
with the request_rec with a now-known request string.


> AIUI, what sets the request line to NULL in mpm_worker is when
> ap_read_request() fails (mostly after connection closed remotely or
> keep-alive timeout, actually any empty/NULL r->the_request from
> read_request_line() would do it).
> This may also happen in mpm_event but since the worker (scoreboard
> entry) has probably changed in between, this is possibly less visible.
>

You don't understand it.

Envision this sequence, which is how the scoreboard was designed;

Initialization -> entire score record is voided

Connection -> score entry tagged READ, what is known of the
connecting client and the target vhost is recorded

Connection SNI -> score entry updated with new target vhost

Request read -> score entry again updated with new target vhost,
the request identifier is updated, score entry tagged WRITE

Request body is read, Response body is prepared

Request complete -> score entry tagged LOGGING, and NO
other fields need to be updated

Request logged, left in keepalive state -> score entry tagged
KEEPALIVE, NO other fields need to be updated

Connection closed -> score entry tagged IDLE, NO OTHER
FIELDS SHOULD BE UPDATED until the worker is reused
(even in the case of event MPM).




So how about:
>
> Index: server/protocol.c
> ===
> --- server/protocol.c(revision 1741108)
> +++ server/protocol.c(working copy)
> @@ -1093,13 +1093,14 @@ request_rec *ap_read_request(conn_rec *conn)
>  access_status = r->status;
>  r->status = HTTP_OK;
>  ap_die(access_status, r);
> -ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
> +ap_update_child_status(conn->sbh, SERVER_BUSY_LOG,
> +   r->the_request ? r : NULL);
>

That could be updated, or we could equally pass NULL always, but it doesn't
impact the correctness of the proposed backport.


>  ap_run_log_transaction(r);
>  r = NULL;
>  apr_brigade_destroy(tmp_bb);
>  goto traceout;
>  case HTTP_REQUEST_TIME_OUT:
> -ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
> +ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, NULL);
>  if (!r->connection->keepalives)
>  ap_run_log_transaction(r);
>  apr_brigade_destroy(tmp_bb);
> ?
>
>
> Regarding 

h2 request processing score behavior...

2016-04-27 Thread William A. Rowe Jr.
Within h2_task.c, we have

static apr_status_t h2_task_process_request(h2_task *task, conn_rec *c)
{
const h2_request *req = task->request;
conn_state_t *cs = c->cs;
request_rec *r;

ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
  "h2_task(%s): create request_rec", task->id);
r = h2_request_create_rec(req, c);
if (r && (r->status == HTTP_OK)) {
ap_update_child_status(c->sbh, SERVER_BUSY_READ, r);

if (cs) {
cs->state = CONN_STATE_HANDLER;
}
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
  "h2_task(%s): start process_request", task->id);
ap_process_request(r);

Contrast this to http_core.c...

ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ, c);
while ((r = ap_read_request(c)) != NULL) {
apr_interval_time_t keep_alive_timeout =
r->server->keep_alive_timeout;

/* To preserve legacy behaviour, use the keepalive timeout from the
 * base server (first on this IP:port) when none is explicitly
 * configured on this server.
 */
if (!r->server->keep_alive_timeout_set) {
keep_alive_timeout = c->base_server->keep_alive_timeout;
}

c->keepalive = AP_CONN_UNKNOWN;
/* process the request if it was read without error */

ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
if (r->status == HTTP_OK) {
if (cs)
cs->state = CONN_STATE_HANDLER;
ap_process_request(r);

It certainly seems this slave worker needs to be in
SERVER_BUSY_WRITE at the time we run the
process_request logic to correspond to the expected
behavior of status analysis tools, so will update this
logic accordingly.

The next issue I do not understand, this seems broken
whether it is happening in the master or slave worker...

static apr_status_t pass_out(apr_bucket_brigade *bb, void *ctx)
{
pass_out_ctx *pctx = ctx;
conn_rec *c = pctx->c;
apr_status_t status;
apr_off_t bblen;

if (APR_BRIGADE_EMPTY(bb)) {
return APR_SUCCESS;
}

ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_WRITE, c);
apr_brigade_length(bb, 0, );
h2_conn_io_bb_log(c, 0, APLOG_TRACE2, "master conn pass", bb);
status = ap_pass_brigade(c->output_filters, bb);

This does whack the request line so I've corrected the logic
accordingly by not passing the conn_rec, but if this is only
invoked in the slave worker, correcting the first issue above
seems to make this redundant (and offers a small optimization
to simply remove it.)

The only other issue I see is in h2_session.c...

ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ,
c);
if (!h2_is_acceptable_connection(c, 1)) {
update_child_status(session, SERVER_BUSY_READ,
"inadequate security");
h2_session_shutdown(session,
NGHTTP2_INADEQUATE_SECURITY, NULL, 1);
}
else {
update_child_status(session, SERVER_BUSY_READ, "init");

Can h2_is_acceptable_connection update the connection's
virtual host?  If it does, we want to repeat the _from_conn
status refresh to correct the vhost before setting the status
to "init".  I have not touched this code.


Re: svn commit: r1739876 - /httpd/httpd/branches/2.4.x/STATUS

2016-04-27 Thread William A Rowe Jr
So just to summarize how in h2 there may be issues...

modules/http2/h2_conn_io.c:ap_update_child_status_from_conn(c->sbh,
SERVER_BUSY_WRITE, c);
  - this will trash any request line info

modules/http2/h2_task.c:ap_update_child_status(c->sbh,
SERVER_BUSY_READ, r);
  - is only appropriate if this is reading the request body... if we are
still in the read_request hook then the request headers and values such as
r->useragent_ip etc aren't set up yet.

modules/http2/h2_request.c:ap_update_child_status(c->sbh,
SERVER_BUSY_LOG, r);
  - good

modules/http2/h2_session.c:
 ap_update_child_status_descr(session->c->sbh, status, session->status);
  - looks fine, provided that session->status isn't "\0"

modules/http2/h2_session.c:
 ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ, c);
  - this is the correct logic if we are reading the request headers

modules/http2/h2_session.c:
 ap_update_child_status(session->c->sbh, SERVER_BUSY_READ, NULL);
  - this looks a bit odd, note that the request / descr is not updated here
so hopefully it isn't ready/was updated already

modules/http2/h2_session.c:
 ap_update_child_status(session->c->sbh, SERVER_BUSY_WRITE, NULL);
  - note that the request / descr is not updated here so hopefully it was
updated elsewhere already


On Wed, Apr 27, 2016 at 9:15 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> For example... in event.c;
>
> if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
> int not_complete_yet;
>
> ap_update_child_status_from_conn(sbh, SERVER_BUSY_WRITE, c);
>
> Yuck, that just trashed the score for that connection.
>
> We do it again here;
>
> if (cs->pub.state == CONN_STATE_LINGER) {
> start_lingering_close_blocking(cs);
> }
> else if (cs->pub.state == CONN_STATE_CHECK_REQUEST_LINE_READABLE) {
> ap_update_child_status_from_conn(sbh, SERVER_BUSY_KEEPALIVE, c);
>
> I'm sure we can find similar examples, I'm still going through a pretty
> broad grep of the trunk sources.
>
>
>
>
>
> On Wed, Apr 27, 2016 at 8:43 AM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
>
>> I'm not certain the error is -in- scoreboard.c.
>>
>> Suspecting that we used to call ap_update_child_status[_xxx] with the
>> NULL req/conn rec pointer when we were idling for keepalive or ending the
>> request.
>> It seems we may now be calling it in these situations with a now-NULL
>> req_rec
>> and the dying/idling conn_rec pointer, causing the request value to null
>> out (or
>> calling it with a descr pointer to "\0").
>>
>>
>> On Wed, Apr 27, 2016 at 7:54 AM, Jim Jagielski <j...@jagunet.com> wrote:
>>
>>> +1...
>>>
>>> I had just applied the patch in STATUS w/o seeing this thread.
>>> Best to just revert this all and get back to 2.4.18 behavior
>>> (and code)
>>>
>>> > On Apr 27, 2016, at 12:40 AM, Stefan Eissing <
>>> stefan.eiss...@greenbytes.de> wrote:
>>> >
>>> >
>>> >
>>> >> Am 27.04.2016 um 03:53 schrieb William A Rowe Jr <wr...@rowe-clan.net
>>> >:
>>> >>
>>> >>
>>> >> My gut instinct is to propose scoreboard.c for a full svn revert back
>>> >> to a working state,
>>> >
>>> > I did not realize that it is that deep of a mess now. Please revert,
>>> uncomment the new calls use in http2 and I'll find another approach there.
>>> >
>>> > It seems that there are a lot of expectations on how this part of the
>>> server behaves, but no way to verify any changes/additions against them for
>>> a newbie like me.
>>> >
>>> > Since this whole h2 thing is experimental and needs to evolve based on
>>> user feedback, it better either use the extension possibilities of
>>> mod_status or come up with a new, separate approach that allows frequent
>>> changes.
>>> >
>>> > -Stefan
>>>
>>>
>>
>


Re: svn commit: r1739876 - /httpd/httpd/branches/2.4.x/STATUS

2016-04-27 Thread William A Rowe Jr
For example... in event.c;

if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
int not_complete_yet;

ap_update_child_status_from_conn(sbh, SERVER_BUSY_WRITE, c);

Yuck, that just trashed the score for that connection.

We do it again here;

if (cs->pub.state == CONN_STATE_LINGER) {
start_lingering_close_blocking(cs);
}
else if (cs->pub.state == CONN_STATE_CHECK_REQUEST_LINE_READABLE) {
ap_update_child_status_from_conn(sbh, SERVER_BUSY_KEEPALIVE, c);

I'm sure we can find similar examples, I'm still going through a pretty
broad grep of the trunk sources.





On Wed, Apr 27, 2016 at 8:43 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> I'm not certain the error is -in- scoreboard.c.
>
> Suspecting that we used to call ap_update_child_status[_xxx] with the
> NULL req/conn rec pointer when we were idling for keepalive or ending the
> request.
> It seems we may now be calling it in these situations with a now-NULL
> req_rec
> and the dying/idling conn_rec pointer, causing the request value to null
> out (or
> calling it with a descr pointer to "\0").
>
>
> On Wed, Apr 27, 2016 at 7:54 AM, Jim Jagielski <j...@jagunet.com> wrote:
>
>> +1...
>>
>> I had just applied the patch in STATUS w/o seeing this thread.
>> Best to just revert this all and get back to 2.4.18 behavior
>> (and code)
>>
>> > On Apr 27, 2016, at 12:40 AM, Stefan Eissing <
>> stefan.eiss...@greenbytes.de> wrote:
>> >
>> >
>> >
>> >> Am 27.04.2016 um 03:53 schrieb William A Rowe Jr <wr...@rowe-clan.net
>> >:
>> >>
>> >>
>> >> My gut instinct is to propose scoreboard.c for a full svn revert back
>> >> to a working state,
>> >
>> > I did not realize that it is that deep of a mess now. Please revert,
>> uncomment the new calls use in http2 and I'll find another approach there.
>> >
>> > It seems that there are a lot of expectations on how this part of the
>> server behaves, but no way to verify any changes/additions against them for
>> a newbie like me.
>> >
>> > Since this whole h2 thing is experimental and needs to evolve based on
>> user feedback, it better either use the extension possibilities of
>> mod_status or come up with a new, separate approach that allows frequent
>> changes.
>> >
>> > -Stefan
>>
>>
>


Re: svn commit: r1739876 - /httpd/httpd/branches/2.4.x/STATUS

2016-04-27 Thread William A Rowe Jr
I'm not certain the error is -in- scoreboard.c.

Suspecting that we used to call ap_update_child_status[_xxx] with the
NULL req/conn rec pointer when we were idling for keepalive or ending the
request.
It seems we may now be calling it in these situations with a now-NULL
req_rec
and the dying/idling conn_rec pointer, causing the request value to null
out (or
calling it with a descr pointer to "\0").


On Wed, Apr 27, 2016 at 7:54 AM, Jim Jagielski <j...@jagunet.com> wrote:

> +1...
>
> I had just applied the patch in STATUS w/o seeing this thread.
> Best to just revert this all and get back to 2.4.18 behavior
> (and code)
>
> > On Apr 27, 2016, at 12:40 AM, Stefan Eissing <
> stefan.eiss...@greenbytes.de> wrote:
> >
> >
> >
> >> Am 27.04.2016 um 03:53 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
> >>
> >>
> >> My gut instinct is to propose scoreboard.c for a full svn revert back
> >> to a working state,
> >
> > I did not realize that it is that deep of a mess now. Please revert,
> uncomment the new calls use in http2 and I'll find another approach there.
> >
> > It seems that there are a lot of expectations on how this part of the
> server behaves, but no way to verify any changes/additions against them for
> a newbie like me.
> >
> > Since this whole h2 thing is experimental and needs to evolve based on
> user feedback, it better either use the extension possibilities of
> mod_status or come up with a new, separate approach that allows frequent
> changes.
> >
> > -Stefan
>
>


Re: svn commit: r1741239 - /httpd/httpd/branches/2.4.x/STATUS

2016-04-27 Thread William A Rowe Jr
On Wed, Apr 27, 2016 at 7:20 AM,  wrote:

> Author: jim
> Date: Wed Apr 27 12:20:57 2016
> New Revision: 1741239
>
> URL: http://svn.apache.org/viewvc?rev=1741239=rev
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1741239=1741238=1741239=diff
>
> ==
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Wed Apr 27 12:20:57 2016
> @@ -114,6 +114,18 @@ RELEASE SHOWSTOPPERS:
>  PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
>[ start all new proposals below, under PATCHES PROPOSED. ]
>
> +  *) scoreboard/status: Keep previous worker connection/request data when
> idle as
> + prior to 2.4.20.
> + trunk patch: http://svn.apache.org/r1739008
> + trunk patch: http://svn.apache.org/r1739146
> + trunk patch: http://svn.apache.org/r1739151
> + trunk patch: http://svn.apache.org/r1739193
> + 2.4.x patch: trunk works (modulo CHANGES) or
> +
> http://home.apache.org/~ylavic/patches/httpd-2.4.x-scoreboard_preserve-v2.patch
> + +1: ylavic, rpluem, jim
> + ylavic: diff with 2.4.18 after this merge:
> +
> http://home.apache.org/~ylavic/patches/scoreboard-2.4.18.diff


Hi Jim, see https://bz.apache.org/bugzilla/show_bug.cgi?id=59333

This doesn't seem to be 'fixed' yet.  I don't mind incremental patches
moving forward, but we still appear to be zeroing out the request element
during keep-alive activity, which is the usual precursor to closing the
connection.


Re: svn commit: r1739876 - /httpd/httpd/branches/2.4.x/STATUS

2016-04-26 Thread William A Rowe Jr
On Tue, Apr 26, 2016 at 5:01 PM, Yann Ylavic <ylavic@gmail.com> wrote:

> On Tue, Apr 26, 2016 at 10:48 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > On Tue, Apr 26, 2016 at 3:47 PM, William A Rowe Jr <wr...@rowe-clan.net>
> > wrote:
> >>
> >> Reviewing the applicable patch, the client, request and protocol now
> >> appear
> >
> > ... the client, vhost and protocol now appear ...
> >
> >> to persist for idle connections, but the request is still NULL?
> >>
> >> Is this by design? Or is something missing from the patch?
>
> Is it different behaviour than 2.4.18 (it shouldn't)?
>
> We are discussing different issues/solutions in PR 59333...
>

AIUI the previous behavior was to retain the prior request until the
new keep-alive request began processing the post-read-request
phase, until that point the previous request on that pipeline should
still appear in the connection's status.

I'm now already following that thread since this afternoon and have
to compare notes against 2.4.17-.18-.20-post-patch behaviors.

My gut instinct is to propose scoreboard.c for a full svn revert back
to a working state, but will help if folks demonstrate some effort
toward preserving ABI and functional state compatibility... at least
over the next few days.

Cheers,

Bill


Re: svn commit: r1739876 - /httpd/httpd/branches/2.4.x/STATUS

2016-04-26 Thread William A Rowe Jr
On Tue, Apr 26, 2016 at 3:47 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> Reviewing the applicable patch, the client, request and protocol now appear
>

... the client, vhost and protocol now appear ...


> to persist for idle connections, but the request is still NULL?
>
> Is this by design? Or is something missing from the patch?
>
> On Tue, Apr 19, 2016 at 4:19 AM, <rpl...@apache.org> wrote:
>
>> Author: rpluem
>> Date: Tue Apr 19 09:19:44 2016
>> New Revision: 1739876
>>
>> URL: http://svn.apache.org/viewvc?rev=1739876=rev
>> Log:
>> * Vote
>>
>> Modified:
>> httpd/httpd/branches/2.4.x/STATUS
>>
>> Modified: httpd/httpd/branches/2.4.x/STATUS
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1739876=1739875=1739876=diff
>>
>> ==
>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>> +++ httpd/httpd/branches/2.4.x/STATUS Tue Apr 19 09:19:44 2016
>> @@ -191,7 +191,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>   trunk patch: http://svn.apache.org/r1739193
>>   2.4.x patch: trunk works (modulo CHANGES) or
>>
>> http://home.apache.org/~ylavic/patches/httpd-2.4.x-scoreboard_preserve-v2.patch
>> - +1: ylavic
>> + +1: ylavic, rpluem
>>   ylavic: diff with 2.4.18 after this merge:
>>
>> http://home.apache.org/~ylavic/patches/scoreboard-2.4.18.diff
>>
>>
>>
>>
>


Re: svn commit: r1739876 - /httpd/httpd/branches/2.4.x/STATUS

2016-04-26 Thread William A Rowe Jr
Reviewing the applicable patch, the client, request and protocol now appear
to persist for idle connections, but the request is still NULL?

Is this by design? Or is something missing from the patch?

On Tue, Apr 19, 2016 at 4:19 AM,  wrote:

> Author: rpluem
> Date: Tue Apr 19 09:19:44 2016
> New Revision: 1739876
>
> URL: http://svn.apache.org/viewvc?rev=1739876=rev
> Log:
> * Vote
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1739876=1739875=1739876=diff
>
> ==
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Tue Apr 19 09:19:44 2016
> @@ -191,7 +191,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>   trunk patch: http://svn.apache.org/r1739193
>   2.4.x patch: trunk works (modulo CHANGES) or
>
> http://home.apache.org/~ylavic/patches/httpd-2.4.x-scoreboard_preserve-v2.patch
> - +1: ylavic
> + +1: ylavic, rpluem
>   ylavic: diff with 2.4.18 after this merge:
>
> http://home.apache.org/~ylavic/patches/scoreboard-2.4.18.diff
>
>
>
>


Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)

2016-04-19 Thread William A Rowe Jr
On Tue, Apr 19, 2016 at 10:59 AM, Stefan Eissing <
stefan.eiss...@greenbytes.de> wrote:

>
> > Am 19.04.2016 um 17:47 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
> >
> > I agree with your analysis, "h2" is not an upgrade candidate.
> >
> > "h2c" is an upgrade candidate.
> >
> > This isn't even an HTTP/2 issue (unless the working group reverses
> themselves
> > on accepting Upgrade: h2 protocol switching), until we accept Upgrade:
> h2 we
> > should be dropping h2 from the server Upgrade: response header.  But do
> let
> > us know what the wg feedback is.
>
> While I do not feel strongly about this feature, I'd like to add that the
> "Upgrade: h2" is sent out as that very mechanism is available to a client.
> And I do not feel strongly because I do not know of a client that might be
> able to use it. It is just the result of a sane software architecture that
> this has become visible.
>
> We would probably not be talking about this if some Javascript client
> implementation had not consciously decided to freak out on *any* Upgrade:
> header from the server.
>
> If http-wg thinks that it should not be visible, I'll add the extra 'if'
> to our implementation.
>

Skimming the responses, they just keep getting more and more amusing, and
shining a candle to the absurdity of keeping this non-sequitur request
response.

Could you go ahead and add that conditional?

We still haven't determined if the "reply Upgrade: once, then pretend we
didn't"
is valid HTTP/1.1, which I read that it is not.  Need to come back to that
topic.

Cheers,

Bill


Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)

2016-04-19 Thread William A Rowe Jr
On Tue, Apr 19, 2016 at 11:26 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Tue, Apr 19, 2016 at 11:04 AM, Jacob Champion <champio...@gmail.com>
> wrote:
>
>> On 04/19/2016 08:47 AM, William A Rowe Jr wrote:
>> > I agree with your analysis, "h2" is not an upgrade candidate.
>> >
>> > "h2c" is an upgrade candidate.
>>
>> Is an h2c upgrade allowed over an HTTP/1.1+TLS connection? 7540 seems to
>> hint that it's not ("The 'h2c' string is reserved from the ALPN
>> identifier space but describes a protocol that does not use TLS"), but I
>> can't find any "MUST NOT" language.
>>
>
> Not according to the HTTP/2 wg, they declared that SNI ALPN "h2"
> connections were the only way to establish TLS h2 connections, and
> that h2c is plaintext, by their definition.
>

(There is a suggestion that HTTP/2 wg proscribed the *client* behavior,
and that server implementors are free to offer h2c over https upgrade,
or h2 over http upgrade, but if there is no client that would consume
that behavior... well...)


Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)

2016-04-19 Thread William A Rowe Jr
On Tue, Apr 19, 2016 at 11:04 AM, Jacob Champion <champio...@gmail.com>
wrote:

> On 04/19/2016 08:47 AM, William A Rowe Jr wrote:
> > I agree with your analysis, "h2" is not an upgrade candidate.
> >
> > "h2c" is an upgrade candidate.
>
> Is an h2c upgrade allowed over an HTTP/1.1+TLS connection? 7540 seems to
> hint that it's not ("The 'h2c' string is reserved from the ALPN
> identifier space but describes a protocol that does not use TLS"), but I
> can't find any "MUST NOT" language.
>

Not according to the HTTP/2 wg, they declared that SNI ALPN "h2"
connections were the only way to establish TLS h2 connections, and
that h2c is plaintext, by their definition.


Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)

2016-04-19 Thread William A Rowe Jr
I agree with your analysis, "h2" is not an upgrade candidate.

"h2c" is an upgrade candidate.

This isn't even an HTTP/2 issue (unless the working group reverses
themselves
on accepting Upgrade: h2 protocol switching), until we accept Upgrade: h2 we
should be dropping h2 from the server Upgrade: response header.  But do let
us know what the wg feedback is.


On Tue, Apr 19, 2016 at 7:47 AM, Michael Kaufmann 
wrote:

> Hi,
>
> you may already know that HTTP/2 clients use ALPN to advertise support for
> HTTP/2 when TLS is used.
>
> The issue is this: When mod_http2 is enabled, Apache sends an "Upgrade:
> h2" response header to clients that have *not* advertised support for
> HTTP/2 (clients that speak only HTTP/1.x).
>
> I think that this is wrong, because of this sentence in RFC 7540:
>
>> A server MUST ignore an "h2" token in an Upgrade header field. Presence
>> of a token with "h2" implies HTTP/2 over TLS, which is instead negotiated
>> as described in Section 3.3.
>>
>
> What do you think about this issue, and what do you think about the
> attached patch in bug 59311?
>
> Regards,
> Michael
>
>


Re: allow newlines in T_ESCAPE_LOGITEM?

2016-04-18 Thread William A Rowe Jr
That doesn't work because it's no longer atomic, those log lines end up
fanned out among all the other worker threads doing the same thing.  Very
hard to parse.

On Mon, Apr 18, 2016 at 3:08 PM, Stefan Fritsch  wrote:

> On Monday 18 April 2016 10:42:23, Eric Covener wrote:
> > I'm won over and not tinkering with this FWIW.
>
> Maybe add a utility function that splits a message on \n and calls
> ap_log_rerror for each line?
>


Re: New segfault with 2.4.20 with mod_perl

2016-04-14 Thread William A Rowe Jr
The defect appears to be in t/protocol/TestProtocol/pseudo_http.pm...

First, the handler is registered using

  PerlProcessConnectionHandler TestProtocol::pseudo_http

so its activities are outside of the request handling phase.

Note that this logic has been broken, for a long time;

   2.4.1>
  
  Order Deny,Allow
  Allow from @servername@
  
  

Where @servername@ is a hostname, this module defect was
identified in version 2.4.20 when we began using the per-req
hostname in comparison (based on r->useragent_addr, which
is obviously is null during part of the read_request phase).

But this module using mod_access_compat during the connection
phase has been broken for much longer, since Allow from {ip-addr}
would already have failed since 2.4.1 was released, due to the
same null r->useragent_addr.

Effectively, mod_access_compat.c never supported per-connection
IP addresses since it was added.  The fact that it supported
per-connection hostname comparison was a quirk, and that the
pseudo_http tests only looked at hostname and not ip comparisons
was an oversight.

But the module will fail in other manners if attempting to use
http request_rec processing since that record is never fleshed
out with the proper read/post_read request hook phases.

My thought is to simply decouple access_compat from this
module test... opinions?

See also; https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=820824;msg=5


On Thu, Apr 14, 2016 at 11:55 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> We can be more vigilant about unexpectedly null values, however...
>
> how are you running request processing in the connection callback
> of mod_perl?  That makes no sense, and probably signals a deeper
> logic error.
>
> The access checker is configured per-dir, so until the request rec
> is completely initialized during read_request, this doesn't make
> much sense to me (full backtrace .. including frames #6-#10, for
> those who are curious...)
>
> Either the callback list registered for modperl_callback_connection,
> or the Perl_runops_standard, or the Perl_pp_entersub invoking the
> run_access_checker hook seem the most suspect here.
>
> #0  apr_getnameinfo (hostname=hostname@entry=0x7fd4461ee368, sockaddr=0x0, 
> flags=flags@entry=0)
> at /tmp/buildd/apr-1.5.2/network_io/unix/sockaddr.c:663
> #1  0x55feaf0f513a in ap_get_useragent_host (r=r@entry=0x7fd4461ee0a0, 
> type=type@entry=3,
> str_is_ip=str_is_ip@entry=0x7fd44740c9c4) at core.c:990
> #2  0x7fd4519d7212 in find_allowdeny (r=r@entry=0x7fd4461ee0a0, 
> method=method@entry=0, a=,
> a=) at mod_access_compat.c:279
> #3  0x7fd4519d74b2 in check_dir_access (r=0x7fd4461ee0a0) at 
> mod_access_compat.c:332
> #4  0x55feaf0f8f30 in ap_run_access_checker (r=r@entry=0x7fd4461ee0a0) at 
> request.c:87
> #5  0x7fd448a6f7dd in XS_Apache2__RequestRec_run_access_checker 
> (my_perl=0x55feb2964a20, cv=)
> at HookRun.c:235
> #6  0x7fd44f5f7e6a in Perl_pp_entersub () from 
> /usr/lib/x86_64-linux-gnu/libperl.so.5.22
> #7  0x7fd44f5f0ca6 in Perl_runops_standard () from 
> /usr/lib/x86_64-linux-gnu/libperl.so.5.22
> #8  0x7fd44f575f06 in Perl_call_sv () from 
> /usr/lib/x86_64-linux-gnu/libperl.so.5.22
> #9  0x7fd44f91ec28 in modperl_callback 
> (my_perl=my_perl@entry=0x55feb2964a20, handler=0x7fd4461f2750,
> p=p@entry=0x7fd4461f2028, r=r@entry=0x0, s=s@entry=0x7fd453ddc628, 
> args=0x55feb3beebd0)
> at modperl_callback.c:100
> #10 0x7fd44f91f576 in modperl_callback_run_handlers (idx=0, 
> type=type@entry=1, r=r@entry=0x0,
> c=, s=0x7fd453ddc628, pconf=pconf@entry=0x0, plog=0x0, 
> ptemp=0x0, run_mode=MP_HOOK_RUN_FIRST)
> at modperl_callback.c:236
> #11 0x7fd44f91fd4f in modperl_callback_connection (idx=, 
> c=,
> run_mode=) at modperl_callback.c:359
> #12 0x55feaf10cdf0 in ap_run_process_connection 
> (c=c@entry=0x7fd4461f22b8) at connection.c:42
> #13 0x55feaf10d340 in ap_process_connection (c=c@entry=0x7fd4461f22b8, 
> csd=csd@entry=0x7fd4461f20a0)
> at connection.c:226
> #14 0x7fd4523f3e6b in process_socket (bucket_alloc=0x7fd4461f0028, 
> my_thread_num=1, my_child_num=0,
> sock=0x7fd4461f20a0, p=0x7fd4461f2028, thd=0x7fd453eb27a0) at worker.c:631
> #15 worker_thread (thd=0x7fd453eb27a0, dummy=) at worker.c:990
> #16 0x7fd453418454 in start_thread (arg=0x7fd44740d700) at 
> pthread_create.c:334
> #17 0x7fd453155ecd in clone () at 
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
>
>
> Before we chase down a potential non-defect in httpd, any thoughts
> on the underlying modperl or script logic?
>
>
> On Thu, Apr 14, 2016 at 1:44 AM, Takashi Sato <taka...@tks.st> wrote:
>
&g

Re: New segfault with 2.4.20 with mod_perl

2016-04-14 Thread William A Rowe Jr
We can be more vigilant about unexpectedly null values, however...

how are you running request processing in the connection callback
of mod_perl?  That makes no sense, and probably signals a deeper
logic error.

The access checker is configured per-dir, so until the request rec
is completely initialized during read_request, this doesn't make
much sense to me (full backtrace .. including frames #6-#10, for
those who are curious...)

Either the callback list registered for modperl_callback_connection,
or the Perl_runops_standard, or the Perl_pp_entersub invoking the
run_access_checker hook seem the most suspect here.

#0  apr_getnameinfo (hostname=hostname@entry=0x7fd4461ee368,
sockaddr=0x0, flags=flags@entry=0)
at /tmp/buildd/apr-1.5.2/network_io/unix/sockaddr.c:663
#1  0x55feaf0f513a in ap_get_useragent_host
(r=r@entry=0x7fd4461ee0a0, type=type@entry=3,
str_is_ip=str_is_ip@entry=0x7fd44740c9c4) at core.c:990
#2  0x7fd4519d7212 in find_allowdeny (r=r@entry=0x7fd4461ee0a0,
method=method@entry=0, a=,
a=) at mod_access_compat.c:279
#3  0x7fd4519d74b2 in check_dir_access (r=0x7fd4461ee0a0) at
mod_access_compat.c:332
#4  0x55feaf0f8f30 in ap_run_access_checker
(r=r@entry=0x7fd4461ee0a0) at request.c:87
#5  0x7fd448a6f7dd in XS_Apache2__RequestRec_run_access_checker
(my_perl=0x55feb2964a20, cv=)
at HookRun.c:235
#6  0x7fd44f5f7e6a in Perl_pp_entersub () from
/usr/lib/x86_64-linux-gnu/libperl.so.5.22
#7  0x7fd44f5f0ca6 in Perl_runops_standard () from
/usr/lib/x86_64-linux-gnu/libperl.so.5.22
#8  0x7fd44f575f06 in Perl_call_sv () from
/usr/lib/x86_64-linux-gnu/libperl.so.5.22
#9  0x7fd44f91ec28 in modperl_callback
(my_perl=my_perl@entry=0x55feb2964a20, handler=0x7fd4461f2750,
p=p@entry=0x7fd4461f2028, r=r@entry=0x0, s=s@entry=0x7fd453ddc628,
args=0x55feb3beebd0)
at modperl_callback.c:100
#10 0x7fd44f91f576 in modperl_callback_run_handlers (idx=0,
type=type@entry=1, r=r@entry=0x0,
c=, s=0x7fd453ddc628, pconf=pconf@entry=0x0,
plog=0x0, ptemp=0x0, run_mode=MP_HOOK_RUN_FIRST)
at modperl_callback.c:236
#11 0x7fd44f91fd4f in modperl_callback_connection (idx=, c=,
run_mode=) at modperl_callback.c:359
#12 0x55feaf10cdf0 in ap_run_process_connection
(c=c@entry=0x7fd4461f22b8) at connection.c:42
#13 0x55feaf10d340 in ap_process_connection
(c=c@entry=0x7fd4461f22b8, csd=csd@entry=0x7fd4461f20a0)
at connection.c:226
#14 0x7fd4523f3e6b in process_socket (bucket_alloc=0x7fd4461f0028,
my_thread_num=1, my_child_num=0,
sock=0x7fd4461f20a0, p=0x7fd4461f2028, thd=0x7fd453eb27a0) at worker.c:631
#15 worker_thread (thd=0x7fd453eb27a0, dummy=) at worker.c:990
#16 0x7fd453418454 in start_thread (arg=0x7fd44740d700) at
pthread_create.c:334
#17 0x7fd453155ecd in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109


Before we chase down a potential non-defect in httpd, any thoughts
on the underlying modperl or script logic?


On Thu, Apr 14, 2016 at 1:44 AM, Takashi Sato  wrote:

> r->useragent_addr is assigned on ap_read_request (http_core.c),
> called from ap_process_http_(async_)connection
> called from process_connection hook (APR_HOOK_REALLY_LAST).
>
> The SEGV occured on process_connection hook, maybe before
> ap_process_http_(async_)connection,
>
> #11 0x7fd44f91fd4f in modperl_callback_connection (idx= out>, c=,
> run_mode=) at modperl_callback.c:359
> #12 0x55feaf10cdf0 in ap_run_process_connection
> (c=c@entry=0x7fd4461f22b8) at connection.c:42
> #13 0x55feaf10d340 in ap_process_connection
> (c=c@entry=0x7fd4461f22b8, csd=csd@entry=0x7fd4461f20a0)
> at connection.c:226
>
> so r->useragent_addr had not been assigned any value.
>


Re: New segfault with 2.4.20 with mod_perl

2016-04-13 Thread William A Rowe Jr
I wrote the patch for ap_get_useragent_host, but am not familiar
with the details of the report.  Researching...




On Wed, Apr 13, 2016 at 5:33 PM, Stefan Fritsch  wrote:

> Hi,
>
> I got a report about a new crash in mod_perl tests with 2.4.20:
>
> (gdb) bt
> #0  apr_getnameinfo (hostname=hostname@entry=0x7fd4461ee368,
> sockaddr=0x0, flags=flags@entry=0)
> at /tmp/buildd/apr-1.5.2/network_io/unix/sockaddr.c:663
> #1  0x55feaf0f513a in ap_get_useragent_host
> (r=r@entry=0x7fd4461ee0a0, type=type@entry=3,
> str_is_ip=str_is_ip@entry=0x7fd44740c9c4) at core.c:990
> #2  0x7fd4519d7212 in find_allowdeny (r=r@entry=0x7fd4461ee0a0,
> method=method@entry=0, a=,
> a=) at mod_access_compat.c:279
> #3  0x7fd4519d74b2 in check_dir_access (r=0x7fd4461ee0a0) at
> mod_access_compat.c:332
> #4  0x55feaf0f8f30 in ap_run_access_checker
> (r=r@entry=0x7fd4461ee0a0) at request.c:87
> #5  0x7fd448a6f7dd in XS_Apache2__RequestRec_run_access_checker
> (my_perl=0x55feb2964a20, cv=)
> at HookRun.c:235
>
> Does this ring a bell with anyone?
>
> More info is at
>
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=820824
>
> Cheers,
> Stefan
>
>


Re: svn commit: r1738292 - /httpd/httpd/branches/2.4.x/STATUS

2016-04-11 Thread William A Rowe Jr
On Fri, Apr 8, 2016 at 6:25 PM, Eric Covener <cove...@gmail.com> wrote:

> On Fri, Apr 8, 2016 at 6:32 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > It was working, in the sense that it had the intended effect (the
> [un]define
> > took effect) in the broader global context.
> >
> > This is a breaking change to some potentially existing configs, however
> > misguided they are, which is the sort of thing we've avoided in the
> released
> > branch.
> >
> > Could we log an error rather than preventing startup?  One issue is that
> > these directives are encountered prior to opening the error log file.
> One
> > possible fix would be to have a second directive handler with the sole
> > purpose of emitting errors, running at the normal processing scope, and
> > not within exec_on_read.
>
> This seems to work inside a directive handler on unix and ends up in
> the console:
>
>ap_log_perror(APLOG_MARK, APLOG_STARTUP, 0, cmd->pool
>

While it works on windows, this error is logged to the event log (while
on unix for a service, it should wind up in the syslog).  Not sure if that
is entirely simple for the user to observe.


Re: svn commit: r1738292 - /httpd/httpd/branches/2.4.x/STATUS

2016-04-08 Thread William A Rowe Jr
It was working, in the sense that it had the intended effect (the [un]define
took effect) in the broader global context.

This is a breaking change to some potentially existing configs, however
misguided they are, which is the sort of thing we've avoided in the
released
branch.

Could we log an error rather than preventing startup?  One issue is that
these directives are encountered prior to opening the error log file.  One
possible fix would be to have a second directive handler with the sole
purpose of emitting errors, running at the normal processing scope, and
not within exec_on_read.

This shouldn't be allowed to fester on trunk, obviously, but for 2.4 it
seems
like something we shouldn't alter, no matter how much it frustrates users
who used this unintentionally.





On Fri, Apr 8, 2016 at 2:52 PM,  wrote:

> Author: covener
> Date: Fri Apr  8 19:52:19 2016
> New Revision: 1738292
>
> URL: http://svn.apache.org/viewvc?rev=1738292=rev
> Log:
> confused IRC user hit this in 2.4
>
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1738292=1738291=1738292=diff
>
> ==
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Fri Apr  8 19:52:19 2016
> @@ -175,6 +175,15 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>   2.4.x patch: trunk patch works (compatibility note to be adjusted)
>   +1: jailletc36
>
> +  *) core: block Define and Undefine in vhost and directory context.
> Because
> + it is EXEC_ON_READ, it "breaks out" of these contexts anyway.
> + trunk patch: http://svn.apache.org/r1656063
> +  http://svn.apache.org/r1656122
> + 2.4.x patch:
> http://people.apache.org/~covener/patches/2.4.x-define-limits.diff
> +
> + +1: covener (I need to review the docs manually in this area)
> +
> +
>  PATCHES/ISSUES THAT ARE BEING WORKED
>
>*) http: Don't remove the Content-Length of zero from a HEAD response if
>
>
>


Re: proxy API compat break in 2.4.7

2016-04-07 Thread William A Rowe Jr
The first most important documentation of this should be in ap_mmn.h,
as we've done in the past (you'll see some highlights).

A note under the CHANGES entry for 2.4.7 seems worthwhile.

Pointing this out under Announcement seems appropriate, that users
of third party mod_proxy-consuming modules will need to recompile
those modules when upgrading from 2.4.1 - 2.4.6.

I can't think of another obvious place, other than any related doxygen.

It seems to late to correct this situation, as half the world will benefit
from leaving things as they are, and the other half of the world would
have an easier time migrating from 2.4.early to 2.4.now.  Lose lose
situation in my mind, so best to leave as-is, with docs.



On Thu, Apr 7, 2016 at 1:09 PM, Rainer Jung  wrote:

> Revision 1560081 (backport of r1533087) introduced "char
> uds_path[PROXY_WORKER_MAX_NAME_SIZE]" in the middle of the
> proxy_worker_shared structure which IMHO broke compatibility between 2.4.6
> and 2.4.7.
>
> I noticed it just now, because I was implementing a custom LB method and
> during runtime it always found s->status to be 0. I had compiled it against
> an old 2.4.4 to ensure not using later features, but was testing inside
> 2.4.18. Thus the offset of the status field calculated for 2.4.4 actually
> pointed into the long uds_path string which wasn't used and probably
> calloced with zeroes.
>
> Any good idea how to best document the break for other module developers?
>
> Regards,
>
> Rainer
>


Re: Any one doing it?

2016-04-01 Thread William A Rowe Jr
On Fri, Apr 1, 2016 at 11:49 AM, Yann Ylavic  wrote:

> On Fri, Apr 1, 2016 at 6:36 PM, Luca Toscano 
> wrote:
> >
> > +1, let's concentrate on 2.4 :)
>
> You know, there is still many 2.2 around, and some people may have
> backported 2.4 fixes to 2.2 and decide to propose them some days.
> If three PMC members (at least) think a 2.2.32 is worth it, it may
> well be out ;)
>

As an RM of many 2.2 releases, my inclination at this point is to wait for
the point where we have 'something to fix' (vis a vis Security patches) and
hold a vote to announce at the same time that we are starting a 12 month
EOL clock, broadcast widely with a 2.2.32 security release.  That could
happen at any point, but the amount of activity on 2.2 branch since the
last release doesn't seem to merit a parallel release with 2.4.20 this
coming week.  Just my 2c...


> But I agree that we should mainly reference 2.4 in the "generic" docs.
>

We actually have an http://httpd.apache.org/docs/current/ tree that
reflects the now-current release, and needs not be changed when
we finally release a 2.6 or 3.0 flavor.


Re: Status for 2.4.20

2016-03-29 Thread William A Rowe Jr
FULL STOP.

The next person to demand the last word of this thread will be iptables
deleted
from existence at a.o.  Can you all appreciate that ~2000 people have to
read
all of your pissing contests?  This is simply not acceptable.

Be done with it.


Re: Status for 2.4.20

2016-03-28 Thread William A Rowe Jr
@Everyone on this thread - keep it civil.

On Fri, Mar 25, 2016 at 10:13 PM, Noel Butler 
wrote:

> On 25/03/2016 19:52, Graham Leggett wrote:
>
>> On 23 Mar 2016, at 1:58 PM, Noel Butler  wrote:
>>
>> as stated previously, this shit will happen when certain people push with
>>> a release often mentality
>>>
>>> AFAIK there is *ZERO* critical exploit bugs to be patched by any pending
>>> release, so lets get house in order  S T A B L E , then worry about
>>> releases, jesus christ, we are not ubuntu or redhat with set programs to
>>> release every 3 or 6 months regardless if shit is ready or not…..
>>>
>>
>> It sounds like you’re making drama where there is none.
>>
>
> sounds like you only look at this from one perspective, and thats not of
> the users, especially, the larger users.


Precisely the point.  If httpd were commercial software, there would only be
one perspective, that of the largest users with fairly static deployments
that
demand very small deltas - those that ensure few if any regressions.
Smaller
or more nimble users who need the most recent features are neglected in that
scenario.

Instead httpd does not operate as commercial software, it is open source.
When it breaks, you get to keep (and patch) all the pieces.  That's the
origin
story of this software and our continued model for success.  No amount of
pleas that "it shouldn't be that way" are going to change the mindset of the
project participants.  Please remember you are a guest on this list.

When we decided during 1.3.x that things were so shaky (third party module
recompilation was frequently necessary during the early 1.3.0-1.3.14
versions)
that we could do better for user communities.

Therefore, when we released 2.0 as GA, we declared the ABI stable, and
proceeded on ABI and API breaking work on a 2.1-dev trunk branch.  We all
agreed that 2.1 wouldn't be GA, but we would release 2.2.0 once we believed
that branch was ready to be ABI-stable.  That model continues to this day,
breaking changes are on 2.5-dev in trunk, and we seek 100% compatibility
on the 2.4.x branch.  There were contentious discussions that led us to this
model, but it was driven by competing interests by the developers of this
project, who are also users --- as opposed to external "demands".

We will seek to continue to release early and often, and one of our current
faults is that we haven't been releasing 2.5-dev often enough to engage
users
in the next release series, but pouring most of our energy into wedging
these
changes back into the 2.4.x branch.  But unlike commercial software and
many OSS projects, we don't declare 2.4.0 to be "feature complete", and
we continue to improve it in straightforward ways throughout the 2.4
lifetime.

If you want to package a stable "product", you can follow the RedHat and
others' model.  Just to take that single example, httpd 2.4.3 is the
released
flavor by RedHat.  They go to the extra effort to backport fixes-only and
plan
to support that version for some 10 years or so.  That is why many larger
users choose to stick with something like RHEL or CentOS or similar
distributions which are feature-frozen and much more stable than an active
product undergoing constant enhancement.

Just to wrap up another tl;dr post... others offered you a different option,
skip those versions which are too "experimental" for your tastes, and wait
for bugs to shake out.  We assert that 2.4.newest is the best available
version, but in such a large, modular and flexible project, it's impossible
to assure that a change set (release) will be an improvement for each and
every use case.

Use the version that is most appropriate to your use case, and seek a
commercial product if you expect the sort of stasis that your protest
appears to seek.


Re: mod_http2 on Windows (was: Re: Status for 2.4.20)

2016-03-24 Thread William A Rowe Jr
On Thu, Mar 24, 2016 at 11:40 AM, Jan Ehrhardt <php...@ehrhardt.nl> wrote:

> William A Rowe Jr in gmane.comp.apache.devel (Thu, 24 Mar 2016 09:16:17
> -0500):
> >>
> http://windows.php.net/downloads/snaps/master/r454ae8a/logs/make-ts-windows-vc14-x64-r454ae8a.html
> >
> >It's been a *long* time, and I know it hadn't been that well maintained
> >for non-Linux (non-BSD) target architectures.
>
> On the contrary: Windows support for PHP has made a big jump. The release
> master of PHP7 is even a Microsoft employee.
>

Excellent!

>> Even in the 32-bits PHP 7 there are some:
> >>
> >>
> http://windows.php.net/downloads/snaps/master/r454ae8a/logs/make-ts-windows-vc14-x86-r454ae8a.html
> >
> >With a bit of luck - many of these are resolved by simply jumping up to
> >Visual Studio 2015 (and removing any number of win32-specific bandaids
> >that covered some warts in the earlier stdc lib)?
>
> Read carefully: these are the results when compiling with VC14 == VS2015.
> This is the way the official PHP7 binaries are built.
>

And in fairness, not every "conversion from 'size_t' to 'unsigned int'"
error is
actually a lurking bug.  If you know the memory allocation can't exceed some
fixed size (e.g. passed as an httpd header value that already enjoys
truncation
or rejection much earlier in the process), then that flag would generally
mean
nothing.

It's a long process to get such things 100% correct, we have a few defects
lingering in httpd 2.4 release branch today - some due to binary ABI
restrictions.
My entire point is that VC14 makes such a thing possible, where it really
wasn't possible while Microsoft was ignoring POSIX in many places.


Re: mod_status Total Accesses with 2.4.20

2016-03-24 Thread William A Rowe Jr
The inner http2 response to the outer h1 upgrade request remains a
single request IMHO.  Within that h2 collection of streams, each of
the subsequent h2 requests should then be counted individually.

On Thu, Mar 24, 2016 at 10:54 AM, Yann Ylavic  wrote:

> Should the scoreboard really account for both h2 and inner h1
> connections in the same counter?
> It seems to me that they should be distinguish-ed/able.
>
> On Thu, Mar 24, 2016 at 3:56 PM, Stefan Eissing
>  wrote:
> > Found it. Slave connections cleared their scoreboard handle too early
> (and unneccessary) so that the update simply did not happen.
> >
> > Fix will be in trunk/2.4.x soonish..
> >
> >> Am 24.03.2016 um 14:05 schrieb Stefan Eissing <
> stefan.eiss...@greenbytes.de>:
> >>
> >> Quick test: http/2 connections are not counted here. checking...
> >>
> >>> Am 24.03.2016 um 14:02 schrieb Steffen :
> >>>
> >>> Using  server-status?auto.
> >>>
> >>> Now since running with 2.4.20-dev my statistics graphs shows  less
> Total Accesses.
> >>>
> >>> Is it not counting http/1.1 or http/2 in Total Accesses ?
> >>>
> >>> Steffen
> >>>
> >>
> >
>


Re: mod_http2 on Windows (was: Re: Status for 2.4.20)

2016-03-24 Thread William A Rowe Jr
On Thu, Mar 24, 2016 at 9:31 AM, Yann Ylavic  wrote:

> >> nghttp2_session.c(160) : warning C4996: 'vsnprintf': This function or
> >> variable may be unsafe. Consider using vsnprintf_s instead. To disable
> >> deprecation, use _CRT_SECURE_NO_WARNINGS.
>
> [sarcasm]
> Microsoft being unable to provide a safe vsnprintf() in the first
> place and now warning every user is kind of ironic.
>

Microsoft claims that all the non-MS foo_s variants that have buffer
targets as defined by POSIX are insecure, and offer foo_s variants.
We have apr's safe strncpy etc, so it's not like we didn't agree -
at least for some cases.

They are free to make any claim they like, but they offered an out...
from these warnings ... -D_CRT_SECURE_NO_DEPRECATE


> Not to talk about they very personal conception of sizeof(long) on
> 64bit systems...
>

You might be too young to remember when pointers had little to
no relation to integer registers on many cpu architectures :)

[/sarcasm]
>
> Not a very constructive comment, I agree :)
>

:)


Re: mod_http2 on Windows (was: Re: Status for 2.4.20)

2016-03-24 Thread William A Rowe Jr
On Thu, Mar 24, 2016 at 8:49 AM, Jan Ehrhardt <php...@ehrhardt.nl> wrote:

> William A Rowe Jr in gmane.comp.apache.devel (Thu, 24 Mar 2016 07:58:45
> -0500):
> >Precisely, Jan.  We don't know where these truncation errors lead - do
> they
> >portentially open security holes?  They cetainly interefere with serving
> >huge resources such as .iso images.
> >
> >When I first tripped over this, I breifly pondered putting together a
> >patch.  PHP, httpd (through apr) dealt with all this long ago.
>
> Apparently, you never compiled PHP 5.5/5.6/7.0 on Windows 64-bits. There
> are loads of 'possible loss of data' warnings. See for yourself:
>
> http://windows.php.net/downloads/snaps/master/r454ae8a/logs/make-ts-windows-vc14-x64-r454ae8a.html


It's been a *long* time, and I know it hadn't been that well maintained
for non-Linux (non-BSD) target architectures.


> Even in the 32-bits PHP 7 there are some:
>
> http://windows.php.net/downloads/snaps/master/r454ae8a/logs/make-ts-windows-vc14-x86-r454ae8a.html


With a bit of luck - many of these are resolved by simply jumping up to
Visual Studio 2015 (and removing any number of win32-specific bandaids
that covered some warts in the earlier stdc lib)?

Remaining errors likely still need fixing.  That sizeof(int) == sizeof(void
*)
assumption from 64ILP architectures is *not* part of the spec.  We have
64ILP (int == long == void*), 64LP (long == void* != int) as well as
64P (long == int != void*) and each of these are valid architectures.

The various types such as ptrdiff_t that POSIX exposes *should* resolve
these all when the correct types are chosen, and that is now true with
MSVC 14 on Windows as well.  It just becomes a matter of using the
true and correct type for various operations.

Cheers,

Bill


Re: apr_global_mutex_create vs apr_proc_mutex_create

2016-03-24 Thread William A Rowe Jr
proc_mutex can only be used to block on a cross process event.  It can be
used in a threaded server, but cannot be held in multiple threads, you
might use it to block in a single thread queue per-process.

Thread_mutex will not block between processes, this is used for the
mutex-per-process logic such as a across accept wait queues.

Global_mutex will block, across threads and across processes, as
appropriate.
On Mar 24, 2016 04:08, "Luca Toscano"  wrote:

> +httpd-dev@
>
> Hello!
>
> 2016-03-23 21:49 GMT+01:00 Ali Shah :
>
>> Hi Module maintainers,
>>
>> I'm writing a simple apache module and I'd like to aggregate some
>> statistics.
>> I saw an example module that does this (
>> https://wiki.apache.org/httpd/ModuleLife) using shared memory.
>>
>> My question for those on this list following:
>>
>> What is the main difference between using mutexes created
>> via apr_global_mutex_create and apr_proc_mutex_create in terms of
>> performance and safety of the resource that being protected?
>>
>> The docs seem to discourage usage of apr_global_mutex_create:
>> "There is considerable overhead in using this API if only cross-process or
>> cross-thread mutual exclusion is required."
>>
>> Is the overhead purely in code setup (since global mutex should be
>> initialized in post_config), or is this describing some performance
>> penalties as well?
>>
>
> From what I can see in [1] a global lock uses both apr_proc_mutex_create
> and apr_thread_mutex_create, so this is why the documentation suggests the
> use of a more specialized one if only cross-process or cross-thread sync is
> needed.
>
> Not a great explanation I know, somebody else with more experience will
> give you a better answer on this list :)
>
> Luca
>
>
>
> [1]:
> https://github.com/apache/apr/blob/7951861fb029fd0fb60e5ec8da73015d03afa28d/locks/unix/global_mutex.c#L46
>
>
>


Re: apr_global_mutex_create vs apr_proc_mutex_create

2016-03-24 Thread William A Rowe Jr
proc_mutex can only be used to block on a cross process event.  It can be
used in a threaded server, but cannot be held in multiple threads, you
might use it to block in a single thread queue per-process.

Thread_mutex will not block between processes, this is used for the
mutex-per-process logic such as a across accept wait queues.

Global_mutex will block, across threads and across processes, as
appropriate.
On Mar 24, 2016 04:08, "Luca Toscano"  wrote:

> +httpd-dev@
>
> Hello!
>
> 2016-03-23 21:49 GMT+01:00 Ali Shah :
>
>> Hi Module maintainers,
>>
>> I'm writing a simple apache module and I'd like to aggregate some
>> statistics.
>> I saw an example module that does this (
>> https://wiki.apache.org/httpd/ModuleLife) using shared memory.
>>
>> My question for those on this list following:
>>
>> What is the main difference between using mutexes created
>> via apr_global_mutex_create and apr_proc_mutex_create in terms of
>> performance and safety of the resource that being protected?
>>
>> The docs seem to discourage usage of apr_global_mutex_create:
>> "There is considerable overhead in using this API if only cross-process or
>> cross-thread mutual exclusion is required."
>>
>> Is the overhead purely in code setup (since global mutex should be
>> initialized in post_config), or is this describing some performance
>> penalties as well?
>>
>
> From what I can see in [1] a global lock uses both apr_proc_mutex_create
> and apr_thread_mutex_create, so this is why the documentation suggests the
> use of a more specialized one if only cross-process or cross-thread sync is
> needed.
>
> Not a great explanation I know, somebody else with more experience will
> give you a better answer on this list :)
>
> Luca
>
>
>
> [1]:
> https://github.com/apache/apr/blob/7951861fb029fd0fb60e5ec8da73015d03afa28d/locks/unix/global_mutex.c#L46
>
>
>


Re: 2.4.20 Change dsp: Win32-specific build files.

2016-03-24 Thread William A Rowe Jr
Steffan,

I was plesantly surprised when I tested this on an early VS2015 preview and
the conversion was still supported.

I observed the note below in 2010 when that comment was first made (I
cribbed that comment from 2.2.x checkin).  So I now wonder if that note was
based on an Express edition perhaps?  Or on an early release - and MS later
added or fixed the conversion?  I can't be more specific at this point, but
maybe someone else remembers such an issue.
On Mar 24, 2016 05:39, "Steffen" <i...@apachelounge.com> wrote:

> Good now.
>
> Little note:  Visual Studio 2010 users cannot convert our .dsp files.
> That's not true, I convert them.
>
> Steffen
>
>
>
> From: William A Rowe Jr Sent: Wednesday, March 23, 2016 9:23 PM
> To: httpd Subject: * Re: 2.4.20 Change dsp: Win32-specific build
> files.
>
> On Wed, Mar 23, 2016 at 11:12 AM, Steffen <i...@apachelounge.com> wrote:
>
> I went earlier back, so I did a .dsp/GUI build with no issues.
> Build with SVN revision 1736328.
>
> Running looks also fine sofar.
>
>
>
>
>
> Glad to hear, thanks for testing.  I've updated the commit log message
> to more clearly explain the intent and avoid future confusion;
>
> +Going forwards, keep win32 build in svn once a tree is stable.
> +Visual Studio 2010 users cannot convert our .dsp files, and
> +ddk toolchain users couldn't either.
> +
> +Applies the same logic as r1100294 on the 2.2.x branch.
> +
> Until the cmake build schema is entirely mature, these files
> are needed for command-line builds of the 2.4 tree, and are
> expected to change very little until the EOL of 2.4 branch.
> -The actual .dsp files are entirely unusable for any shipping
> -version of MSVC/Visual Studio, while the .mak exports of these
> -projects are usable on any flavor of the MS build environment.
> +
> +The .dsp source files are not directly usable for any shipping
> +version of MSVC/Visual Studio (post-Visual Studio 97 release),
> +while the .mak exports of these projects are usable on any flavor
> +of the MS nmake build environment.  The .dsp source files are
> +retained for those users wishing to import these projects into
> +the modern vcproj/sln file format.
>
>
> Hope that is more clear, further edit suggestions are welcome.
>
>
> Cheers,
>
>
> Bill
>


mod_http2 on Windows (was: Re: Status for 2.4.20)

2016-03-24 Thread William A Rowe Jr
On Mar 23, 2016 22:19, "Jan Ehrhardt" <php...@ehrhardt.nl> wrote:
>
> William A Rowe Jr in gmane.comp.apache.devel (Wed, 23 Mar 2016 08:00:19
> -0500):
> >On Wed, Mar 23, 2016 at 7:42 AM, William A Rowe Jr <wr...@rowe-clan.net>
> >wrote:
> >
> >> Again, a C89 regression breaking the candidate, but in an experimental
> >> module that we don't promise will always build.  nghttp2 is filled
with C99
> >> code, AIUI - due to bad decisions about basic C types - that library
can
> >> only build.
> >>
> >Whoops, answering too quickly...
> >
> >that library can only build *correctly* under the latest MSVC 14 (aka
> >Studio 2015) when you are compiling for 64-bits on Windows (not an issue
> >with 32 bit builds).
>
> How do you define 'correctly'? nghttp2 (git head) builds with only a
> single warning on VC14 x86:
>
> c:\php-sdk\prebuilt\nghttp2\lib\nghttp2_session.c(6767) : warning C4715:
> 'nghttp2_session_get_remote_settings': not all control paths return a
> value
>
> VC11 x86 and VC9 x86 builds of nghttp2 give a couple of extra warnings:
>
> nghttp2_session.c(160) : warning C4996: 'vsnprintf': This function or
> variable may be unsafe. Consider using vsnprintf_s instead. To disable
> deprecation, use _CRT_SECURE_NO_WARNINGS.
>
> All x64 builds issue many more warnings:
>
> cl -nologo -MD  -W3 -Z7 -DBUILDING_NGHTTP2 -I./includes -Dssize_t=long
> -D_U_="" -FoMSVC_obj /r_nghttp2_session.obj -c nghttp2_session.c
> nghttp2_session.c
> nghttp2_session.c(5092): warning C4244: 'return': conversion from
> '__int64' to 'long', possible loss of data
> nghttp2_session.c(5122): warning C4244: 'return': conversion from
> '__int64' to 'long', possible loss of data
> nghttp2_session.c(5477): warning C4244: 'return': conversion from
> '__int64' to 'long', possible loss of data
> nghttp2_session.c(5704): warning C4244: 'return': conversion from
> '__int64' to 'long', possible loss of data
> nghttp2_session.c(5888): warning C4244: 'return': conversion from
> '__int64' to 'long', possible loss of data
> nghttp2_session.c(5952): warning C4244: 'return': conversion from
> '__int64' to 'long', possible loss of data
> nghttp2_session.c(6082): warning C4244: 'return': conversion from
> '__int64' to 'long', possible loss of data
> nghttp2_session.c(6191): warning C4244: 'return': conversion from
> '__int64' to 'long', possible loss of data

Precisely, Jan.  We don't know where these truncation errors lead - do they
portentially open security holes?  They cetainly interefere with serving
huge resources such as .iso images.

When I first tripped over this, I breifly pondered putting together a
patch.  PHP, httpd (through apr) dealt with all this long ago.

But nghttp2 is a modern C99/POSIX app, why ask the author to code around
Win32-isms, now that MS finally got these right with the VC14 release?


Re: 2.4.20 Change dsp: Win32-specific build files.

2016-03-23 Thread William A Rowe Jr
On Wed, Mar 23, 2016 at 11:12 AM, Steffen  wrote:

> I went earlier back, so I did a .dsp/GUI build with no issues.
>
> Build with SVN revision 1736328.
>
> Running looks also fine sofar.
>

Glad to hear, thanks for testing.  I've updated the commit log message
to more clearly explain the intent and avoid future confusion;

+Going forwards, keep win32 build in svn once a tree is stable.
+Visual Studio 2010 users cannot convert our .dsp files, and
+ddk toolchain users couldn't either.
+
+Applies the same logic as r1100294 on the 2.2.x branch.
+
 Until the cmake build schema is entirely mature, these files
 are needed for command-line builds of the 2.4 tree, and are
 expected to change very little until the EOL of 2.4 branch.
-The actual .dsp files are entirely unusable for any shipping
-version of MSVC/Visual Studio, while the .mak exports of these
-projects are usable on any flavor of the MS build environment.
+
+The .dsp source files are not directly usable for any shipping
+version of MSVC/Visual Studio (post-Visual Studio 97 release),
+while the .mak exports of these projects are usable on any flavor
+of the MS nmake build environment.  The .dsp source files are
+retained for those users wishing to import these projects into
+the modern vcproj/sln file format.

Hope that is more clear, further edit suggestions are welcome.

Cheers,

Bill


Re: 2.4.20 Change dsp: Win32-specific build files.

2016-03-23 Thread William A Rowe Jr
On Mar 23, 2016 3:15 AM, "Steffen"  wrote:
>
> Saw that wrowe did a change win make files in 2.4. branche.
>
> The comment says the the .dsp flies are entirely ! unusable.
>
> I can tell that they are entirely usable with V9-VC14.   What was the
issue ?

They were directly usable on VC98.  They were also directly exportable to
.mak files on that version of the MS build tooling, alone.

In post-VC98 (which cannot be obtained from Microsoft any longer due to the
Sun settlement) they are not "usable" - but they are convertible, although
they are not exportable.  There is lots of documentation on how to
translate these into the modern solution file formats in order to be able
to build httpd on Windows.

But they don't "simply work", e.g. " usable", without this manual
conversion process, do we agree?

The commit did not remove the .dsp files, those who wish to convert the
files into the new GUI schema remain able to do so.

> So please explain why for you it is not usable ?

Some more context can be found here;

http://markmail.org/thread/k2d6nwkz5w3p35yl

If the commit looks familiar, it aught to...

http://svn.apache.org/viewvc?view=revision=1100294

http://markmail.org/message/4gyjktdp7ls3cxij

> Change comment:
>
> The actual .dsp files are entirely unusable for any shipping
> version of MSVC/Visual Studio, while the .mak exports of these
> projects are usable on any flavor of the MS build environment.

I'm happy to edit the commit log message to better reflect the comments
above.

I hope that clarifies this commit and resolves any confusion I might have
created?


Re: Status for 2.4.20

2016-03-23 Thread William A Rowe Jr
On Wed, Mar 23, 2016 at 7:42 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> > branches/2.4.x/modules/http2/h2_filter.c
>
> Again, a C89 regression breaking the candidate, but in an experimental
> module that we don't promise will always build.  nghttp2 is filled with C99
> code, AIUI - due to bad decisions about basic C types - that library can
> only build.
>
Whoops, answering too quickly...

that library can only build *correctly* under the latest MSVC 14 (aka
Studio 2015)
when you are compiling for 64-bits on Windows (not an issue with 32 bit
builds).

In the past, size_t/ssize_t/ptrdiff_t and other alignment primitives failed
to
correspond to the width of their defined domain (size reflects memory, ergo
thes must be the same size, bitwise, as a void*).  This showed up very
egregiously in nghttp2 which (fairly) demands that the POSIX types reflect
their definitions.

MS finally is catching up to POSIX with their Studio 2015 product release.
So the antique .dsp is something of a red herring, you probably don't want
to ever run an nghttp2 app or library compiled against VC98, if building to
64-bits.  It is hardly a fully-conformant C99 compiler, but it comes a whole
lot closer than previous attempts.


Re: Status for 2.4.20

2016-03-23 Thread William A Rowe Jr
On Mar 23, 2016 6:23 AM, "Steffen"  wrote:
>
> Just did a export;
>
> Diff with the vote 2.4.19 one:
>
> branches/2.4.x/modules/cache/mod_socache_shmcb.c

Correct.  I'm not claiming this is win32-specific, it only happens to show
up on that and other edge cases.

> branches/2.4.x/modules/http2/h2_filter.c

Again, a C89 regression breaking the candidate, but in an experimental
module that we don't promise will always build.  nghttp2 is filled with C99
code, AIUI - due to bad decisions about basic C types - that library can
only build.

> branches/2.4.x/modules/http2/mod_http2.dsp

Looking good.

> For remove mod_lbmethod-rr:
> branches/2.4.x/Makefile.win
> branches/2.4.x/Readme.cmake
> branches/2.4.x/Apache.dsw
> branches/2.4.x/Apache-apr2.dsw

This may be reverted of course, if someone believes we should persist this
non-compileable example.

> And a bunch of .dep and mak files added

This simplifies the process for users who don't want to go through a manual
process of converting archaic project files to Windows.

This is the same structure as in every httpd 2.2 tarball.

But an explanation is in order... I haven't directly used these build files
-myself- for creating anything other than ASF binaries in over a decade.
They were not suited for building component-by-component, only for building
the whole shooting match at once.  I've been building each component (pcre,
expat, zlib, apr etc etc) for forever now.

The .mak/.dep files and Makefile.win (which invokes these) supports only
the srclib/ subcomponents structure.  You'll find Windows .mak/.dep files
in those apr source projects as well.

So this change neither breaks nor fixes - it is an alternative to a manual
conversion - it merely simplifies building on Windows - until cmake builds
are considered the replacement.  The addition of these to the svn, rather
than only a win32 source tarball, has a lengthy discussion and rational in
dev@ archive already.

But I have no preference as a user, myself - my entire interest lies in
cmake.


Re: fate of mod_lbmethod_rr (was: Re: [VOTE] Release Apache httpd 2.4.19 as GA)

2016-03-22 Thread William A Rowe Jr
On Tue, Mar 22, 2016 at 4:15 PM, Jeff Trawick <traw...@gmail.com> wrote:

> On Tue, Mar 22, 2016 at 5:03 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
>
>> On Tue, Mar 22, 2016 at 3:38 PM, Jeff Trawick <traw...@gmail.com> wrote:
>>
>>> On Tue, Mar 22, 2016 at 3:55 PM, William A Rowe Jr <wr...@rowe-clan.net>
>>> wrote:
>>>
>>>> Can anyone get mod_lbmethod_rr.c to build?
>>>>
>>>
>>> That's funny actually.  The very first version README.cmake in trunk
>>> says that mod_lbmethod_rr.c doesn't build on Windows
>>>
>>
>>  When I added the .dsp, it certainly did build.  --enable-mods=all should
>> be
>> triggering the build of those sources.
>>
>> I think this illustrates that we have played fast and loose with
>> something that
>> 1. is a public API, 2. not experimental, and 3. was illustrated with an
>> example
>> that has been frequently broken by Major ABI changes.
>>
>> If devs want to promote an API and then continuously break ABI on trunk,
>> I'm way beyond arguing with such individuals.  Just a few choice examples
>> which had necessitated major MMN bumps that did not receive one...
>>
>> http://svn.apache.org/viewvc?view=revision=1560081
>> http://svn.apache.org/viewvc?view=revision=1477649 (no
>> bitwise-alignment assurance)
>> http://svn.apache.org/viewvc?view=revision=1436919 (no
>> bitwise-alignment assurance)
>>
>> However, this module appears to have been broken prior to 2.4.1 GA with
>> this
>> at least this commit...
>> http://svn.apache.org/viewvc?view=revision=1209958
>> ... which tells me it is simply an abandoned example.
>>
>> I propose we remove it from 2.4.x branch and trunk, rather than
>> pretending
>> we have maintained it?
>>
>
> +1 for removing from 2.4.x branch
>
> no arguments here if someone actually wants it to hang around in trunk,
> but I don't actually know if anybody cares so no vote on trunk ATM...
>

I agree, this discussion is only about 2.4.x branch for the imminent T

If I have a third +1 for removing this horridly wrong example/, I'll commit
in the next 2 hours.  If there is disagreement later, we can always revert.

Bill


Re: Need hints from those building mod_http2 and mod_proxy_http2 on Windows

2016-03-22 Thread William A Rowe Jr
I've had success with Jeff's minimalist nghttp build schema.  A larger
effort at a cmake for the full stack (lib+apps) has been in the works for a
month or two now, haven't had a chance to try it.
On Mar 22, 2016 6:27 PM, "Jan Ehrhardt"  wrote:

> Jeff Trawick in gmane.comp.apache.devel (Tue, 22 Mar 2016 18:11:59 -0400):
> >What version of nghttp2 are you using?
>
> I always use git head of nghttp2. So that is > v1.8.0 ATM
>
> >Are you using the cmake build for httpd?
>
> No. I open the *.dsw / *.dsp in with VC9 / VC11, after applying these
> operations:
>
>  cd C:\build\2.4.x
>  srclib\apr\build\lineends.pl
>  srclib\apr\build\cvtdsp.pl -2005
>
> See https://www.apachelounge.com/viewtopic.php?p=33069#33069
> That is the way Apachelounge & Apachehaus build httpd.
> FYI: James Blond == Mario Brandt
>
> Jan
>
>


<    6   7   8   9   10   11   12   13   14   15   >