thanks for your contribution, some comments inline:
Darshit Shah dar...@gmail.com writes:
- if (statcode == HTTP_STATUS_TEMPORARY_REDIRECT)
-return NEWLOCATION_KEEP_POST;
+ switch (statcode)
+ {
+case HTTP_STATUS_TEMPORARY_REDIRECT:
+
Thanks, the code seems correct but since you are going to write a lot of
code during your Summer of Code project, I will be very pedantic. This
time I have just amended the changes before push your patch.
Darshit Shah dar...@gmail.com writes:
Please be aware that Wget needs to know the size
Attaching a patch that should (hopefully) fix all the problems listed here.
I have tested it and it looks good to me.
On Thu, May 9, 2013 at 9:55 AM, Darshit Shah dar...@gmail.com wrote:
We currently suspend any method if the response code is a non 307
redirect. That I believe is wrong. We
Darshit Shah dar...@gmail.com writes:
We suspend the post data only when we receive a 307, or do you
mean we
shouldn't suspend in this case too?
It's the other way round. A 307 response code is used when the server
wishes to explicitly ask the client to not suspend.
We currently suspend any method if the response code is a non 307
redirect. That I believe is wrong. We should only be suspending the
POST method and not others.
It sounds good, but we must be careful also with 303.
Do you agree that the matrix for the method to use on the next request
Darshit Shah dar...@gmail.com writes:
While DELETE *may* be redirected, the document did not mention
anything explicitly about it. From what I gauge, we should not suspend
a DELETE command upon a redirect.
We suspend the post data only when we receive a 307, or do you mean we
shouldn't
We suspend the post data only when we receive a 307, or do you mean we
shouldn't suspend in this case too?
It's the other way round. A 307 response code is used when the server
wishes to explicitly ask the client to not suspend. And that is also the
current behaviour.
We currently suspend
Darshit Shah dar...@gmail.com writes:
I also think some more explicit sanity checks are needed in the code.
Like sending data with a HEAD request makes absolutely no sense (does
it?).
no, it doesn't, same for GET.
--
Giuseppe
while I agree that your patch makes the code more readable and it is
good to apply it, I don't see how the previous version was broken. What
compiler have you used?
I see you pushed the patch. But have we gotten to the bottom of the cause?
If the result changes with the GCC version, we
Darshit Shah dar...@gmail.com writes:
while I agree that your patch makes the code more readable and it is
good to apply it, I don't see how the previous version was broken.
What
compiler have you used?
I see you pushed the patch. But have we gotten to the bottom of
Hi Giuseppe,
The patch supplied by you adding cmd_uppercase_String seems to break the
header generation since (*q++) causes the pointer to increment before the
c_toupper() method returns.
We should instead increment it only after the statement successfully
returns.
I have also converted the loop
Darshit Shah dar...@gmail.com writes:
I am fixing this issue, but it is a terribly ugly hack. If someone could
help improve it I'd be most truly grateful.
I have a couple of ideas, but I will need to work them out and implement
them when I have the time.
The reason it has to be so ugly is
Am Mittwoch, 1. Mai 2013 schrieb Darshit Shah:
First, sorry for the quick and dirty hack which was the perfect example of
how NOT to do things.
Than it was a good example ;-)
Secondly, it lies upon me that this feature wasn't tested before submitting
the patch. I had however relied on the
hi Gijs,
Gijs van Tulder gvtul...@gmail.com writes:
Giuseppe Scrivano wrote:
what about this patch? Any comment?
Another suggestion: why not save the original opt.method, set
opt.method to NULL and put the original opt.method back later?
thanks for your suggestion but I think we should
On Thu, May 2, 2013 at 12:56 AM, Gijs van Tulder gvtul...@gmail.com wrote:
Giuseppe Scrivano wrote:
what about this patch? Any comment?
Another suggestion: why not save the original opt.method, set opt.method
to NULL and put the original opt.method back later?
Gijs
Unless we explicitly
Hi Giuseppe,
Dropping the bit that sanitizes the opt.method is probably a good idea.
(Perhaps I shouldn't have replied to your patch directly.)
Still, even if the sanitization is removed: I think it would be better
if RESTORE_POST_DATA restores the previous value of opt.method, instead
of
Still, even if the sanitization is removed: I think it would be better if
RESTORE_POST_DATA restores the previous value of opt.method, instead of
overwriting it with a hardcoded POST. Isn't it?
As double safety yes. Maybe we should do that.
A related question: how is a redirect response
On Thu, 2 May 2013, Giuseppe Scrivano wrote:
RFC 2606 doesn't seem very clear about it, and I can't find anywhere that
PUT/OPTIONS/ANYTHING should be handled differently than POST wrt
redirections. I don't see why suspending a PUT request would be incorrect.
Darshit, do you have any pointer?
Here is the reproducer:
$ wget --passive-ftp -nv --recursive --level=1 --no-directories --no-parent \
--no-check-certificate -A '*.po'
http://translationproject.org/latest/make
2013-04-30 16:41:25 URL:https://translationproject.org/latest/make/
[5489/5489] - make [1]
Segmentation
On 04/30/2013 07:01 PM, Darshit Shah wrote:
Okay, I could not prevent myself from looking into it.
It seems as if the SUSPEND_POST_DATA macro was being called during a
recursive download attempt.
Attaching a hack around the situation. Will look more deeply when I have
more time to
Hi Darshit,
I understand that your patch was just a quick hack.
But even than you should avoid doing
opt.method == POST
for string comparisons.
This definitely not portable.
Not every compiler/linker aggregates two occurrences of the same static string
into one address in memory.
You
21 matches
Mail list logo
