As expected, explicitly setting opt.method = "POST" results in a Segmentation Fault if the server sends a redirect response. This will occur because, when processing the new url, both opt.method and opt.post-data / opt.post_file_name are set. The only sane way of making post-data / post-file ride on the code for body-data / body-file is to set the options in main.c through setoptval().
However, much as I try, I haven't been able to understand the section of the code that handles setting these options. This is what I tried, if someone can help me out here, I'd be very thankful! Since, two options need to be set for one command line parameter, I thought about adding a non-standard option, "OPT__ALIAS" in struct cmdline_options Change --post-file and --post-data in option_data to: { "post-data", 0, OPT__ALIAS, NULL, required_argument }, { "post-file", 0, OPT__ALIAS, "postfile", required_argument }, However, after this, I am unsure as to what parameters to pass to setoptval(). All the calls to setoptval() have opt->long_name as their third argument, but that isn't valid in this case, since opt->long_name will refer to the original --post-* command. On Sun, Apr 7, 2013 at 2:02 AM, Darshit Shah <dar...@gmail.com> wrote: > > > + if (opt.method) >> > + { >> > + request_set_header (req, "Content-Type", >> > + "application/x-www-form-urlencoded", >> rel_none); >> > + >> > + if (opt.body_data || opt.body_file) >> > + { >> >> please indent '{' with two empty spaces. >> > > Sorry, still not completely used to GNU's indentation method. I sometimes > slip up and forget that indent. > Currently mobile, I'll make the change as soon as I get access to a > machine. > > >> > + if (opt.body_data) >> > + body_data_size = strlen (opt.body_data); >> > + else >> > + { >> > + body_data_size = file_size (opt.body_file); >> > + if (body_data_size == -1) >> > + { >> > + logprintf (LOG_NOTQUIET, _("BODY data file %s missing: >> %s\n"), >> > + quote (opt.body_file), strerror (errno)); >> > + return FILEBADFILE; >> > + } >> > + } >> > + request_set_header (req, "Content-Length", >> > + xstrdup (number_to_static_string >> (body_data_size)), >> > + rel_value); >> > + } >> > + } >> > + >> >> maybe the previous "if (opt.post_data || opt.post_file_name)" case can >> be handled here? >> >> What will break if we just do: >> >> if (opt.post_data || opt.post_file_name) >> { >> opt.method = "POST"; >> opt.body_data = opt.post_data; >> opt.body_file = opt.post_file; >> } >> >> before you check for if (opt.method)? >> >> Do you think we can get ride of the old code to handle POST data? >> > > That would be possible, if we set opt.post_data and opt.post_file_name as > aliases to set opt.method="POST" and opt.body_data or opt.body_file in > main.c > > Otherwise, this will cause some inconsistent output, wherein in one place > it says Post-Data and in another wget outputs Body-Data. > That can however be avoided, if the line reads: > logprintf (LOG_NOTQUIET, _("%s data file %s missing: %s\n"), > quote (opt.method), quote (opt.body_file), > strerror (errno)); > > However, since the code is explicitly checking for opt.method and > opt.post_data / opt.post_file_name in multiple places, this has a very high > chance of breaking if the server redirects. > I'll run some tests after the change and let you know. > > >> In any case I can't still apply your patch upstream, I am waiting for >> your copyright assignment papers to be processed (or received). >> > I have already sent the papers by snail mail. Wish there was a way to > submit them electronically. > > > > -- > Thanking You, > Darshit Shah > Research Lead, Code Innovation > Kill Code Phobia. > B.E.(Hons.) Mechanical Engineering, '14. BITS-Pilani > -- Thanking You, Darshit Shah Research Lead, Code Innovation Kill Code Phobia. B.E.(Hons.) Mechanical Engineering, '14. BITS-Pilani