Miquel Llobet <mllobet...@gmail.com> writes: > From d71c7cc43689fc752dedb2a3500673f9981f7fc9 Mon Sep 17 00:00:00 2001 > From: Miquel Llobet <mllobet...@gmail.com> > Date: Wed, 1 Apr 2015 17:32:50 +0200 > Subject: [PATCH] Fixed #44628 honoring RFC 6266 content-disposition > > src/http.c (parse_content_disposition): stores filename* and filename > separately and choses filename* if available > src/http.c (test_parse_content_disposition): added new tests for the > reported bu
this line seems incomplete. > --- > src/http.c | 27 ++++++++++++++++++++++----- > 1 file changed, 22 insertions(+), 5 deletions(-) > > diff --git a/src/http.c b/src/http.c > index 9994d13..1f98e5e 100644 > --- a/src/http.c > +++ b/src/http.c > @@ -1202,6 +1202,7 @@ parse_content_disposition (const char *hdr, char > **filename) > bool is_url_encoded = false; > > *filename = NULL; > + char *encodedFilename = NULL; declaration after statement, please move it one line up. > for ( ; extract_param (&hdr, &name, &value, ';', &is_url_encoded); > is_url_encoded = false) > { > @@ -1218,17 +1219,27 @@ parse_content_disposition (const char *hdr, char > **filename) > if (value.b == value.e) > continue; > > - if (*filename) > - append_value_to_filename (filename, &value, is_url_encoded); > + /* Check if the name is "filename*" as specified in RFC 6266. trailing whitespace here. I find useful to have these lines [color] branch = auto diff = auto interactive = auto status = auto in my ~/.gitconfig file. > + * Since "filename" could be broken up as "filename*N" (RFC 2231), > + * a check is needed to make sure this is not the case */ > + int isEncodedFilename = *name.e == '*'&& !c_isdigit(*(name.e + 1)); Please use a "bool" instead of "int". Space before && and after the function name: c_isdigit (*(name.e + 1)); > + char **outFilename = isEncodedFilename ? &encodedFilename : > filename; both isEncodedFilename and outFilename must be declared immediately after the '{' before any code statement. > + if (*outFilename) > + append_value_to_filename (outFilename, &value, is_url_encoded); > else > { > - *filename = strdupdelim (value.b, value.e); > + *outFilename = strdupdelim (value.b, value.e); > if (is_url_encoded) > - url_unescape (*filename); > + url_unescape (*outFilename); > } > } > } > > + if (encodedFilename) > + { > + xfree (*filename); > + *filename = encodedFilename; > + } using filename as a temporary in case !encodedFilename is fine, but maybe we can make it clearer and use two different local temporary values (encodedFilename and unencodedFilename) and set *filename according to which one is available? What do you think? if (encodedFilename) { xfree (unencodedfilename); *filename = encodedFilename; } else { xfree (encodedfilename); *filename = unencodedFilename; } Regards, Giuseppe