ID: 44393 Comment by: Richard dot Krehbiel at gmail dot com Reported By: richard dot krehbiel at gmail dot com Status: Open Bug Type: Feature/Change Request Operating System: Windows PHP Version: 5.2.5 New Comment:
I haven't been paying attention. The patch is badly word-wrapped, and so doesn't apply. Here's a link to an unformatted plain-text version: http://home.comcast.net/~krehbiel3/php5isapi.c.patch Previous Comments: ------------------------------------------------------------------------ [2008-03-26 17:35:32] Richard dot Krehbiel at gmail dot com New feature: It now detects the presence of a "Content-Length" header and declines to emit "chunked" transfer encoding. I also tried to make the code use more of the conventions established in the original module. Note: It works great with zlib.output_compression. However (and I'm sure this is not news to many of you) supplying a Content-Length header mixed with compression will almost certainly do the wrong thing. The Content-Length must supply the compressed size, which your script will almost certainly be unaware of. So, compression will definitely work best with chunked - meaning, don't supply Content-Length. The latest patch version: --- /mnt/rich3/c/php-5.2.5/sapi/isapi/php5isapi.c 2007-02-23 17:08:30.000000000 -0500 +++ /mnt/rich3/c/buildphp/php-5.2.5/sapi/isapi/php5isapi.c 2008-03-26 13:25:10.000000000 -0400 @@ -143,6 +143,19 @@ NULL }; +typedef struct +{ + int chunked; /* Whether to send the data "chunked" */ +} PHP_STREAM_INFO; + +ts_rsrc_id tls_php_stream_info; + +static void php_stream_info_ctor(void *vsi, void ***foo) { + memset(vsi, 0, sizeof(PHP_STREAM_INFO)); +} + +static void php_stream_info_dtor(void *vsi, void ***foo) { +} static void php_info_isapi(ZEND_MODULE_INFO_FUNC_ARGS) { @@ -206,11 +219,36 @@ { DWORD num_bytes = str_length; LPEXTENSION_CONTROL_BLOCK ecb; - + /* For chunked write */ + char chunksize[16]; + uint chunksizelen; + PHP_STREAM_INFO *phpinfo; + ecb = (LPEXTENSION_CONTROL_BLOCK) SG(server_context); - if (ecb->WriteClient(ecb->ConnID, (char *) str, &num_bytes, HSE_IO_SYNC) == FALSE) { - php_handle_aborted_connection(); + + phpinfo = (PHP_STREAM_INFO*)ts_resource(tls_php_stream_info); + + if(phpinfo->chunked) { + if(str_length > 0) { + uint two = 2; + _snprintf(chunksize, sizeof(chunksize), "%lX\r\n", str_length); + chunksizelen = strlen(chunksize); + if (ecb->WriteClient(ecb->ConnID, chunksize, &chunksizelen, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } + if (ecb->WriteClient(ecb->ConnID, (char *) str, &num_bytes, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } + if (ecb->WriteClient(ecb->ConnID, "\r\n", &two, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } + } + } else { + if (ecb->WriteClient(ecb->ConnID, (char *) str, &num_bytes, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } } + return num_bytes; } @@ -220,52 +258,82 @@ return SAPI_HEADER_ADD; } - - -static void accumulate_header_length(sapi_header_struct *sapi_header, uint *total_length TSRMLS_DC) +struct header_params { - *total_length += sapi_header->header_len+2; -} - + char *combined_headers_ptr; /* pointer to headers, Advances during contat_header calls */ + int content_length; /* Indicates whether Content-Length: was encountered while counting up headers' total length */ + int total_length; /* Accumulates headers' total length */ +}; -static void concat_header(sapi_header_struct *sapi_header, char **combined_headers_ptr TSRMLS_DC) +static void accumulate_header_length(sapi_header_struct *sapi_header, struct header_params *params TSRMLS_DC) { - memcpy(*combined_headers_ptr, sapi_header->header, sapi_header->header_len); - *combined_headers_ptr += sapi_header->header_len; - **combined_headers_ptr = '\r'; - (*combined_headers_ptr)++; - **combined_headers_ptr = '\n'; - (*combined_headers_ptr)++; + params->total_length += sapi_header->header_len+2; + /* Make a note here if the script provided a Content-Length: header. */ + if(_strnicmp(sapi_header->header, "Content-Length:", 15) == 0) + params->content_length = 1; +} + +static void concat_header(sapi_header_struct *sapi_header, struct header_params *params TSRMLS_DC) +{ + memcpy(params->combined_headers_ptr, sapi_header->header, sapi_header->header_len); + params->combined_headers_ptr += sapi_header->header_len; + *params->combined_headers_ptr = '\r'; + (params->combined_headers_ptr)++; + *params->combined_headers_ptr = '\n'; + (params->combined_headers_ptr)++; } - static int sapi_isapi_send_headers(sapi_headers_struct *sapi_headers TSRMLS_DC) { - uint total_length = 2; /* account for the trailing \r\n */ - char *combined_headers, *combined_headers_ptr; + char *combined_headers; LPEXTENSION_CONTROL_BLOCK lpECB = (LPEXTENSION_CONTROL_BLOCK) SG(server_context); - HSE_SEND_HEADER_EX_INFO header_info; + HSE_SEND_HEADER_EX_INFO header_info = { 0, }; sapi_header_struct default_content_type; + sapi_header_struct chunked_transfer = { "Transfer-Encoding: chunked", 26, }; char *status_buf = NULL; + PHP_STREAM_INFO *phpinfo = (PHP_STREAM_INFO*)ts_resource(tls_php_stream_info); + + struct header_params hdrinfo = { NULL, }; + + /* HTTP/1.0 requestors don't get "chunked" replies */ + { + char protocol[64]; + DWORD protosize = sizeof(protocol); + + lpECB->GetServerVariable(lpECB->ConnID, "SERVER_PROTOCOL", protocol, &protosize); + phpinfo->chunked = (strcmp(protocol, "HTTP/1.0") != 0); + } /* Obtain headers length */ if (SG(sapi_headers).send_default_content_type) { sapi_get_default_content_type_header(&default_content_type TSRMLS_CC); - accumulate_header_length(&default_content_type, (void *) &total_length TSRMLS_CC); + accumulate_header_length(&default_content_type, (void *) &hdrinfo TSRMLS_CC); } - zend_llist_apply_with_argument(&SG(sapi_headers).headers, (llist_apply_with_arg_func_t) accumulate_header_length, (void *) &total_length TSRMLS_CC); + + zend_llist_apply_with_argument(&SG(sapi_headers).headers, (llist_apply_with_arg_func_t) accumulate_header_length, + (void *) &hdrinfo TSRMLS_CC); + + /* If the script supplied a Content-Length header, I decline to do "chunked" */ + if(hdrinfo.content_length) + phpinfo->chunked = 0; + + if(phpinfo->chunked) + accumulate_header_length(&chunked_transfer, &hdrinfo TSRMLS_CC); /* Generate headers */ - combined_headers = (char *) emalloc(total_length+1); - combined_headers_ptr = combined_headers; + combined_headers = (char *) emalloc(hdrinfo.total_length+4); /* +4 is for "\r\n\0" */ + hdrinfo.combined_headers_ptr = combined_headers; if (SG(sapi_headers).send_default_content_type) { - concat_header(&default_content_type, (void *) &combined_headers_ptr TSRMLS_CC); + concat_header(&default_content_type, (void *) &hdrinfo TSRMLS_CC); sapi_free_header(&default_content_type); /* we no longer need it */ } - zend_llist_apply_with_argument(&SG(sapi_headers).headers, (llist_apply_with_arg_func_t) concat_header, (void *) &combined_headers_ptr TSRMLS_CC); - *combined_headers_ptr++ = '\r'; - *combined_headers_ptr++ = '\n'; - *combined_headers_ptr = 0; + zend_llist_apply_with_argument(&SG(sapi_headers).headers, (llist_apply_with_arg_func_t) concat_header, (void *) &hdrinfo TSRMLS_CC); + + if(phpinfo->chunked) + concat_header(&chunked_transfer, &hdrinfo TSRMLS_CC); + + /* headers end with a blank line */ + strcpy(hdrinfo.combined_headers_ptr, "\r\n"); switch (SG(sapi_headers).http_response_code) { case 200: @@ -299,8 +367,10 @@ } header_info.cchStatus = strlen(header_info.pszStatus); header_info.pszHeader = combined_headers; - header_info.cchHeader = total_length; - header_info.fKeepConn = FALSE; + header_info.cchHeader = hdrinfo.total_length + 2; + /* Can I keep the connection open? Only say "yes" if either "chunked" or "content-length", else no. */ + /* Note that I won't support HTTP/1.0 Keep-Alive. */ + header_info.fKeepConn = phpinfo->chunked || hdrinfo.content_length; lpECB->dwHttpStatusCode = SG(sapi_headers).http_response_code; lpECB->ServerSupportFunction(lpECB->ConnID, HSE_REQ_SEND_RESPONSE_HEADER_EX, &header_info, NULL, NULL); @@ -780,7 +850,7 @@ static void php_isapi_report_exception(char *message, int message_len TSRMLS_DC) { if (!SG(headers_sent)) { - HSE_SEND_HEADER_EX_INFO header_info; + HSE_SEND_HEADER_EX_INFO header_info = { 0, }; LPEXTENSION_CONTROL_BLOCK lpECB = (LPEXTENSION_CONTROL_BLOCK) SG(server_context); header_info.pszStatus = "500 Internal Server Error"; @@ -836,8 +906,11 @@ #ifdef PHP_ENABLE_SEH LPEXCEPTION_POINTERS e; #endif + PHP_STREAM_INFO *phpinfo; TSRMLS_FETCH(); + phpinfo = (PHP_STREAM_INFO*)ts_resource(tls_php_stream_info); + zend_first_try { #ifdef PHP_ENABLE_SEH __try { @@ -928,6 +1001,15 @@ return HSE_STATUS_ERROR; } zend_end_try(); + /* Finish a chunked transmission, send 0 length EOF chunk and trailing headers (none) */ + + if(phpinfo->chunked) { + uint five = 5; + if (lpECB->WriteClient(lpECB->ConnID, "0\r\n\r\n", &five, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } + } + return HSE_STATUS_SUCCESS; } @@ -946,6 +1028,8 @@ if (isapi_sapi_module.startup) { isapi_sapi_module.startup(&sapi_module); } + ts_allocate_id(&tls_php_stream_info, sizeof(PHP_STREAM_INFO), + php_stream_info_ctor, php_stream_info_dtor); break; case DLL_THREAD_ATTACH: break; ------------------------------------------------------------------------ [2008-03-21 12:08:29] Richard dot Krehbiel at gmail dot com This DOES work with compression (zlib.output_compression=true). It alleviates the "little chunks" issue too, because the compression does some buffering. ------------------------------------------------------------------------ [2008-03-13 12:09:13] Richard dot Krehbiel at gmail dot com Bug: it doesn't properly serve HTTP 1.0 clients (wget), which don't support chunked transfers. This patch introduces a thread-local variable to indicate whether the transfer is chunked or not, and logic to detect an HTTP/1.0 transfer. --- /mnt/rich3/c/php-5.2.5/sapi/isapi/php5isapi.c 2007-02-23 17:08:30.000000000 -0500 +++ /mnt/rich3/c/buildphp/php-5.2.5/sapi/isapi/php5isapi.c 2008-03-12 14:28:34.000000000 -0400 @@ -143,6 +143,19 @@ NULL }; +typedef struct +{ + int chunked; +} PHP_STREAM_INFO; + +ts_rsrc_id tls_php_stream_info; + +static void php_stream_info_ctor(void *vsi, void ***foo) { + memset(vsi, 0, sizeof(PHP_STREAM_INFO)); +} + +static void php_stream_info_dtor(void *vsi, void ***foo) { +} static void php_info_isapi(ZEND_MODULE_INFO_FUNC_ARGS) { @@ -206,11 +219,36 @@ { DWORD num_bytes = str_length; LPEXTENSION_CONTROL_BLOCK ecb; - + // For chunked write + char chunksize[16]; + uint chunksizelen; + PHP_STREAM_INFO *phpinfo; + ecb = (LPEXTENSION_CONTROL_BLOCK) SG(server_context); - if (ecb->WriteClient(ecb->ConnID, (char *) str, &num_bytes, HSE_IO_SYNC) == FALSE) { - php_handle_aborted_connection(); + + phpinfo = (PHP_STREAM_INFO*)ts_resource(tls_php_stream_info); + + if(phpinfo->chunked) { + if(str_length > 0) { + uint two = 2; + _snprintf(chunksize, sizeof(chunksize), "%lX\r\n", str_length); + chunksizelen = strlen(chunksize); + if (ecb->WriteClient(ecb->ConnID, chunksize, &chunksizelen, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } + if (ecb->WriteClient(ecb->ConnID, (char *) str, &num_bytes, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } + if (ecb->WriteClient(ecb->ConnID, "\r\n", &two, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } + } + } else { + if (ecb->WriteClient(ecb->ConnID, (char *) str, &num_bytes, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } } + return num_bytes; } @@ -247,6 +285,7 @@ HSE_SEND_HEADER_EX_INFO header_info; sapi_header_struct default_content_type; char *status_buf = NULL; + PHP_STREAM_INFO *phpinfo = (PHP_STREAM_INFO*)ts_resource(tls_php_stream_info); /* Obtain headers length */ if (SG(sapi_headers).send_default_content_type) { @@ -256,16 +295,27 @@ zend_llist_apply_with_argument(&SG(sapi_headers).headers, (llist_apply_with_arg_func_t) accumulate_header_length, (void *) &total_length TSRMLS_CC); /* Generate headers */ - combined_headers = (char *) emalloc(total_length+1); + combined_headers = (char *) emalloc(total_length+64); combined_headers_ptr = combined_headers; if (SG(sapi_headers).send_default_content_type) { concat_header(&default_content_type, (void *) &combined_headers_ptr TSRMLS_CC); sapi_free_header(&default_content_type); /* we no longer need it */ } zend_llist_apply_with_argument(&SG(sapi_headers).headers, (llist_apply_with_arg_func_t) concat_header, (void *) &combined_headers_ptr TSRMLS_CC); - *combined_headers_ptr++ = '\r'; - *combined_headers_ptr++ = '\n'; - *combined_headers_ptr = 0; + + // HTTP/1.0 requestors don't get "chunked" replies + { + char protocol[64]; + DWORD protosize = sizeof(protocol); + + lpECB->GetServerVariable(lpECB->ConnID, "SERVER_PROTOCOL", protocol, &protosize); + phpinfo->chunked = (strcmp(protocol, "HTTP/1.0") != 0); + if(phpinfo->chunked) { + strcpy(combined_headers_ptr, "Transfer-Encoding: chunked\r\n\r\n"); + } else { + strcpy(combined_headers_ptr, "\r\n"); + } + } switch (SG(sapi_headers).http_response_code) { case 200: @@ -300,7 +350,7 @@ header_info.cchStatus = strlen(header_info.pszStatus); header_info.pszHeader = combined_headers; header_info.cchHeader = total_length; - header_info.fKeepConn = FALSE; + header_info.fKeepConn = phpinfo->chunked; lpECB->dwHttpStatusCode = SG(sapi_headers).http_response_code; lpECB->ServerSupportFunction(lpECB->ConnID, HSE_REQ_SEND_RESPONSE_HEADER_EX, &header_info, NULL, NULL); @@ -836,8 +886,11 @@ #ifdef PHP_ENABLE_SEH LPEXCEPTION_POINTERS e; #endif + PHP_STREAM_INFO *phpinfo; TSRMLS_FETCH(); + phpinfo = (PHP_STREAM_INFO*)ts_resource(tls_php_stream_info); + zend_first_try { #ifdef PHP_ENABLE_SEH __try { @@ -928,6 +981,15 @@ return HSE_STATUS_ERROR; } zend_end_try(); + // Finish a chunked transmission, send 0 length EOF chunk and trailing headers (none) + + if(phpinfo->chunked) { + uint five = 5; + if (lpECB->WriteClient(lpECB->ConnID, "0\r\n\r\n", &five, HSE_IO_SYNC) == FALSE) { + php_handle_aborted_connection(); + } + } + return HSE_STATUS_SUCCESS; } @@ -946,6 +1008,8 @@ if (isapi_sapi_module.startup) { isapi_sapi_module.startup(&sapi_module); } + ts_allocate_id(&tls_php_stream_info, sizeof(PHP_STREAM_INFO), + php_stream_info_ctor, php_stream_info_dtor); break; case DLL_THREAD_ATTACH: break; ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/44393 -- Edit this bug report at http://bugs.php.net/?id=44393&edit=1
