fetch extension - use local filename from
content-dispositionheader (new diff)
Gavin Atkinson
gavin.atkinson at ury.york.ac.uk
Mon Jan 9 09:30:50 PST 2006
On Mon, 2006-01-09 at 11:36 -0500, Martin Cracauer wrote:
> All right, I hope we all calmed down a little.
>
> To make people a little happier, I changed the meaning of the -O
> flag.
>
> Previously giving this non-argument -O flag would use the
> Content-Disposition header after a basic safey check.
>
> Now this flag takes an expected filename as an argument. If the
> argument is given, the server-supplied name will only be used if it
> matches the expected filename. If it doesn't the transport is aborted
> after reading the header.
>
> This will be useful when ports distfiles are distributed from servers
> using this mechanism, which is bound to happen (if it doesn't
> already). This way a port can say "download this URL", where the URL
> is some random php script as a filename, such as
> http://foo.com/download?fileid=3682, and download and save it only if the
> server gave the name of the expected distfile. So we would catch it
> if we make a mistake in the URL or if the server changes the mapping.
> Note that this will abort before actually downloading the file.
>
> For the rest of us who need this for attachments in Bugzilla and
> forums without knowing the expected filename you can give "." which
> means use the server-supplied name as is, after the previously
> mentioned safety checks.
I'm not sure I like the choice of ".", but can't think of a better
choice.
A couple of issues with the patch:
+ for (i = 0; s[i] != '"' && i < length - 1; i++) {
+ if (s[i] < ' ') {
+ name_altered++;
+ target[i] = '_';
+ } else {
Maybe better off using iscntrl(3) rather than "(s[i] < ' ')"?
+ return NULL;
These should be "return (NULL);" per style(9)
- if (us.size == -1)
- printf("Unknown\n");
- else
- printf("%jd\n", (intmax_t)us.size);
+ if (O_flag) {
+ if (us.content_disposition[0])
+ printf("%s\n", us.content_disposition);
+ else
+ printf("No_filename_supplied\n");
+ } else {
+ if (us.size == -1)
+ printf("Unknown_size\n");
+ else
+ printf("%jd\n", (intmax_t)us.size);
+ }
I don't like the underscores in this output. You're also changing the
output in the case of "-s" on it's own, which could break scripts (the
most likely user for the -s option).
There's also some overlap with the -Q option, I'm not sure it's needed
at all. Why not, in the -O case, always print the filename unless -q is
specified?
+ /* start the transfer */
You move this block of code to before the checks for "-r" and subsequent
stat(2) of the local file - I haven't tested it, but I suspect this will
break the -r option's ability to resume a partially downloaded file.
+ exit(34);
Why 34? The man page says fetch will return 1 on failure. I wonder if
this should be replaced with a "goto failure;" instead?
Other than those points, the patch seems good. I can see a definite
need for this functionality.
Gavin
More information about the freebsd-current
mailing list