Re: [Bug-wget] Conditional GET requests

[Giuseppe Scrivano]

few minor comments:

Hubert Tarasiuk <address@hidden> writes:

> From af5f411a699484482bc951d6b4efb6a90575f653 Mon Sep 17 00:00:00 2001
> From: Hubert Tarasiuk <address@hidden>
> Date: Sat, 9 May 2015 22:47:24 +0200
> Subject: [PATCH 5/6] Prototype of If-Modified-Since.
>
> @@ -1723,6 +1776,21 @@ initialize_request (struct url *u, struct http_stat 
> *hs, int *dt, struct url *pr
>        /* ... but some HTTP/1.0 caches doesn't implement Cache-Control.  */
>        request_set_header (req, "Pragma", "no-cache", rel_none);
>      }
> +  if (*dt & IF_MODIF_SINCE)
> +    {
> +      char strtime[32];
> +      uerr_t err;
> +
> +      err = time_to_rfc1123 (hs->orig_file_tstamp, strtime, countof 
> (strtime));
> +      if (err != RETROK)
> +        {
> +          logputs (LOG_VERBOSE, _("Cannot convert timestamp to http format. "
> +                                  "Falling back to time 0 as last 
> modification "
> +                                  "time.\n"));
> +          strcpy (strtime, "Thu, 01 Jan 1970 00:00:00 GMT");
> +        }
> +      request_set_header (req, "If-Modified-Since", xstrdup(strtime), 
> rel_value);

space after xstrup.

> @@ -2421,6 +2500,9 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, 
> struct url *proxy,
>       POST). */
>    bool head_only = !!(*dt & HEAD_ONLY);
>  
> +  /* Whether conditional get request will be issued */
> +  bool cond_get = !!(*dt & IF_MODIF_SINCE);
> +
>    char *head = NULL;
>    struct response *resp = NULL;
>    char hdrval[512];
> @@ -3020,6 +3102,35 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, 
> struct url *proxy,
>          }
>      }
>  
> +  if (cond_get && statcode == HTTP_STATUS_NOT_MODIFIED)
> +    {
> +      logprintf (LOG_VERBOSE, _("File %s not modified on server. "
> +                                "Omiting download.\n\n"), 
> quote(hs->local_file));

s|Omiting|Omitting|

Please leave a space after "quote".

> +  if (cond_get && statcode == HTTP_STATUS_OK && hs->remote_time)
> +    {
> +      time_t tmr;
> +      tmr = http_atotm (hs->remote_time);
> +      if (tmr != (time_t) -1 && tmr <= hs->orig_file_tstamp
> +          && (contlen == -1 || contlen == hs->orig_file_size))
> +        {
> +          logprintf (LOG_VERBOSE, _("Server ignored If-Modified-Since header 
> "
> +                                    "for file %s. "
> +                                    "You might want to add 
> --no-if-modified-since "
> +                                    "option.\n\n"), quote(hs->local_file));

same here.  Maybe we want to split this long message on two separate lines?

I tested it and I think we can think about merging these patches (also
please change the first line of the 5/6 commit message :))

Regards,
Giuseppe