Using UpnpReadHttpGet to download large files causes the application to

crash. This happens when the file being downloaded exceeds the device
memory - entirely possible when transferring video files.
The programmatic cause is that the logic implemented in the function
http_ReadHttpGet (which UpnpReadHttpGet calls) reads the entire file
into memory. The fix modifies the existing logic to discard data after
it's been read; there's no reason to keep it around since the caller
of UpnpReadHttpGet already has a copy of it.

This issue exists in 1.6.6 as well as the latest sources.

Patch submitted by Chandra (inactiveneurons).
This commit is contained in:
Marcelo Roberto Jimenez 2010-09-07 22:15:21 -03:00
parent 21660334e4
commit 4657e57766
4 changed files with 49 additions and 32 deletions

View File

@ -2,6 +2,20 @@
Version 1.6.7 Version 1.6.7
******************************************************************************* *******************************************************************************
2010-09-07 Marcelo Jimenez <mroberto(at)users.sourceforge.net>
Using UpnpReadHttpGet to download large files causes the application to
crash. This happens when the file being downloaded exceeds the device
memory - entirely possible when transferring video files.
The programmatic cause is that the logic implemented in the function
http_ReadHttpGet (which UpnpReadHttpGet calls) reads the entire file
into memory. The fix modifies the existing logic to discard data after
it's been read; there's no reason to keep it around since the caller
of UpnpReadHttpGet already has a copy of it.
This issue exists in 1.6.6 as well as the latest sources.
Patch submitted by Chandra (inactiveneurons).
2010-09-07 Marcelo Jimenez <mroberto(at)users.sourceforge.net> 2010-09-07 Marcelo Jimenez <mroberto(at)users.sourceforge.net>
In the latest sources, http_RequestAndResponse and other methods that In the latest sources, http_RequestAndResponse and other methods that
use connect() are broken. More specifically, connect() in these methods use connect() are broken. More specifically, connect() in these methods

View File

