[FFmpeg-devel] [PATCH] mpjpeg: CRLF terminating a sequence of MIME headers should not cause an error

[Alex Agranovsky]

This is as intended: stopping at the empty line is exactly what we should do in this case, as it separates headers from the content.
If the two headers we’re looking for hadn’t been encountered prior to the empty line, error should be returned. 

This is based on http://tools.ietf.org/html/rfc2046:

    body-part := MIME-part-headers [CRLF *OCTET]
                  ; Lines in a body-part must not start
                  ; with the specified dash-boundary and
                  ; the delimiter must not appear anywhere
                  ; in the body part.  Note that the
                  ; semantics of a body-part differ from
                  ; the semantics of a message, as
                  ; described in the text.


-- 
Alex Agranovsky
Sighthound, Inc
www.sighthound.com		

On September 12, 2015 at 8:14:01 PM, Michael Niedermayer (michaelni at gmx.at) wrote:

On Sat, Sep 12, 2015 at 07:04:26PM -0400, Alex Agranovsky wrote:  
> You’re correct, sir. New patch is attached.  
>  
>  
> --   
> Alex Agranovsky  
> Sighthound, Inc  
> www.sighthound.com		
>  
> On September 12, 2015 at 5:54:23 PM, Michael Niedermayer (michaelni at gmx.at) wrote:  
>  
> On Sat, Sep 12, 2015 at 05:05:04PM -0400, Alex Agranovsky wrote:  
> > Amended: one of the new blocks ended up in a wrong location.  
> >  
> >  libavformat/mpjpegdec.c | 19 +++++++++++++++----  
> >  1 file changed, 15 insertions(+), 4 deletions(-)  
> >  
> > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c  
> > index 8413ae7..1661b9d 100644  
> > --- a/libavformat/mpjpegdec.c  
> > +++ b/libavformat/mpjpegdec.c  
> > @@ -43,11 +43,20 @@ static int get_line(AVIOContext *pb, char *line, int line_size)  
> >  static int split_tag_value(char **tag, char **value, char *line)  
> >  {  
> >      char *p = line;  
> > +    int  foundData = 0;  
> >    
> > -    while (*p != '\0' && *p != ':')  
> > +    *tag = NULL;  
> > +    *value = NULL;  
> > +  
> > +  
> > +    while (*p != '\0' && *p != ':') {  
> > +        if (!av_isspace(*p)) {  
> > +            foundData = 1;  
> > +        }  
> >          p++;  
> > +    }  
> >      if (*p != ':')  
> > -        return AVERROR_INVALIDDATA;  
> > +        return foundData?AVERROR_INVALIDDATA:0;  
> >    
> >      *p   = '\0';  
> >      *tag = line;  
> > @@ -67,7 +76,7 @@ static int check_content_type(char *line)  
> >      char *tag, *value;  
> >      int ret = split_tag_value(&tag, &value, line);  
> >    
> > -    if (ret < 0)  
> > +    if (ret < 0 || tag == NULL || value == NULL)  
> >          return ret;  
>  
> doesnt this result in a probe score of AVPROBE_SCORE_MAX for an  
> empty line, or am i misreading the code ?  
>  
>  
> [...]  
> --  
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB  
>  
> Good people do not need laws to tell them to act responsibly, while bad  
> people will find a way around the laws. -- Plato  
> _______________________________________________  
> ffmpeg-devel mailing list  
> ffmpeg-devel at ffmpeg.org  
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel  

> mpjpegdec.c | 19 ++++++++++++++++---  
> 1 file changed, 16 insertions(+), 3 deletions(-)  
> 7f04b810839ce200528855c2c9b50aaf45028045 patch2b.diff  
> diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c  
> index e4d76f3..8c91bc4 100644  
> --- a/libavformat/mpjpegdec.c  
> +++ b/libavformat/mpjpegdec.c  
> @@ -43,11 +43,20 @@ static int get_line(AVIOContext *pb, char *line, int line_size)  
> static int split_tag_value(char **tag, char **value, char *line)  
> {  
> char *p = line;  
> + int foundData = 0;  
>  
> - while (*p != '\0' && *p != ':')  
> + *tag = NULL;  
> + *value = NULL;  
> +  
> +  
> + while (*p != '\0' && *p != ':') {  
> + if (!av_isspace(*p)) {  
> + foundData = 1;  
> + }  
> p++;  
> + }  
> if (*p != ':')  
> - return AVERROR_INVALIDDATA;  
> + return foundData?AVERROR_INVALIDDATA:0;  
>  
> *p = '\0';  
> *tag = line;  
> @@ -70,7 +79,9 @@ static int check_content_type(char *line)  
> if (ret < 0)  
> return ret;  
>  
> - if (av_strcasecmp(tag, "Content-type") ||  
> + if ( tag == NULL ||  
> + value == NULL ||  
> + av_strcasecmp(tag, "Content-type") ||  
> av_strcasecmp(value, "image/jpeg"))  
> return AVERROR_INVALIDDATA;  
>  

> @@ -176,6 +187,8 @@ static int parse_multipart_header(AVFormatContext *s)  
> ret = split_tag_value(&tag, &value, line);  
> if (ret < 0)  
> return ret;  
> + if (value==NULL || tag==NULL)  
> + break;  

this would stop parsing on an empty line,  
could there be further lines afterwards that need parsing ?  

[...]  

--  
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB  

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope  
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel at ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel