[vlc-devel] [PATCH] added handling of wrong MIME boundary separator to mjpeg demux
Sergey Radionov
rsatom at gmail.com
Thu Aug 30 13:46:43 CEST 2012
2012/8/30 Rémi Denis-Courmont <remi at remlab.net>:
> Le jeudi 30 août 2012 06:18:44 Sergey Radionov, vous avez écrit :
>> ---
>> modules/demux/mjpeg.c | 94
>> ++++++++++++++++++++++++++++++++++++++++++++++--- 1
> files changed, 89
>> insertions(+), 5 deletions(-)
>>
>> diff --git a/modules/demux/mjpeg.c b/modules/demux/mjpeg.c
>> index a057db3..f5bd1dd 100644
>> --- a/modules/demux/mjpeg.c
>> +++ b/modules/demux/mjpeg.c
>> @@ -200,10 +200,10 @@ static bool CheckMimeHeader( demux_t *p_demux, int
>> *p_header_size ) char *content_type = stream_ContentType( p_demux->s ); if
>> ( content_type == NULL )
>> return false;
>> - const char* boundary = strstr( content_type, "boundary=--" );
>> + const char* boundary = strstr( content_type, "boundary=" );
>> if ( boundary != NULL )
>> {
>> - p_sys->psz_separator = strdup( boundary + strlen(
>> "boundary=--" ) ); + p_sys->psz_separator = strdup(
>> boundary + strlen("boundary=") ); msg_Dbg( p_demux, "Video boundary
>> extracted from Content-Type: %s", p_sys->psz_separator ); free(
>> content_type );
>> /* Skip to HTTP header parsing as there's no boundary to
>> extract @@ -223,7 +223,8 @@ static bool CheckMimeHeader( demux_t *p_demux,
>> int *p_header_size ) }
>> else
>> {
>> - i_pos = *p_sys->p_peek == '-' ? 2 : 4;
>> + i_pos = *p_sys->p_peek == '-' ? 0 : 2;//skip "\r\n" if exist
>> + int i_pos_save = i_pos;//to allow restore after missing "\r\n"...
>> psz_line = GetLine( p_demux, &i_pos );
>> if( NULL == psz_line )
>> {
>> @@ -235,13 +236,92 @@ static bool CheckMimeHeader( demux_t *p_demux, int
>> *p_header_size ) /* Read the separator and remember it if not yet stored
>> */ if( p_sys->psz_separator == NULL )
>> {
>> - p_sys->psz_separator = psz_line;
>> + char *content_type = stream_ContentType( p_demux->s );
>> + char* boundary = NULL;
>> + if ( content_type != NULL )
>> + {
>> + boundary = strstr( content_type, "boundary=" );
>> + if( boundary ) boundary += strlen("boundary=");
>> + }
>
> That's not a correct way to parse the content type per specification.
ok, then what this
http://git.videolan.org/?p=vlc.git;a=blob;f=modules/demux/mjpeg.c;hb=HEAD#l203
doing there?
>
>> +
>> + //some vendors add "--" at boundary begin and end in
>> ContentType declaration, + //and don't add additional "--" in
>> the body - it's incorrect... + //some other vendors, forget to
>> add "\r\n" after boundary - it's incorrect too... + //and some
>> else vendors, forget to add "\r\n" and forget to add "--" both +
>> //(or add excess "--" to ContentType, which is the same)... +
>> + size_t boundary_len = boundary ? strlen(boundary) : 0;
>> + //if used boundary without additional "--"
>> + //(i.e. used same string was declared in ContentType), then
>> + if( boundary && !strncmp( psz_line, boundary, boundary_len ) )
>> + {
>> + //if boundary identical to psz_line, then
>> + if( *(psz_line + boundary_len) == '\0' )
>> + {
>> + //just use whole line as separator
>> + p_sys->psz_separator = psz_line;
>> + }
>> + else
>> + {
>> + //restore buffer position to right place
>> + i_pos = i_pos_save + boundary_len;
>> + p_sys->psz_separator = strdup( boundary );
>> + free( psz_line );
>> + psz_line = NULL;
>> +
>> + if( !p_sys->psz_separator )
>> + {
>> + free( content_type );
>> + *p_header_size = -4;
>> + return false;
>> + }
>> + }
>> + }
>> +
>> + //if used boundary with additional "--"
>> + if( !p_sys->psz_separator && boundary &&
>> + !strncmp( psz_line + strlen("--"), boundary, boundary_len
>> ) ) + {
>> + char* s = psz_line + strlen("--") + boundary_len;
>> + if( !strncmp( s , "--", 2 ) ) //skip "--" if boundary is
>> last + s += 2;
>> +
>> + //"\r\n" is forgotten
>> + if ( *s != '\0' )
>> + {
>> + //restore buffer position to right place
>> + i_pos = i_pos_save + boundary_len;
>> + }
>> +
>> + p_sys->psz_separator = strdup( boundary );
>> + free( psz_line );
>> + psz_line = NULL;
>> +
>> + if( !p_sys->psz_separator )
>> + {
>> + free( content_type );
>> + *p_header_size = -4;
>> + return false;
>> + }
>> + }
>> +
>> + if( !p_sys->psz_separator )
>> + {
>> + //separator was not found, then just use whole line
>> + //FIXME: don't sure it's right
>> + p_sys->psz_separator = psz_line;
>> + }
>> +
>
> This entire block looks completely insane. I can't understand it but it
> definitely does not look like what the specification says.
I know, we are talking about _broken_ sources.
>
>> msg_Dbg( p_demux, "Multipart MIME detected, using separator:
>> %s", p_sys->psz_separator );
>> +
>> + free( content_type );
>> }
>> else
>> {
>> - if( strcmp( psz_line, p_sys->psz_separator ) )
>> + size_t separator_len = strlen(p_sys->psz_separator);
>> + if( strncmp( psz_line + strlen("--"), p_sys->psz_separator,
>> separator_len ) && + strncmp( psz_line,
>> p_sys->psz_separator, separator_len ) ) {
>> msg_Warn( p_demux, "separator %s does not match %s",
>> psz_line, p_sys->psz_separator );
>> @@ -527,7 +607,11 @@ static int MimeDemux( demux_t *p_demux )
>> }
>> }
>>
>> + //some vendors use as separator same string which was defined in
>> ContentType + //without additional "--"
>> if( !strncmp( p_sys->psz_separator, (char *)(p_sys->p_peek + i +
>> 2), + strlen( p_sys->psz_separator ) ) ||
>> + !strncmp( p_sys->psz_separator, (char *)(p_sys->p_peek + i),
>> strlen( p_sys->psz_separator ) ) )
>> {
>> break;
>
> No way. Absolutely no way. The two leading dashes are *_MANDATORY_*.
>
in that case p_sys->psz_separator = "--video separator--", i.e. it
already have "--"
(but I know, that in this case it should be, "----video separator--"
or "----video separator----", but it not)
> Skipping them is not only against the specification, it will cause too many
> false positives in matching the file format.
>
> Fix the broken software instead.
ok, say it to guys in TRENDnet...
>
> --
> Rémi Denis-Courmont
> http://www.linkedin.com/in/remidenis
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list