Re: svn commit: r1853407 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_http.c

2019-02-14 Thread Stefan Eissing
Yann,

The new v3 patch runs without any problems in my test setups. Nice work!

As to the errors you see: that seems to be the output parsing for nghttp
in the test classes that needs some more work. On your systems, DATA
packages arrive different than my MacOS had seen so far. That messed
up the response body compare.

Working things out on a ubuntu image now. Will let you know when I
have something for you to verify.

Cheers,
Stefan

> Am 13.02.2019 um 18:55 schrieb Yann Ylavic :
> 
> Thanks Stefan,
> 
> I think the "400" issue is fixed (r1853518, added to backport
> proposal), but two tests keep failing for in the test suite, namely
> test_004_post and test_500_proxy. They fail with or without my changes
> (trunk and 2.4.x), so I don't think it's related (mod_proxy does not
> seem to be reached for failing requests)..
> 
> My system is debian/openssl-1.1.1a, dunno if openssl version is
> related but forcing "SSLProtocol TLSv1.2" didn't help.
> The attached tarball contains an "output.log" (from: make test 2>&1
> |tee output.log), "mod_h2-test.diff" which is the configuration
> changes I ran with (mainly trace6 with dumpio, and a small/unrelated
> ProxyPass "fix" for the trailing slash), and finally the error_log
> (trace6/dumpio).
> 
> HTH...



Re: svn commit: r1853407 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_http.c

2019-02-13 Thread Yann Ylavic
Thanks Stefan,

I think the "400" issue is fixed (r1853518, added to backport
proposal), but two tests keep failing for in the test suite, namely
test_004_post and test_500_proxy. They fail with or without my changes
(trunk and 2.4.x), so I don't think it's related (mod_proxy does not
seem to be reached for failing requests)..

My system is debian/openssl-1.1.1a, dunno if openssl version is
related but forcing "SSLProtocol TLSv1.2" didn't help.
The attached tarball contains an "output.log" (from: make test 2>&1
|tee output.log), "mod_h2-test.diff" which is the configuration
changes I ran with (mainly trace6 with dumpio, and a small/unrelated
ProxyPass "fix" for the trailing slash), and finally the error_log
(trace6/dumpio).

HTH...


Re: svn commit: r1853407 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_http.c

2019-02-12 Thread Stefan Eissing
Ok, took the opportunity to add that to my pytest suite in mod-h2.

You need a local "pytest", "curl" and "nghttp" executable. If you checkout 
https://github.com/icing/mod_h2 master, run the configure:

> autoreconf -i
> automake
> autoconf
> ./configure --with-apxs=/opt/apache-2.4.x/bin/apxs --enable-werror
> make
> make test

("/opt/apache-2.4.x" is where I install my locally build 2.4.x branch. Adjust 
to your needs.)

You should see, on an unpatched 2.4.x
= 
test session starts platform darwin -- Python 2.7.10, pytest-3.0.7, py-1.4.33, 
pluggy-0.4.0
rootdir: /Users/sei/projects/mod-h2/test/e2e, inifile:
collected 86 items

test_001_httpd_alive.py ..
test_002_curl_basics.py ..
test_003_get.py .
test_004_post.py 
test_100_conn_reuse.py .
test_101_ssl_reneg.py ...
test_102_require.py ..
test_103_upgrade.py ...
test_200_header_invalid.py ...
test_201_header_conditional.py ..
test_202_trailer.py 
test_300_interim.py ...
test_400_push.py ...
test_401_early_hints.py ..
test_500_proxy.py ...

= 86 passed 
in 23.13 seconds 

On a 2.4.x installed with the proposed proxy patches, the test_500_24 fails 
with a 400 response code from the server.
You can just run that test case via

> (cd test/e2e && pytest -k 500_24)

Hope this helps.

-Stefan

