%s", slash); > >> >> - html_txt(fullpath); > >> >>

From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Tue, 12 Aug 2014 20:15:23 +0100 Subject: [PATCH] Handle If-None-Match HTTP header in plain view In-Reply-To: References: <1407790403-12468-1-git-send-email-damiannohales@gmail.com> <20140811213205.GD1261@serenity.lan> <20140812090037.GE1261@serenity.lan> Message-ID: <20140812191523.GA7857@serenity.lan> On Tue, Aug 12, 2014 at 12:38:35PM -0300, Dami?n Nohales wrote: > 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. Another way to do it would be to only handle "If-None-Match" if the content is already cached and then parse the Etag header from the cached content. I think it comes back to what Lars wrote in the commit that added the current level of Etag support (commit 488a214): Todo: add support for HEAD requests... If we have sufficient infrastructure to handle HEAD requests then it should be trivial to add proper Etag handling on top, but I don't think it's trivial to add that. > 2014-08-12 6:00 GMT-03:00 John Keeping : > > 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 : > >> > 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 > >> >> --- > >> > > >> > 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("%s", slash); > >> >> - html_txt(fullpath); > >> >> - htmlf("\n\n

%s", slash); > >> >> - html_txt(fullpath); > >> >> - html("

"); > >> >> - cgit_plain_link("../", NULL, NULL, ctx.qry.head, ctx.qry.sha1, > >> >> - fullpath); > >> >> - html("

%s", slash); > >> >> + html_txt(fullpath); > >> >> + htmlf("

%s", slash); > >> >> + html_txt(fullpath); > >> >> + html("

"); > >> >> + cgit_plain_link("../", NULL, NULL, ctx.qry.head, ctx.qry.sha1, > >> >> + fullpath); > >> >> + html("