Oops, forgot to attach the diff file. It is attached here:
On Fri, 17 Oct 2003, Norman Tuttle wrote:
> This new patch for flood_socket_keepalive.c fixes some issues discovered
> during opening and closing sockets. Predicated on the changes previously
> made to the socket structure (reference my email with the 6 diffs), the
> reopen_socket member, which has now been changed to "available", now takes
> on an ordered enum of values. The keepalive_begin_conn() function has been
> modified to properly handle error conditions and transitions, which can
> occur between pages using ssl or no ssl, different hosts, or different
> ports and require closure of the previous connection.
>
> Other changes: keepalive_end_conn() also contains logic to prevent certain
> error conditions from crashing the function. The setting of the
> socket's associated request, which was established in the patch mentioned
> above, has been moved to the case where sockets are not being opened,
> since otherwise the open_socket() or ssl_open_socket() function sets the
> request.
>
> Please review the attached diff, which compares the latest
> flood_socket_keepalive.c file with the version produced from the patch
> creating the unified socket structure, which is named
> flood_socket_keepalive_2.c
>
> -Norman Tuttle, Developer, OpenDemand Systems ([EMAIL PROTECTED])
>
>
>
>
--- flood_socket_keepalive.c 2003-10-17 11:22:20.000000000 -0400
+++ flood_socket_keepalive_2.c 2003-10-15 19:06:44.000000000 -0400
@@ -76,18 +76,6 @@
ksock->ssl ? ssl_write_socket(ksock, req) : \
write_socket(ksock, req)
-#define does_socket_need_reopen(ssl, ksock, req) \
- ((ssl!=ksock->ssl)||(req->parsed_uri->port!=ksock->port)|| \
- (strcasecmp(req->parsed_uri->hostname, ksock->hostname)))
-/* case of either mismatch in ssl scheme, port, or hostname */
-
-enum
-{
- NOT_AVAILABLE,
- AVAILABLE_OPEN,
- AVAILABLE_CLOSED
-};
-
/**
* Keep-alive implementation for socket_init.
*/
@@ -99,7 +87,7 @@
if (new_ksock == NULL)
return APR_ENOMEM;
new_ksock->socket = NULL;
- new_ksock->available = AVAILABLE_CLOSED;
+ new_ksock->available = 1;
new_ksock->wantresponse = 1;
new_ksock->ssl = 0;
@@ -112,55 +100,44 @@
*/
apr_status_t keepalive_begin_conn(socket_t *sock, request_t *req, apr_pool_t
*pool)
{
- apr_status_t status = APR_SUCCESS;
flood_socket_t *ksock = (flood_socket_t *)sock;
- int ssl = 0; /* new value for ssl, set up in next 10 lines */
- if (strcasecmp(req->parsed_uri->scheme, "https") == 0)
- {
- /* If we don't have SSL, error out. */
-#if FLOOD_HAS_OPENSSL
- ssl = 1;
-#else
- return APR_ENOTIMPL;
-#endif
- }
- /* If socket open, !ksock->socket is bogus state but don't close. */
- if ((ksock->available == AVAILABLE_OPEN)&&(ksock->socket)&&
- ((does_socket_need_reopen(ssl, ksock, req))||
- /* above line includes test for compatible ssl, port, host, etc. */
- (check_socket(ksock) != APR_SUCCESS))) /* checks socket integrity */
- { /* first make sure it's closed! */
- ksock->ssl ? ssl_close_socket(ksock) : close_socket(ksock);
- ksock->available = AVAILABLE_CLOSED;
+ if (!ksock->available && ksock->socket) {
+ apr_status_t e;
+ e = check_socket(ksock);
+ if (e != APR_SUCCESS) {
+ ksock->available = 1;
+ }
}
- ksock->ssl = ssl;
- if ((ksock->available==AVAILABLE_CLOSED) || (!ksock->socket))
- { /* this logic handles above bogus condition. */
+ if (ksock->available || ksock->socket == NULL) {
+ apr_status_t rv;
+ if (strcasecmp(req->parsed_uri->scheme, "https") == 0) {
+ /* If we don't have SSL, error out. */
#if FLOOD_HAS_OPENSSL
- if (ssl) ssl_open_socket(ksock, req, &status);
- else
+ ksock->ssl = 1;
+#else
+ return APR_ENOTIMPL;
#endif
- open_socket(ksock, req, &status);
- }
- else /* Problem: keepalive introduces poss of begin_connect without socket/
- * request linkage through (ssl_)open_socket. Soln: In this case */
- { /* (available==AVAILABLE_OPEN), manually create the link here: */
- ksock->request = req;
+ }
+ else {
+ ksock->ssl = 0;
+ }
+
+ /* The return types are not identical, so it can't be a ternary
+ * operation. */
+ if (ksock->ssl)
+ ssl_open_socket(ksock, req, &rv);
+ else
+ open_socket(ksock, req, &rv);
+
+ if (ksock->socket == NULL)
+ return rv;
+
+ ksock->available = 0; /* we just opened it */
}
- if (!ksock->socket)
- {
- ksock->available = AVAILABLE_CLOSED; /* Errors-reopen socket next time */
- if (status == APR_SUCCESS)
- { /* There is still some error, although did not get returned in open. */
- status = APR_EGENERAL; /* opening null socket would be problematic. */
- }
- }
- if (status==APR_SUCCESS) ksock->available = NOT_AVAILABLE; /* socket now
opened and IN USE */
- ksock->hostname = req->parsed_uri->hostname;
- ksock->port = req->parsed_uri->port;
+ ksock->request = req;
req->keepalive = 1;
- return status;
+ return APR_SUCCESS;
}
/**
@@ -591,12 +568,11 @@
{
flood_socket_t *ksock = (flood_socket_t *)sock;
- if (ksock && (ksock->socket) && (!resp || (resp->keepalive == 0)))
- { /* This logic will leave the socket open if still keepalive. */
+ if (resp->keepalive == 0) {
ksock->ssl ? ssl_close_socket(ksock) : close_socket(ksock);
- ksock->available = AVAILABLE_CLOSED; /* we just closed it */
+ ksock->available = 1; /* we just closed it */
}
- else ksock->available = AVAILABLE_OPEN;
+
return APR_SUCCESS;
}
