You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2022/01/25 15:54:22 UTC
svn commit: r1897458 - in /httpd/httpd/trunk: changes-entries/ab-ssl-sense-fix.txt support/ab.c
Author: minfrin
Date: Tue Jan 25 15:54:22 2022
New Revision: 1897458
URL: http://svn.apache.org/viewvc?rev=1897458&view=rev
Log:
ab: Respond appropriately to SSL_ERROR_WANT_READ and SSL_ERROR_WANT_WRITE.
Previously the correct event was polled for, but the response to the poll
would call write instead of read, and read instead of write. PR 55952
Added:
httpd/httpd/trunk/changes-entries/ab-ssl-sense-fix.txt
Modified:
httpd/httpd/trunk/support/ab.c
Added: httpd/httpd/trunk/changes-entries/ab-ssl-sense-fix.txt
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/changes-entries/ab-ssl-sense-fix.txt?rev=1897458&view=auto
==============================================================================
--- httpd/httpd/trunk/changes-entries/ab-ssl-sense-fix.txt (added)
+++ httpd/httpd/trunk/changes-entries/ab-ssl-sense-fix.txt Tue Jan 25 15:54:22 2022
@@ -0,0 +1,5 @@
+ *) ab: Respond appropriately to SSL_ERROR_WANT_READ and SSL_ERROR_WANT_WRITE.
+ Previously the correct event was polled for, but the response to the poll
+ would call write instead of read, and read instead of write. PR 55952
+ [Graham Leggett]
+
Modified: httpd/httpd/trunk/support/ab.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/support/ab.c?rev=1897458&r1=1897457&r2=1897458&view=diff
==============================================================================
--- httpd/httpd/trunk/support/ab.c (original)
+++ httpd/httpd/trunk/support/ab.c Tue Jan 25 15:54:22 2022
@@ -237,7 +237,11 @@ typedef enum {
* know if it worked yet
*/
STATE_CONNECTED, /* we know TCP connect completed */
- STATE_READ
+#ifdef USE_SSL
+ STATE_HANDSHAKE, /* in the handshake phase */
+#endif
+ STATE_WRITE, /* in the write phase */
+ STATE_READ /* in the read phase */
} connect_state_e;
#define CBUFFSIZE (8192)
@@ -520,21 +524,13 @@ static void set_polled_events(struct con
}
}
-static void set_conn_state(struct connection *c, connect_state_e new_state)
+static void set_conn_state(struct connection *c, connect_state_e new_state,
+ apr_int16_t events)
{
- apr_int16_t events_by_state[] = {
- 0, /* for STATE_UNCONNECTED */
- APR_POLLOUT, /* for STATE_CONNECTING */
- APR_POLLIN, /* for STATE_CONNECTED; we don't poll in this state,
- * so prepare for polling in the following state --
- * STATE_READ
- */
- APR_POLLIN /* for STATE_READ */
- };
c->state = new_state;
- set_polled_events(c, events_by_state[new_state]);
+ set_polled_events(c, events);
}
/* --------------------------------------------------------- */
@@ -707,7 +703,7 @@ static void ssl_print_info(struct connec
}
ssl_print_connection_info(bio_err,c->ssl);
SSL_SESSION_print(bio_err, SSL_get_session(c->ssl));
- }
+}
static void ssl_proceed_handshake(struct connection *c)
{
@@ -783,14 +779,19 @@ static void ssl_proceed_handshake(struct
}
#endif
write_request(c);
+
do_next = 0;
break;
case SSL_ERROR_WANT_READ:
- set_polled_events(c, APR_POLLIN);
+
+ set_conn_state(c, STATE_HANDSHAKE, APR_POLLIN);
+
do_next = 0;
break;
case SSL_ERROR_WANT_WRITE:
- set_polled_events(c, APR_POLLOUT);
+
+ set_conn_state(c, STATE_HANDSHAKE, APR_POLLOUT);
+
do_next = 0;
break;
case SSL_ERROR_WANT_CONNECT:
@@ -810,9 +811,6 @@ static void ssl_proceed_handshake(struct
static void write_request(struct connection * c)
{
- if (started >= requests) {
- return;
- }
do {
apr_time_t tnow;
@@ -845,10 +843,14 @@ static void write_request(struct connect
if (e <= 0) {
switch (SSL_get_error(c->ssl, e)) {
case SSL_ERROR_WANT_READ:
- set_polled_events(c, APR_POLLIN);
+
+ set_conn_state(c, STATE_WRITE, APR_POLLIN);
+
break;
case SSL_ERROR_WANT_WRITE:
- set_polled_events(c, APR_POLLOUT);
+
+ set_conn_state(c, STATE_WRITE, APR_POLLOUT);
+
break;
default:
BIO_printf(bio_err, "SSL write failed - closing connection\n");
@@ -871,7 +873,7 @@ static void write_request(struct connect
close_connection(c);
}
else {
- set_polled_events(c, APR_POLLOUT);
+ set_conn_state(c, STATE_WRITE, APR_POLLOUT);
}
return;
}
@@ -883,7 +885,8 @@ static void write_request(struct connect
c->endwrite = lasttime = apr_time_now();
started++;
- set_conn_state(c, STATE_READ);
+
+ set_conn_state(c, STATE_READ, APR_POLLIN);
}
/* --------------------------------------------------------- */
@@ -1355,8 +1358,9 @@ static void start_connect(struct connect
{
apr_status_t rv;
- if (!(started < requests))
+ if (!(started < requests)) {
return;
+ }
c->read = 0;
c->bread = 0;
@@ -1435,12 +1439,12 @@ static void start_connect(struct connect
#endif
if ((rv = apr_socket_connect(c->aprsock, destsa)) != APR_SUCCESS) {
if (APR_STATUS_IS_EINPROGRESS(rv)) {
- set_conn_state(c, STATE_CONNECTING);
+ set_conn_state(c, STATE_CONNECTING, APR_POLLOUT);
c->rwrite = 0;
return;
}
else {
- set_conn_state(c, STATE_UNCONNECTED);
+ set_conn_state(c, STATE_UNCONNECTED, 0);
apr_socket_close(c->aprsock);
if (good == 0 && destsa->next) {
destsa = destsa->next;
@@ -1461,7 +1465,6 @@ static void start_connect(struct connect
}
/* connected first time */
- set_conn_state(c, STATE_CONNECTED);
#ifdef USE_SSL
if (c->ssl) {
ssl_proceed_handshake(c);
@@ -1509,7 +1512,7 @@ static void close_connection(struct conn
}
}
- set_conn_state(c, STATE_UNCONNECTED);
+ set_conn_state(c, STATE_UNCONNECTED, 0);
#ifdef USE_SSL
if (c->ssl) {
SSL_shutdown(c->ssl);
@@ -1567,10 +1570,14 @@ read_more:
return;
}
else if (scode == SSL_ERROR_WANT_READ) {
- set_polled_events(c, APR_POLLIN);
+
+ set_conn_state(c, STATE_READ, APR_POLLIN);
+
}
else if (scode == SSL_ERROR_WANT_WRITE) {
- set_polled_events(c, APR_POLLOUT);
+
+ set_conn_state(c, STATE_READ, APR_POLLOUT);
+
}
else {
/* some fatal error: */
@@ -1664,7 +1671,7 @@ read_more:
}
else {
/* header is in invalid or too big - close connection */
- set_conn_state(c, STATE_UNCONNECTED);
+ set_conn_state(c, STATE_UNCONNECTED, 0);
apr_socket_close(c->aprsock);
err_response++;
if (bad++ > 10) {
@@ -1754,7 +1761,13 @@ read_more:
goto read_more;
}
- if (c->keepalive && (c->bread >= c->length)) {
+ /* are we done? */
+ if (started >= requests && (c->bread >= c->length)) {
+ close_connection(c);
+ }
+
+ /* are we keepalive? if so, reuse existing connection */
+ else if (c->keepalive && (c->bread >= c->length)) {
/* finished a keep-alive connection */
good++;
/* save out time */
@@ -1786,7 +1799,7 @@ read_more:
c->read = c->bread = 0;
/* zero connect time with keep-alive */
c->start = c->connect = lasttime = apr_time_now();
- set_conn_state(c, STATE_CONNECTED);
+
write_request(c);
}
}
@@ -1976,6 +1989,7 @@ static void test(void)
do {
status = apr_pollset_poll(readbits, aprtimeout, &n, &pollresults);
} while (APR_STATUS_IS_EINTR(status));
+
if (status != APR_SUCCESS)
apr_err("apr_pollset_poll", status);
@@ -2011,8 +2025,23 @@ static void test(void)
* connection is done and we loop here endlessly calling
* apr_poll().
*/
- if ((rtnev & APR_POLLIN) || (rtnev & APR_POLLPRI) || (rtnev & APR_POLLHUP))
- read_connection(c);
+ if ((rtnev & APR_POLLIN) || (rtnev & APR_POLLPRI) || (rtnev & APR_POLLHUP)) {
+
+ switch (c->state) {
+#ifdef USE_SSL
+ case STATE_HANDSHAKE:
+ ssl_proceed_handshake(c);
+ break;
+#endif
+ case STATE_WRITE:
+ write_request(c);
+ break;
+ case STATE_READ:
+ read_connection(c);
+ break;
+ }
+
+ }
if ((rtnev & APR_POLLERR) || (rtnev & APR_POLLNVAL)) {
if (destsa->next && c->state == STATE_CONNECTING && good == 0) {
destsa = destsa->next;
@@ -2036,7 +2065,7 @@ static void test(void)
/* call connect() again to detect errors */
rv = apr_socket_connect(c->aprsock, destsa);
if (rv != APR_SUCCESS) {
- set_conn_state(c, STATE_UNCONNECTED);
+ set_conn_state(c, STATE_UNCONNECTED, 0);
apr_socket_close(c->aprsock);
err_conn++;
if (bad++ > 10) {
@@ -2048,7 +2077,7 @@ static void test(void)
continue;
}
else {
- set_conn_state(c, STATE_CONNECTED);
+
#ifdef USE_SSL
if (c->ssl)
ssl_proceed_handshake(c);
@@ -2056,16 +2085,24 @@ static void test(void)
#endif
write_request(c);
}
+
}
else {
- /* POLLOUT is one shot */
- set_polled_events(c, APR_POLLIN);
- if (c->state == STATE_READ) {
- read_connection(c);
- }
- else {
+
+ switch (c->state) {
+#ifdef USE_SSL
+ case STATE_HANDSHAKE:
+ ssl_proceed_handshake(c);
+ break;
+#endif
+ case STATE_WRITE:
write_request(c);
+ break;
+ case STATE_READ:
+ read_connection(c);
+ break;
}
+
}
}
}
@@ -2682,7 +2719,7 @@ int main(int argc, const char * const ar
if (ssl_cert != NULL) {
if (SSL_CTX_use_certificate_chain_file(ssl_ctx, ssl_cert) <= 0) {
BIO_printf(bio_err, "unable to get certificate from '%s'\n",
- ssl_cert);
+ ssl_cert);
ERR_print_errors(bio_err);
exit(1);
}
Re: svn commit: r1897458 - in /httpd/httpd/trunk: changes-entries/ab-ssl-sense-fix.txt support/ab.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 1/25/22 4:54 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Tue Jan 25 15:54:22 2022
> New Revision: 1897458
>
> URL: http://svn.apache.org/viewvc?rev=1897458&view=rev
> Log:
> ab: Respond appropriately to SSL_ERROR_WANT_READ and SSL_ERROR_WANT_WRITE.
> Previously the correct event was polled for, but the response to the poll
> would call write instead of read, and read instead of write. PR 55952
>
> Added:
> httpd/httpd/trunk/changes-entries/ab-ssl-sense-fix.txt
> 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=1897458&r1=1897457&r2=1897458&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/support/ab.c (original)
> +++ httpd/httpd/trunk/support/ab.c Tue Jan 25 15:54:22 2022
> @@ -810,9 +811,6 @@ static void ssl_proceed_handshake(struct
>
> static void write_request(struct connection * c)
> {
> - if (started >= requests) {
> - return;
> - }
Why is this no longer needed?
>
> do {
> apr_time_t tnow;
> @@ -1461,7 +1465,6 @@ static void start_connect(struct connect
> }
>
> /* connected first time */
> - set_conn_state(c, STATE_CONNECTED);
Why don't we set the state to connected any longer?
> #ifdef USE_SSL
> if (c->ssl) {
> ssl_proceed_handshake(c);
> @@ -1786,7 +1799,7 @@ read_more:
> c->read = c->bread = 0;
> /* zero connect time with keep-alive */
> c->start = c->connect = lasttime = apr_time_now();
> - set_conn_state(c, STATE_CONNECTED);
Why don't we set the state to connected any longer?
> +
> write_request(c);
> }
> }
> @@ -2048,7 +2077,7 @@ static void test(void)
> continue;
> }
> else {
> - set_conn_state(c, STATE_CONNECTED);
Why don't we set the state to connected any longer?
> +
> #ifdef USE_SSL
> if (c->ssl)
> ssl_proceed_handshake(c);
Regards
Rüdiger