From mboxrd@z Thu Jan  1 00:00:00 1970
From: damiannohales at gmail.com (=?UTF-8?Q?Dami=C3=A1n_Nohales?=)
Date: Mon, 11 Aug 2014 19:45:53 -0300
Subject: [PATCH] Handle If-None-Match HTTP header in plain view
In-Reply-To: <20140811213205.GD1261@serenity.lan>
References: <1407790403-12468-1-git-send-email-damiannohales@gmail.com>
 <20140811213205.GD1261@serenity.lan>
Message-ID: <CALUQbeqEdt5a6ekC61bv=275joRfbTrrO+d-fJs7C08gNZ+NfQ@mail.gmail.com>

(Sorry, I forgot to include the list address in my response.)

This does not interact with CGit's cache at all, this interacts with
client side cache.

If the client sends an Etag it means "Hey, I have the content cached
for this URL in the client and its Etag is abc123", so, for a given
URL, if server calculates the Etag and results to the same as the Etag
sent by client, it means "Ok, client has the same Etag than me, so
client's cached content is up-to-date".

The Etag is calculated from the requested object hash. I'm not a Git
expert, but I know that the hash will be modified when the file is
modified by a commit.

So, if we request a file, then, one year later, we request the same
file and we still have it cached with its Etag, CGit will respond 304
Not Modified if the file was not modified by newer commits (CGit
doesn't check its internal cache, it checks the client's cache).

Actually, we should avoid send "Expires" and "Last-Modified" when
"Etag" is available, so the caching information is consistent (and
also, using Etag we have a more precise and smart caching).

