[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [Patch] fix bug #44628 not honoring RFC 6266 in --content
From: |
Giuseppe Scrivano |
Subject: |
Re: [Bug-wget] [Patch] fix bug #44628 not honoring RFC 6266 in --content-disposition |
Date: |
Thu, 02 Apr 2015 15:01:18 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.4 (gnu/linux) |
Miquel Llobet <address@hidden> writes:
> From d71c7cc43689fc752dedb2a3500673f9981f7fc9 Mon Sep 17 00:00:00 2001
> From: Miquel Llobet <address@hidden>
> 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