Re: [Bug-wget] Conditional GET requests

[Tim Ruehsen]

On Tuesday 12 May 2015 22:05:25 Hubert Tarasiuk wrote:
> Hello Everyone,
> 
> I have prepared conditional GET requests support for Wget (specifiaclly
> If-Modified-Since header).
> 
> Following steps were taken (each is one patch):
> 1) local file timestamp support added to testenv
> 2) 304 return code added to testenv (ie. do not send response body when
> 304 was requested for given file)
> 3) Test-condget.py - unit test for conditional requests
> 4-5) "--condget" option handled in src/http.c
> 6) Short description in user manual
> 
> I would be very pleased if you could review my code and maybe point out
> some mistakes or any other comments/suggestions. Maybe you have ideas
> about some improvements to it?
> 
> In case you prefer to view it on Github, here is a link:
> https://github.com/jy987321/Wget/commits/master-hubert

Hi Hubert,

nice to see your work... it looks very good to me.

Just from a quick first glimpse, there are a few small points:

- please avoid abort() (found in time_to_rfc1123()). The function returns 
RETROK on success but calls abort() on failure. This might end up in a 
frustrating user experience. On error, you could fall back to not using if-
modified-since at all, fall back to using HEAD or fall back to use a time 
value of 0 (corresponding to 1.1.1970 00:00:00). Or whatever you think is 
appropriate.

- typo in Test-condget.py (usiong instead using) - have a look if your 
IDE/editor supports spell checking.

- it would be nice if your test case tests all variants of HTTP-date (= 
rfc1123-date | rfc850-date | asctime-date)

Regards, Tim

signature.asc
Description: This is a digitally signed message part.