2014-08-11 18:32 GMT-03:00 John Keeping <john at keeping.me.uk>:
> On Mon, Aug 11, 2014 at 05:53:23PM -0300, Dami?n Nohales wrote:
>> We are sending Etag to clients but this header is basically unusefulness
>> if the server doesn't tell the client if the content has been changed or
>> not for a given Path/Etag pair.
>>
>> Signed-off-by: Dami?n Nohales <damiannohales at gmail.com>
>> ---
>
> How does this interact with CGit's cache?  What happens if the original
> page expires from the cache and then a request comes in with a matching
> Etag?
>
>>  cgit.c      |  1 +
>>  cgit.h      |  1 +
>>  ui-plain.c  | 41 +++++++++++++++++++++--------------------
>>  ui-shared.c | 20 ++++++++++++++++++++
>>  ui-shared.h |  1 +
>>  5 files changed, 44 insertions(+), 20 deletions(-)
>>
>> diff --git a/cgit.c b/cgit.c
>> index 8c4517d..7af7712 100644
>> --- a/cgit.c
>> +++ b/cgit.c
>> @@ -385,6 +385,7 @@ static void prepare_context(void)
>>       ctx.env.server_port = getenv("SERVER_PORT");
>>       ctx.env.http_cookie = getenv("HTTP_COOKIE");
>>       ctx.env.http_referer = getenv("HTTP_REFERER");
>> +     ctx.env.if_none_match = getenv("HTTP_IF_NONE_MATCH");
>>       ctx.env.content_length = getenv("CONTENT_LENGTH") ? strtoul(getenv("CONTENT_LENGTH"), NULL, 10) : 0;
>>       ctx.env.authenticated = 0;
>>       ctx.page.mimetype = "text/html";
>> diff --git a/cgit.h b/cgit.h
>> index 0badc64..eddd4c7 100644
>> --- a/cgit.h
>> +++ b/cgit.h
>> @@ -282,6 +282,7 @@ struct cgit_environment {
>>       const char *server_port;
>>       const char *http_cookie;
>>       const char *http_referer;
>> +     const char *if_none_match;
>>       unsigned int content_length;
>>       int authenticated;
>>  };
>> diff --git a/ui-plain.c b/ui-plain.c
>> index 30fff89..a08dc5b 100644
>> --- a/ui-plain.c
>> +++ b/ui-plain.c
>> @@ -103,8 +103,8 @@ static int print_object(const unsigned char *sha1, const char *path)
>>       ctx.page.filename = path;
>>       ctx.page.size = size;
>>       ctx.page.etag = sha1_to_hex(sha1);
>> -     cgit_print_http_headers();
>> -     html_raw(buf, size);
>> +     if (!cgit_print_http_headers_matching_etag())
>> +             html_raw(buf, size);
>>       /* If we allocated this, then casting away const is safe. */
>>       if (freemime)
>>               free((char*) ctx.page.mimetype);
>> @@ -128,24 +128,25 @@ static void print_dir(const unsigned char *sha1, const char *base,
>>       fullpath = buildpath(base, baselen, path);
>>       slash = (fullpath[0] == '/' ? "" : "/");
>>       ctx.page.etag = sha1_to_hex(sha1);
>> -     cgit_print_http_headers();
>> -     htmlf("<html><head><title>%s", slash);
>> -     html_txt(fullpath);
>> -     htmlf("</title></head>\n<body>\n<h2>%s", slash);
>> -     html_txt(fullpath);
>> -     html("</h2>\n<ul>\n");
>> -     len = strlen(fullpath);
>> -     if (len > 1) {
>> -             fullpath[len - 1] = 0;
>> -             slash = strrchr(fullpath, '/');
>> -             if (slash)
>> -                     *(slash + 1) = 0;
>> -             else
>> -                     fullpath = NULL;
>> -             html("<li>");
>> -             cgit_plain_link("../", NULL, NULL, ctx.qry.head, ctx.qry.sha1,
>> -                             fullpath);
>> -             html("</li>\n");
>> +     if (!cgit_print_http_headers_matching_etag()) {
>> +             htmlf("<html><head><title>%s", slash);
>> +             html_txt(fullpath);
>> +             htmlf("</title></head>\n<body>\n<h2>%s", slash);
>> +             html_txt(fullpath);
>> +             html("</h2>\n<ul>\n");
>> +             len = strlen(fullpath);
>> +             if (len > 1) {
>> +                     fullpath[len - 1] = 0;
>> +                     slash = strrchr(fullpath, '/');
>> +                     if (slash)
>> +                             *(slash + 1) = 0;
>> +                     else
>> +                             fullpath = NULL;
>> +                     html("<li>");
>> +                     cgit_plain_link("../", NULL, NULL, ctx.qry.head, ctx.qry.sha1,
>> +                                     fullpath);
>> +                     html("</li>\n");
>> +             }
>>       }
>>       free(fullpath);
>>  }
>> diff --git a/ui-shared.c b/ui-shared.c
>> index 9dde0a3..84c7efd 100644
>> --- a/ui-shared.c
>> +++ b/ui-shared.c
>> @@ -661,6 +661,26 @@ void cgit_print_http_headers(void)
>>               exit(0);
>>  }
>>
>> +int cgit_print_http_headers_matching_etag(void)
>> +{
>> +     int match = 0;
>> +     char *etag;
>> +     if (ctx.page.etag && ctx.env.if_none_match) {
>> +             etag = fmtalloc("\"%s\"", ctx.page.etag);
>> +             if (!strcmp(etag, ctx.env.if_none_match)) {
>> +                     ctx.page.status = 304;
>> +                     ctx.page.statusmsg = "Not Modified";
>> +                     ctx.page.mimetype = NULL;
>> +                     ctx.page.size = 0;
>> +                     ctx.page.filename = NULL;
>> +                     match = 1;
>> +             }
>> +             free(etag);
>> +     }
>> +     cgit_print_http_headers();
>> +     return match;
>> +}
>> +
>>  void cgit_print_docstart(void)
>>  {
>>       if (ctx.cfg.embedded) {
>> diff --git a/ui-shared.h b/ui-shared.h
>> index 3e7a91b..e279f42 100644
>> --- a/ui-shared.h
>> +++ b/ui-shared.h
>> @@ -60,6 +60,7 @@ extern void cgit_vprint_error(const char *fmt, va_list ap);
>>  extern void cgit_print_date(time_t secs, const char *format, int local_time);
>>  extern void cgit_print_age(time_t t, time_t max_relative, const char *format);
>>  extern void cgit_print_http_headers(void);
>> +extern int cgit_print_http_headers_matching_etag(void);
>>  extern void cgit_print_docstart(void);
>>  extern void cgit_print_docend();
>>  extern void cgit_print_pageheader(void);
>> --
>> 2.0.4
>>
>> _______________________________________________
>> CGit mailing list
>> CGit at lists.zx2c4.com
>> http://lists.zx2c4.com/mailman/listinfo/cgit