> Am 12.02.2019 um 13:59 schrieb Yann Ylavic :
> 
> OK, thanks Stefan, will look at it.
> 
> 
> On Tue, Feb 12, 2019 at 11:12 AM ste...@eissing.org  
> wrote:
>> 
>> Yann,
>> 
>> this works fine in my tests on trunk. However on 2.4.x I get often errors 
>> when uploading data without content-length.
>> 
>> Scenario from the h2 test suite, HTTP/2 on the front, old skool HTTP/1.1 
>> proxy to itself:
>> 
>>> while true; do nghttp -v --expect-continue --data=gen/tmp/updata 
>>> -H'Content-Type: multipart/form-data; boundary=DSAJKcd9876' 
>>> --no-content-length http://test.example.org:12345/proxy/upload.py; done | 
>>> fgrep 400
>> 
>> [  0.009] recv (stream_id=13) :status: 400
>> 400 Bad Request
>> [  0.007] recv (stream_id=13) :status: 400
>> 400 Bad Request
>> ...
>> 
>> 
>> /etc/hosts
>> 
>> 127.0.0.1   test.example.orgtest
>> 
>> httpd config:
>> 
>>ProxyPass "/proxy" "balancer://http-local"
>>ProxyPassReverse "/proxy" "balancer://http-local"
>> 
>>
>>BalancerMember "http://127.0.0.1:12345; hcmethod=GET hcuri=/ ttl=4
>>
>> 
>> 
>>> cat gen/tmp/updata
>> --DSAJKcd9876
>> Content-Disposition: form-data; name="xxx"; filename="x"
>> Content-Type: text/plain
>> 
>> testing mod_h2
>> --DSAJKcd9876
>> Content-Disposition: form-data; name="file"; filename="data-1k"
>> Content-Type: application/octet-stream
>> Content-Transfer-Encoding: binary
>> 
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>> 
>> --DSAJKcd9876--
>> 
>> 
>> 
>>> Am 11.02.2019 um 22:55 schrieb yla...@apache.org:
>>> 
>>> Author: ylavic
>>> Date: Mon Feb 11 21:55:43 2019
>>> New Revision: 1853407
>>> 
>>> URL: http://svn.apache.org/viewvc?rev=1853407=rev
>>> Log:
>>> mod_proxy_http: rework the flushing strategy when forwarding the request 
>>> body.
>>> 
>>> Since the forwarding of 100-continue (end to end) in r1836588, we depended 
>>> on
>>> reading all of the requested HUGE_STRING_LEN bytes to avoid the flushes, but
>>> this is a bit fragile.
>>> 
>>> This commit introduces the new stream_reqbody_read() function which will 
>>> try a
>>> nonblocking read first and, if it fails with EAGAIN, will flush on the 
>>> backend
>>> side before blocking for the next client side read.
>>> 
>>> We can then use it in stream_reqbody_{chunked,cl}() to flush client 
>>> forwarded
>>> data only when necessary. This both allows "optimal" flushing and simplifies
>>> code (note that spool_reqbody_cl() also makes use of the new function but 
>>> not
>>> its nonblocking/flush functionality, thus only for consistency with the two
>>> 

Re: svn commit: r1853407 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_http.c

2019-02-12 Thread Yann Ylavic
OK, thanks Stefan, will look at it.


On Tue, Feb 12, 2019 at 11:12 AM ste...@eissing.org  wrote:
>
> Yann,
>
> this works fine in my tests on trunk. However on 2.4.x I get often errors 
> when uploading data without content-length.
>
> Scenario from the h2 test suite, HTTP/2 on the front, old skool HTTP/1.1 
> proxy to itself:
>
> >  while true; do nghttp -v --expect-continue --data=gen/tmp/updata 
> > -H'Content-Type: multipart/form-data; boundary=DSAJKcd9876' 
> > --no-content-length http://test.example.org:12345/proxy/upload.py; done | 
> > fgrep 400
>
> [  0.009] recv (stream_id=13) :status: 400
> 400 Bad Request
> [  0.007] recv (stream_id=13) :status: 400
> 400 Bad Request
> ...
>
>
> /etc/hosts
>
> 127.0.0.1   test.example.orgtest
>
> httpd config:
>
> ProxyPass "/proxy" "balancer://http-local"
> ProxyPassReverse "/proxy" "balancer://http-local"
>
> 
> BalancerMember "http://127.0.0.1:12345; hcmethod=GET hcuri=/ ttl=4
> 
>
>
> > cat gen/tmp/updata
> --DSAJKcd9876
> Content-Disposition: form-data; name="xxx"; filename="x"
> Content-Type: text/plain
>
> testing mod_h2
> --DSAJKcd9876
> Content-Disposition: form-data; name="file"; filename="data-1k"
> Content-Type: application/octet-stream
> Content-Transfer-Encoding: binary
>
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
> 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
>
> --DSAJKcd9876--
>
>
>
> > Am 11.02.2019 um 22:55 schrieb yla...@apache.org:
> >
> > Author: ylavic
> > Date: Mon Feb 11 21:55:43 2019
> > New Revision: 1853407
> >
> > URL: http://svn.apache.org/viewvc?rev=1853407=rev
> > Log:
> > mod_proxy_http: rework the flushing strategy when forwarding the request 
> > body.
> >
> > Since the forwarding of 100-continue (end to end) in r1836588, we depended 
> > on
> > reading all of the requested HUGE_STRING_LEN bytes to avoid the flushes, but
> > this is a bit fragile.
> >
> > This commit introduces the new stream_reqbody_read() function which will 
> > try a
> > nonblocking read first and, if it fails with EAGAIN, will flush on the 
> > backend
> > side before blocking for the next client side read.
> >
> > We can then use it in stream_reqbody_{chunked,cl}() to flush client 
> > forwarded
> > data only when necessary. This both allows "optimal" flushing and simplifies
> > code (note that spool_reqbody_cl() also makes use of the new function but 
> > not
> > its nonblocking/flush functionality, thus only for consistency with the two
> > others, simplification and common error handling).
> >
> > Also, since proxy_http_req_t::flushall/subprocess_env::proxy-flushall are 
> > now
> > meaningless (and unused) on the backend side, they are renamed respectively 
> > to
> > prefetch_nonblocking/proxy-prefetch-nonblocking, and solely determine 
> > whether
> > to prefetch in nonblocking mode or not. These flags were trunk only and may
> > not be really useful if we decided to prefetch in nonblocking mode in any 
> > case,
> > but for 2.4.x the opt-in looks wise.
> >
> > Modified:
> >httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
> >httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> >
> > Modified: httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
> > URL: 
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/mod_proxy_http2.c?rev=1853407=1853406=1853407=diff
> > ==
> > --- httpd/httpd/trunk/modules/http2/mod_proxy_http2.c (original)
> > +++ httpd/httpd/trunk/modules/http2/mod_proxy_http2.c Mon Feb 11 21:55:43 
> > 2019
> > @@ -77,7 +77,6 @@ typedef struct h2_proxy_ctx {
> >
> > unsigned standalone : 1;
> > unsigned is_ssl : 1;
> > -unsigned flushall : 1;
> >
> > apr_status_t r_status; /* status of our first request work */
> > h2_proxy_session *session; /* current http2 session against backend */
> > @@ -509,7 +508,6 @@ static int proxy_http2_handler(request_r
> > ctx->is_ssl = is_ssl;
> > ctx->worker = worker;
> > ctx->conf   = conf;
> > -ctx->flushall   

Re: svn commit: r1853407 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_http.c

2019-02-12 Thread ste...@eissing.org
Yann,

this works fine in my tests on trunk. However on 2.4.x I get often errors when 
uploading data without content-length.

Scenario from the h2 test suite, HTTP/2 on the front, old skool HTTP/1.1 proxy 
to itself:

>  while true; do nghttp -v --expect-continue --data=gen/tmp/updata 
> -H'Content-Type: multipart/form-data; boundary=DSAJKcd9876' 
> --no-content-length http://test.example.org:12345/proxy/upload.py; done | 
> fgrep 400

[  0.009] recv (stream_id=13) :status: 400
400 Bad Request
[  0.007] recv (stream_id=13) :status: 400
400 Bad Request
...


/etc/hosts

127.0.0.1   test.example.orgtest

httpd config:

ProxyPass "/proxy" "balancer://http-local"
ProxyPassReverse "/proxy" "balancer://http-local"


BalancerMember "http://127.0.0.1:12345; hcmethod=GET hcuri=/ ttl=4



> cat gen/tmp/updata
--DSAJKcd9876
Content-Disposition: form-data; name="xxx"; filename="x"
Content-Type: text/plain

testing mod_h2
--DSAJKcd9876
Content-Disposition: form-data; name="file"; filename="data-1k"
Content-Type: application/octet-stream
Content-Transfer-Encoding: binary

012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678

--DSAJKcd9876--

#!/usr/bin/env python
import cgi, os
import cgitb; cgitb.enable()

status = '200 Ok'


def mk_body(s):
return ("\n%s\n" % (s))

try: # Windows needs stdio set for binary mode.
import msvcrt
msvcrt.setmode (0, os.O_BINARY) # stdin  = 0
msvcrt.setmode (1, os.O_BINARY) # stdout = 1
except ImportError:
pass

form = cgi.FieldStorage()
body=''

# Test if the file was uploaded
if 'file' in form:
# A nested FieldStorage instance holds the file
fileitem = form['file']

# strip leading path from file name to avoid directory traversal attacks
fn = os.path.basename(fileitem.filename)
f = open('./files/' + fn, 'wb');
f.write(fileitem.file.read())
f.close()
body = mk_body('The file "%s" was uploaded successfully' %fn)

elif 'remove' in form:
remove = form['remove'].value
try:
fn = os.path.basename(remove)
os.remove('./files/' + fn)
message = 'The file "' + fn + '" was removed successfully'
except OSError, e:
message = 'Error removing ' + fn + ': ' + e.strerror
status = '404 File Not Found'
else:
body = mk_body('''Upload File

Upload''')

print "Status: %s" % (status)
print "Content-Type: text/html"
print "Content-Length: %d"  % (len(body) + 1)
print ""
print "%s" % (body)


> Am 11.02.2019 um 22:55 schrieb yla...@apache.org:
> 
> Author: ylavic
> Date: Mon Feb 11 21:55:43 2019
> New Revision: 1853407
> 
> URL: http://svn.apache.org/viewvc?rev=1853407=rev
> Log:
> mod_proxy_http: rework the flushing strategy when forwarding the request body.
> 
> Since the forwarding of 100-continue (end to end) in r1836588, we depended on
> reading all of the requested HUGE_STRING_LEN bytes to avoid the flushes, but
> this is a bit fragile.
> 
> This commit introduces the new stream_reqbody_read() function which will try a
> nonblocking read first and, if it fails with EAGAIN, will flush on the backend
> side before blocking for the next client side read.
> 
> We can then use it in stream_reqbody_{chunked,cl}() to flush client forwarded
> data only when necessary. This both allows "optimal" flushing and simplifies
> code (note that spool_reqbody_cl() also makes use of the new function but not
> its nonblocking/flush functionality, thus only for consistency with the two
> others, simplification and common error handling).
> 
> Also, since proxy_http_req_t::flushall/subprocess_env::proxy-flushall are now
> meaningless (and unused) on the backend side, they are renamed respectively to
> prefetch_nonblocking/proxy-prefetch-nonblocking, and solely determine whether
> to prefetch in nonblocking mode or not. These flags were trunk only and may
> not be really useful if we decided to prefetch in nonblocking mode in any 
> case,
> but for 2.4.x the opt-in looks wise.
> 
> Modified:
>httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
>