@ -1909,9 +1909,9 @@ parser_parse_entity_using_clen( INOUT http_parser_t * parser )
assert( parser->ent_position == ENTREAD_USING_CLEN ); assert( parser->ent_position == ENTREAD_USING_CLEN );
// determine entity (i.e. body) length so far // determine entity (i.e. body) length so far
//entity_length = parser->msg.msg.length - parser->entity_start_position;
parser->msg.entity.length = parser->msg.entity.length =
parser->msg.msg.length - parser->entity_start_position; parser->msg.msg.length - parser->entity_start_position +
parser->msg.entity_offset;
if( parser->msg.entity.length < parser->content_length ) { if( parser->msg.entity.length < parser->content_length ) {
// more data to be read // more data to be read
@ -1919,7 +1919,8 @@ parser_parse_entity_using_clen( INOUT http_parser_t * parser )
} else { } else {
if( parser->msg.entity.length > parser->content_length ) { if( parser->msg.entity.length > parser->content_length ) {
// silently discard extra data // silently discard extra data
parser->msg.msg.buf[parser->entity_start_position + parser->msg.msg.buf[parser->entity_start_position -
parser->msg.entity_offset +
parser->content_length] = '\0'; parser->content_length] = '\0';
} }
// save entity length // save entity length
@ -2304,6 +2305,7 @@ parser_response_init( OUT http_parser_t * parser,
parser_init( parser ); parser_init( parser );
parser->msg.is_request = FALSE; parser->msg.is_request = FALSE;
parser->msg.request_method = request_method; parser->msg.request_method = request_method;
parser->msg.entity_offset = 0;
parser->position = POS_RESPONSE_LINE; parser->position = POS_RESPONSE_LINE;
} }

View File

@ -256,7 +256,7 @@ int http_RecvMessage(
"<<< (RECVD) <<<\n%s\n-----------------\n", "<<< (RECVD) <<<\n%s\n-----------------\n",
parser->msg.msg.buf ); parser->msg.msg.buf );
print_http_headers( &parser->msg ); print_http_headers( &parser->msg );
if (parser->content_length > (unsigned int)g_maxContentLength) { if (g_maxContentLength > 0 && parser->content_length > (unsigned int)g_maxContentLength) {
*http_error_code = HTTP_REQ_ENTITY_TOO_LARGE; *http_error_code = HTTP_REQ_ENTITY_TOO_LARGE;
line = __LINE__; line = __LINE__;
ret = UPNP_E_OUTOF_BOUNDS; ret = UPNP_E_OUTOF_BOUNDS;
@ -1070,10 +1070,10 @@ errorHandler:
} }
typedef struct HTTPGETHANDLE { typedef struct HTTPGETHANDLE {
http_parser_t response; http_parser_t response;
SOCKINFO sock_info; SOCKINFO sock_info;
int entity_offset; int entity_offset;
int cancel; int cancel;
} http_get_handle_t; } http_get_handle_t;
/************************************************************************ /************************************************************************
@ -1327,9 +1327,10 @@ http_ReadHttpGet( IN void *Handle,
int ret_code = 0; int ret_code = 0;
if( ( !handle ) || ( !size ) || ( ( ( *size ) > 0 ) && !buf ) if( !handle || !size || (*size > 0 && !buf) || *size < 0) {
|| ( ( *size ) < 0 ) ) { if(size) {
if(size) ( *size ) = 0; *size = 0;
}
return UPNP_E_INVALID_PARAM; return UPNP_E_INVALID_PARAM;
} }
//first parse what has already been gotten //first parse what has already been gotten
@ -1346,11 +1347,11 @@ http_ReadHttpGet( IN void *Handle,
&& ( status != PARSE_CONTINUE_1 ) && ( status != PARSE_CONTINUE_1 )
&& ( status != PARSE_INCOMPLETE ) ) { && ( status != PARSE_INCOMPLETE ) ) {
//error //error
( *size ) = 0; *size = 0;
return UPNP_E_BAD_RESPONSE; return UPNP_E_BAD_RESPONSE;
} }
//read more if necessary entity //read more if necessary entity
while( ( ( handle->entity_offset + ( *size ) ) > while( ( ( handle->response.msg.entity_offset + *size ) >
handle->response.msg.entity.length ) handle->response.msg.entity.length )
&& ( ! handle->cancel ) && ( ! handle->cancel )
&& ( handle->response.position != POS_COMPLETE ) ) { && ( handle->response.position != POS_COMPLETE ) ) {
@ -1365,7 +1366,7 @@ http_ReadHttpGet( IN void *Handle,
// set failure status // set failure status
handle->response.http_error_code = handle->response.http_error_code =
HTTP_INTERNAL_SERVER_ERROR; HTTP_INTERNAL_SERVER_ERROR;
( *size ) = 0; *size = 0;
return PARSE_FAILURE; return PARSE_FAILURE;
} }
status = parser_parse_entity( &handle->response ); status = parser_parse_entity( &handle->response );
@ -1376,7 +1377,7 @@ http_ReadHttpGet( IN void *Handle,
&& ( status != PARSE_CONTINUE_1 ) && ( status != PARSE_CONTINUE_1 )
&& ( status != PARSE_INCOMPLETE ) ) { && ( status != PARSE_INCOMPLETE ) ) {
//error //error
( *size ) = 0; *size = 0;
return UPNP_E_BAD_RESPONSE; return UPNP_E_BAD_RESPONSE;
} }
} else if( num_read == 0 ) { } else if( num_read == 0 ) {
@ -1387,31 +1388,33 @@ http_ReadHttpGet( IN void *Handle,
handle->response.position = POS_COMPLETE; handle->response.position = POS_COMPLETE;
} else { } else {
// partial msg // partial msg
( *size ) = 0; *size = 0;
handle->response.http_error_code = HTTP_BAD_REQUEST; // or response handle->response.http_error_code = HTTP_BAD_REQUEST; // or response
return UPNP_E_BAD_HTTPMSG; return UPNP_E_BAD_HTTPMSG;
} }
} else { } else {
( *size ) = 0; *size = 0;
return num_read; return num_read;
} }
} }
if( ( handle->entity_offset + ( *size ) ) > if ((handle->response.msg.entity_offset + *size) > handle->response.msg.entity.length) {
handle->response.msg.entity.length ) { *size = handle->response.msg.entity.length - handle->response.msg.entity_offset;
( *size ) =
handle->response.msg.entity.length - handle->entity_offset;
} }
memcpy( buf, memcpy(buf,
&handle->response.msg.msg.buf[handle-> &handle->response.msg.msg.buf[handle->response.entity_start_position],
response.entity_start_position + *size);
handle->entity_offset], if (*size > 0) {
( *size ) ); membuffer_delete(&handle->response.msg.msg,
handle->entity_offset += ( *size ); handle->response.entity_start_position,
*size);
}
if ( handle->cancel ) handle->response.msg.entity_offset += *size;
if (handle->cancel) {
return UPNP_E_CANCELED; return UPNP_E_CANCELED;
}
return UPNP_E_SUCCESS; return UPNP_E_SUCCESS;
} }
@ -1498,7 +1501,6 @@ http_CloseHttpGet( IN void *Handle )
sock_destroy( &handle->sock_info, SD_BOTH ); //should shutdown completely sock_destroy( &handle->sock_info, SD_BOTH ); //should shutdown completely
httpmsg_destroy( &handle->response.msg ); httpmsg_destroy( &handle->response.msg );
handle->entity_offset = 0;
free( handle ); free( handle );
return UPNP_E_SUCCESS; return UPNP_E_SUCCESS;
} }
@ -1609,7 +1611,6 @@ int http_OpenHttpGetProxy(
if (!handle) { if (!handle) {
return UPNP_E_OUTOF_MEMORY; return UPNP_E_OUTOF_MEMORY;
} }
handle->entity_offset = 0;
handle->cancel = 0; handle->cancel = 0;
parser_response_init(&handle->response, HTTPMETHOD_GET); parser_response_init(&handle->response, HTTPMETHOD_GET);
tcp_connection = socket(peer->hostport.IPaddress.ss_family, SOCK_STREAM, 0); tcp_connection = socket(peer->hostport.IPaddress.ss_family, SOCK_STREAM, 0);
@ -2240,7 +2241,6 @@ int http_OpenHttpGetEx(
break; break;
} }
memset(handle, 0, sizeof(*handle)); memset(handle, 0, sizeof(*handle));
handle->entity_offset = 0;
parser_response_init(&handle->response, HTTPMETHOD_GET); parser_response_init(&handle->response, HTTPMETHOD_GET);
tcp_connection = socket(url.hostport.IPaddress.ss_family, SOCK_STREAM, 0); tcp_connection = socket(url.hostport.IPaddress.ss_family, SOCK_STREAM, 0);
if (tcp_connection == -1) { if (tcp_connection == -1) {

View File

@ -193,7 +193,8 @@ typedef struct // http_message_t
// private fields // private fields
membuffer msg; // entire raw message membuffer msg; // entire raw message
char *urlbuf; // storage for url string char *urlbuf; // storage for url string
size_t entity_offset;
} http_message_t; } http_message_t;
typedef struct // http_parser_t typedef struct // http_parser_t