[vlc-devel] [PATCH] added handling of wrong MIME boundary separator to mjpeg demux

[Rémi Denis-Courmont]

Le jeudi 30 août 2012 14:46:43 Sergey Radionov, vous avez écrit :
> 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#l
> 203 doing there?

You should not copy code in first place.

And yes, the old code is wrong. That's one more reason not to copy it.
Also, calling stream_ContentType() should only be called during probe phase, 
not during demux. And that's a third reason why you must not copy it.


(...)
> > 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)

Yes. So the code is wrong.

> > 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...

Why would I care about them?

-- 
Rémi Denis-Courmont
http://www.linkedin.com/in/remidenis