[PATCH] Handle If-None-Match HTTP header in plain view
Damián Nohales
damiannohales at gmail.com
Tue Aug 12 17:38:35 CEST 2014
Ah... I got it.
We could implement cache disabling per command to disable it for the
plain command. I don't know if there is another way since Etag is
calculated at command execution.
2014-08-12 6:00 GMT-03:00 John Keeping <john at keeping.me.uk>:
> On Mon, Aug 11, 2014 at 07:45:53PM -0300, Damián Nohales wrote:
>> (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).
>
> Yes, but I think the code you've changed comes after CGit has duplicated
> stdout into the cache file. So if a request comes in with an
> If-None-Match header, then CGit will cache the 304 response and send it
> in response to all future request for the same page (until the cached
> copy times out).
>
>> 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
More information about the CGit
mailing list