ap_get_server_port for subrequest different than main request
Hello,
I noticed some interesting behavior when retrieving the port of a subrequest
and am trying to determine if it's a bug or by design. Basically, the sub-
request's parsed_uri port fields aren't preserved from the main request. With
default canonical name settings, this leads to ap_get_server_port falling back
to the port from the ServerName, whereas calling ap_get_server_port on the
main request would use the port from the host header, potentially giving
different results.
You can see this effect using mod_rewrite and lookahead variables:
Listen 9090
ServerName localhost:9092
RewriteEngine On
RewriteRule ^ /?port=%{SERVER_PORT}=%{LA-U:SERVER_PORT} [R]
A request to localhost:9090/port redirects to /?port=9090=9092
This is different than internal redirects, which copy the port fields upon
creating the new request (see internal_internal_redirect in http_request.c).
The attached trunk patch would do the same for subrequests created via
ap_sub_req_method_uri (but not for others). I also tried writing a test case,
but httpd-test specifies a ServerName, and I'm not familiar enough with the
framework to get around that.
Any thoughts on the expected behavior would be appreciated.
Regards,
Paul Spangler
LabVIEW R
National Instruments
subrequest_port.patch
Description: subrequest_port.patch
Re: [PATCH 60223] non-empty OpenSSL error queue preventing non-blocking reads through mod_ssl
On 10/17/2016 2:04 PM, Paul Spangler wrote: Hello, Due to the way OpenSSL stores errors in a per-thread queue, functions such as SSL_read followed by SSL_get_error may not produce the desired result if the error queue is not empty prior to calling SSL_read[1]. For example, a non-blocking read reports that no data is available by setting up SSL_get_error to return SSL_ERROR_WANT_READ. But if an error is already present in the queue, SSL_get_error sees that error instead and returns SSL_ERROR_SSL. I found at least one case where the error queue may be non-empty prior to a non-blocking read[2] that involves combining mod_session_crypto (which leaves the error queue non-empty) and the third-party mod_websocket (which uses non-blocking reads), resulting in the connection being closed. I included a potential patch to mod_ssl for consideration on the bug report that simply clears the error queue prior to any of the three SSL_* calls that mod_ssl makes. An ideal fix might be to keep the error queue empty at all times (i.e. patch the APR crypto library), but I propose that this patch is more robust in a modular environment. Thanks for your consideration. [1] https://www.openssl.org/docs/manmaster/ssl/SSL_get_error.html#DESCRIPTION [2] https://bz.apache.org/bugzilla/show_bug.cgi?id=60223 Anyone have any thoughts on this small patch? It addresses an issue with OpenSSL's per-thread state causing connections to fail. Regards, Paul Spangler LabVIEW R National Instruments
[PATCH 60223] non-empty OpenSSL error queue preventing non-blocking reads through mod_ssl
Hello, Due to the way OpenSSL stores errors in a per-thread queue, functions such as SSL_read followed by SSL_get_error may not produce the desired result if the error queue is not empty prior to calling SSL_read[1]. For example, a non-blocking read reports that no data is available by setting up SSL_get_error to return SSL_ERROR_WANT_READ. But if an error is already present in the queue, SSL_get_error sees that error instead and returns SSL_ERROR_SSL. I found at least one case where the error queue may be non-empty prior to a non-blocking read[2] that involves combining mod_session_crypto (which leaves the error queue non-empty) and the third-party mod_websocket (which uses non-blocking reads), resulting in the connection being closed. I included a potential patch to mod_ssl for consideration on the bug report that simply clears the error queue prior to any of the three SSL_* calls that mod_ssl makes. An ideal fix might be to keep the error queue empty at all times (i.e. patch the APR crypto library), but I propose that this patch is more robust in a modular environment. Thanks for your consideration. [1] https://www.openssl.org/docs/manmaster/ssl/SSL_get_error.html#DESCRIPTION [2] https://bz.apache.org/bugzilla/show_bug.cgi?id=60223 Regards, Paul Spangler LabVIEW R National Instruments
Re: Random AH01842 errors in mod_session_crypto
On 10/4/2016 10:29 AM, Graham Leggett wrote: On 4 Oct 2016, at 15:47, Paul Spangler <paul.spang...@ni.com> wrote: From my understanding, apr_crypto_key_t is an opaque struct defined separately by each crypto provider, so mod_session_crypto will not be able to do the sizeof. That's a sizeof a pointer to apr_crypto_key_t, not the sizeof apr_crypto_key_t itself. It's possible I'm looking a different version of the code, but when I try that patch: apr_crypto_key_t *key = NULL; ... key = apr_pcalloc(r->pool, sizeof *key); mod_session_crypto.c: In function 'decrypt_string': mod_session_crypto.c:249:11: error: dereferencing pointer to incomplete type Keys are read at server start and reused. Trying to regenerate the key on every request has performance implications. mod_session_crypto's passphrases can also be read from .htaccess, which means at least some keys may be unknown at server start. I agree that regenerating the keys on every request is not ideal. I'm only questioning the feasibility of reusing keys that may come and go from request to request. Regards, Paul Spangler LabVIEW R National Instruments
Re: Random AH01842 errors in mod_session_crypto
On 9/12/2016 2:41 PM, Yann Ylavic wrote:
On Mon, Sep 12, 2016 at 10:31 AM, Ewald Dieterich <ew...@mailbox.org> wrote:
On 06/13/2016 09:38 AM, Ewald Dieterich wrote:
Looks like the problem is this:
Thanks for invertigating!
Yes, I recently found a case where this error comes up as well and this
investigation helped a lot. I'm also interested in resuming the
discussion about potentially fixing it.
Thanks for the patch, possibly a simpler way to fix it would be:
Index: modules/session/mod_session_crypto.c
===
--- modules/session/mod_session_crypto.c(revision 1760149)
+++ modules/session/mod_session_crypto.c(working copy)
@@ -246,6 +246,8 @@ static apr_status_t decrypt_string(request_rec * r
return res;
}
+key = apr_pcalloc(r->pool, sizeof *key);
+
/* try each passphrase in turn */
for (; i < dconf->passphrases->nelts; i++) {
const char *passphrase = APR_ARRAY_IDX(dconf->passphrases, i, char *);
_
so that crypto_passphrase() will use the given key instead of
allocating a new one.
From my understanding, apr_crypto_key_t is an opaque struct defined
separately by each crypto provider, so mod_session_crypto will not be
able to do the sizeof.
My initial reaction is thinking that the crypto providers should be
allocating the keys using the pool passed to apr_crypto_passphrase
instead of the one passed to apr_crypto_make. But it looks like a
relatively recent change to APR trunk [1] makes it more explicit in the
documentation that the allocated keys last until the context is cleaned up.
That implies we either need a context per request, like Ewald's patch,
or use a lock (to avoid multithreded pool access) and reuse the keys (to
avoid increased memory consumption). But the keys may change per
directory, so I don't know if that's feasible.
[1] https://svn.apache.org/viewvc?view=revision=1752008
Regards,
Paul Spangler
LabVIEW R
National Instruments
Re: Issues w/ sessions on trunk?
On 1/8/2016 6:49 AM, Jim Jagielski wrote: Just noticed that the test framework reports issues w/ sessions on trunk: t/modules/session.t . 1/105 # Failed test 8 in t/modules/session.t at line 63 fail #2 # Failed test 18 in t/modules/session.t at line 63 fail #4 # Failed test 38 in t/modules/session.t at line 63 fail #8 # Failed test 43 in t/modules/session.t at line 63 fail #9 # Failed test 48 in t/modules/session.t at line 63 fail #10 # Failed test 53 in t/modules/session.t at line 63 fail #11 *TODO* # Failed test 54 in t/modules/session.t at line 65 fail #11 *TODO* # Failed test 58 in t/modules/session.t at line 63 fail #12 # Failed test 63 in t/modules/session.t at line 63 fail #13 # Failed test 88 in t/modules/session.t at line 63 fail #18 *TODO* # Failed test 89 in t/modules/session.t at line 65 fail #18 *TODO* # Failed test 98 in t/modules/session.t at line 63 fail #20 (of course, I am ignoring the TODOs) Can anyone confirm? I believe this is because of the patch committed for PR 57300 (r1709121), but the test changes that go with it (attached to the bug report) did not get committed. Regards, -- Paul Spangler LabVIEW R National Instruments
Re: Apache Module Development Query on character encodings.
On 10/20/2015 2:23 PM, John Dougrez-Lewis wrote: How do I go about massaging the input & output into UTF-8 and fixed width 16-bit Unicode? Are there any good references on how to achieve this? I can't speak for whether or not it's a good reference, but mod_authnz_ldap can do the "Request Input character encoding => UTF-8" part of what you described to convert usernames and passwords to UTF-8 before passing them to LDAP. It uses the apr_xlate API along with a file specified by the AuthLDAPCharsetConfig directive to map the Accept-Language header to a character encoding. I believe the xlate API is using iconv under the hood, which can be its own source of problems if the server environment isn't set up properly. I'd also be interested to hear if there are other ways for modules to handle character encodings, which is never an easy topic. I suppose ideally the protocol would dictate a required encoding that the client must use (via Content-Types), which simplifies things quite a bit. Regards, Paul Spangler LabVIEW R National Instruments
Re: [PATCH 57300] mod_session save optimization
On 8/20/2015 4:58 PM, Paul Spangler wrote: Hello, The bug report contains a more detailed explanation of the patch, but there are some points I thought might lead to some discussion. First a quick summary of the issue: mod_session writes out the session every request even if there aren't any changes to the data. This makes some sense when the session has a max age and the expiry value needs to be refreshed. However, there isn't likely to be much benefit in repeatedly refreshing the expiry by a few milliseconds, possibly generating database traffic each time. This patch proposes a new directive to define an interval of time where the expiry doesn't need to be refreshed if there are no session data changes. 1. We had a hard time coming up with a name for the directive. The patch goes with SessionExpiryUpdateInterval, being the interval of time that may pass before updating the expiry value is required. I don't know if there are any existing directives with a similar function that we should mimic instead. 2. The patch includes a behavior change independent of the new directive when using sessions without a max age: if the data hasn't changed, don't write out the session. Most noticeably, this means new sessions that never get data are discarded without being saved. 3. I wasn't sure how best to add tests for a new directive since the test server won't start if the directive is missing. The patch that includes the test changes look for the 2.5 version to know the new directive is there, and will require a modification if/when the directive is back-ported to 2.4 to enable the new tests. https://bz.apache.org/bugzilla/show_bug.cgi?id=57300 Thanks for your consideration. Bump in case anyone is interested now that the list has died down a bit. -- Paul Spangler LabVIEW R National Instruments
Re: [PATCH 57300] mod_session save optimization
On 8/20/2015 4:58 PM, Paul Spangler wrote: Hello, The bug report contains a more detailed explanation of the patch, but there are some points I thought might lead to some discussion. First a quick summary of the issue: mod_session writes out the session every request even if there aren't any changes to the data. This makes some sense when the session has a max age and the expiry value needs to be refreshed. However, there isn't likely to be much benefit in repeatedly refreshing the expiry by a few milliseconds, possibly generating database traffic each time. This patch proposes a new directive to define an interval of time where the expiry doesn't need to be refreshed if there are no session data changes. 1. We had a hard time coming up with a name for the directive. The patch goes with SessionExpiryUpdateInterval, being the interval of time that may pass before updating the expiry value is required. I don't know if there are any existing directives with a similar function that we should mimic instead. 2. The patch includes a behavior change independent of the new directive when using sessions without a max age: if the data hasn't changed, don't write out the session. Most noticeably, this means new sessions that never get data are discarded without being saved. 3. I wasn't sure how best to add tests for a new directive since the test server won't start if the directive is missing. The patch that includes the test changes look for the 2.5 version to know the new directive is there, and will require a modification if/when the directive is back-ported to 2.4 to enable the new tests. https://bz.apache.org/bugzilla/show_bug.cgi?id=57300 Thanks for your consideration. Quick ping on this. -- Paul Spangler LabVIEW R National Instruments
Re: [PATCH 57300] mod_session save optimization
On 8/20/2015 4:58 PM, Paul Spangler wrote: Hello, The bug report contains a more detailed explanation of the patch, but there are some points I thought might lead to some discussion. First a quick summary of the issue: mod_session writes out the session every request even if there aren't any changes to the data. This makes some sense when the session has a max age and the expiry value needs to be refreshed. However, there isn't likely to be much benefit in repeatedly refreshing the expiry by a few milliseconds, possibly generating database traffic each time. This patch proposes a new directive to define an interval of time where the expiry doesn't need to be refreshed if there are no session data changes. 1. We had a hard time coming up with a name for the directive. The patch goes with SessionExpiryUpdateInterval, being the interval of time that may pass before updating the expiry value is required. I don't know if there are any existing directives with a similar function that we should mimic instead. 2. The patch includes a behavior change independent of the new directive when using sessions without a max age: if the data hasn't changed, don't write out the session. Most noticeably, this means new sessions that never get data are discarded without being saved. 3. I wasn't sure how best to add tests for a new directive since the test server won't start if the directive is missing. The patch that includes the test changes look for the 2.5 version to know the new directive is there, and will require a modification if/when the directive is back-ported to 2.4 to enable the new tests. https://bz.apache.org/bugzilla/show_bug.cgi?id=57300 Thanks for your consideration. Quick ping on this. -- Paul Spangler LabVIEW R National Instruments
Re: [PATCH 57300] mod_session save optimization
On 8/20/2015 4:58 PM, Paul Spangler wrote: Hello, The bug report contains a more detailed explanation of the patch, but there are some points I thought might lead to some discussion. First a quick summary of the issue: mod_session writes out the session every request even if there aren't any changes to the data. This makes some sense when the session has a max age and the expiry value needs to be refreshed. However, there isn't likely to be much benefit in repeatedly refreshing the expiry by a few milliseconds, possibly generating database traffic each time. This patch proposes a new directive to define an interval of time where the expiry doesn't need to be refreshed if there are no session data changes. 1. We had a hard time coming up with a name for the directive. The patch goes with SessionExpiryUpdateInterval, being the interval of time that may pass before updating the expiry value is required. I don't know if there are any existing directives with a similar function that we should mimic instead. 2. The patch includes a behavior change independent of the new directive when using sessions without a max age: if the data hasn't changed, don't write out the session. Most noticeably, this means new sessions that never get data are discarded without being saved. 3. I wasn't sure how best to add tests for a new directive since the test server won't start if the directive is missing. The patch that includes the test changes look for the 2.5 version to know the new directive is there, and will require a modification if/when the directive is back-ported to 2.4 to enable the new tests. https://bz.apache.org/bugzilla/show_bug.cgi?id=57300 Thanks for your consideration. Bump. Anyone interested? -- Paul Spangler LabVIEW R National Instruments
[PATCH 57300] mod_session save optimization
Hello, The bug report contains a more detailed explanation of the patch, but there are some points I thought might lead to some discussion. First a quick summary of the issue: mod_session writes out the session every request even if there aren't any changes to the data. This makes some sense when the session has a max age and the expiry value needs to be refreshed. However, there isn't likely to be much benefit in repeatedly refreshing the expiry by a few milliseconds, possibly generating database traffic each time. This patch proposes a new directive to define an interval of time where the expiry doesn't need to be refreshed if there are no session data changes. 1. We had a hard time coming up with a name for the directive. The patch goes with SessionExpiryUpdateInterval, being the interval of time that may pass before updating the expiry value is required. I don't know if there are any existing directives with a similar function that we should mimic instead. 2. The patch includes a behavior change independent of the new directive when using sessions without a max age: if the data hasn't changed, don't write out the session. Most noticeably, this means new sessions that never get data are discarded without being saved. 3. I wasn't sure how best to add tests for a new directive since the test server won't start if the directive is missing. The patch that includes the test changes look for the 2.5 version to know the new directive is there, and will require a modification if/when the directive is back-ported to 2.4 to enable the new tests. https://bz.apache.org/bugzilla/show_bug.cgi?id=57300 Thanks for your consideration. -- Paul Spangler LabVIEW RD National Instruments
Re: svn commit: r1693140 - in /httpd/test/framework/trunk: c-modules/test_session/ t/htdocs/modules/session/ t/modules/
On 7/29/2015 9:45 AM, Edward Lu wrote: Whoops, sorry, fixed. On Tue, Jul 28, 2015 at 2:59 PM, Eric Covener cove...@gmail.com mailto:cove...@gmail.com wrote: On Tue, Jul 28, 2015 at 2:52 PM, e...@apache.org mailto:e...@apache.org wrote: Added: httpd/test/framework/trunk/c-modules/test_session/ httpd/test/framework/trunk/c-modules/test_session/Makefile (with props) httpd/test/framework/trunk/c-modules/test_session/mod_test_session.c (with props) httpd/test/framework/trunk/c-modules/test_session/mod_test_session.slo Some generated files -- Eric Covener cove...@gmail.com mailto:cove...@gmail.com Thank you for reviewing and committing those tests so quickly! I think the test_session directory is missing the svn:ignore prop: $ svn propget svn:ignore c-modules/test_session $ svn propget svn:ignore c-modules/test_ssl Makefile .libs *.lo *.slo *.la *.so *.exp *.lib -- Paul Spangler LabVIEW RD National Instruments
[PATCH 58172] mod_session tests
Hello, I've been reviewing the open bugs for mod_session and am interesting in contributing patches for some of them. But I couldn't find any tests for mod_session in httpd-test to verify changes and avoid regressions. So I filed a bug report with a patch that includes some functional tests for mod_session. I couldn't find too many similar patches, but hopefully this is the correct way to go about it. https://bz.apache.org/bugzilla/show_bug.cgi?id=58172 Thank you for your time! -- Paul Spangler LabVIEW RD National Instruments
