Re: [Bug-wget] Conditional GET requests

[Tim Rühsen]

Am Donnerstag, 14. Mai 2015, 15:35:29 schrieb Hubert Tarasiuk:
> W dniu 13.05.2015 o 10:24, Tim Ruehsen pisze:
> > 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.
> 
> We could do that but I am not sure that it would be good solution. Are
> there any cases when gmtime would have a good reason to fail?
> I think that when a function like gmtime fails, it could mean that
> something is seriously wrong; and perhaps we should not do anything but
> crash in that case (just as we do in case of xmalloc, for example).
> What do you think?

In case of malloc failing we hardly can recover or continue proper work.
In the case of gmtime failing, we could easily recover and continue our work 
(and inform the user, that something weird is going on). This makes Wget more 
robust and more reliable.

I wouldn't make assumptions on reasons that causes gmtime to fail. It might be 
anything, implementations will differ from OS to OS and from library to 
library. I saw lot's of interesting things in ~30 years of software 
development. I remember seeing time() returning -1 (sporadically, some bug in 
the library code, i guess).

People try to compile and run Wget on almost *any* system, even on 20 or 30 
year old systems.

> > - 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)
> 
> Do you mean to test the conversion Wget -> HTTP or HTTP -> Wget?
> In the former: Wget will currently issue dates exclusively in rfc1123
> format.
> In the latter: I agree that it should be tested (although the function
> that handles this conversion is not part of these patches, it had
> already been present in http.c (http_atotm)). Especially that the tests
> for "-N" do not test formats other than rfc1123, either (as far as I can
> see).

Right, Wget->Server is not interesting.

And your are right again, http_atotm() handles the different date formats.
AFAICS, there are no tests testing the different date formats. They belong 
into one of the *-N* tests and/or into your test. Best would be both, 
explicitly testing with HEAD and with Get+If-Modified-Since requests.
It would polish your test case, making it more complete.

Regards, Tim

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