Though I'm aware that this is not something you may consider useful
for Fossil, I'm posting a minor update, for the sake of correctness:

(0) Wrong Statement

Me:

> I think that "Vary: Cookie" is intended to work with unconditional
> HTTP requests: the browser is directed to stick to the expiry date
> and use the cached page, unless the cookies have changed.

I'm sorry I was wrong here.

"Vary: Cookie" directs clients to include cookies as part of their
"cache key", and it also works with conditional HTTP requests.

But if clients mark resources as stale due to cookie updates, they
will still "revalidate" them with conditional If-None-Match requests,
using their last-received ETag. And if the server does not consult all
the information from the cookies to generate its ETag (here, the user
login time), it won't produce a new ETag and expire the page, but
instead reply with "304 Not Modified".

(1) Introduced two Bugs

With the previous version of the patch, accessing the /uv page with no
file name specified caused an assertion failure, as doc_page()
repeatedly tries various index documents (index.html, index.wiki,
index.md, and 404.md). So the cache checks are only performed if
there's a valid hash (i.e. if "SELECT hash FROM unversioned" returns
non-empty data). Moreover, the Last-Modified response header was not
sent, if an ETag cache hit was already handled (though, at least my
Apache web server seems to purge the Last-Modified header from "304
Not Modified" responses).

(2) Updated If-Modified-Since checks

The updated patch also includes the modifications to the
If-Modified-Since checks, as suggested in a separate post.

--Florian

===================== Patch for Fossil [a7056e64] ======================

Index: src/cgi.c
==================================================================
--- src/cgi.c
+++ src/cgi.c
@@ -249,11 +249,11 @@
     iReplyStatus = 200;
     zReplyStatus = "OK";
   }

   if( g.fullHttpReply ){
-    fprintf(g.httpOut, "HTTP/1.0 %d %s\r\n", iReplyStatus, zReplyStatus);
+    fprintf(g.httpOut, "HTTP/1.1 %d %s\r\n", iReplyStatus, zReplyStatus);
     fprintf(g.httpOut, "Date: %s\r\n", cgi_rfc822_datestamp(time(0)));
     fprintf(g.httpOut, "Connection: close\r\n");
     fprintf(g.httpOut, "X-UA-Compatible: IE=edge\r\n");
   }else{
     fprintf(g.httpOut, "Status: %d %s\r\n", iReplyStatus, zReplyStatus);
@@ -269,10 +269,11 @@
     fprintf(g.httpOut, "Cache-control: no-cache\r\n");
   }
   if( etag_mtime()>0 ){
     fprintf(g.httpOut, "Last-Modified: %s\r\n",
             cgi_rfc822_datestamp(etag_mtime()));
+    fprintf(g.httpOut, "Vary: Cookie\r\n"); /* HTTP/1.0 (no ETags) */
   }

   if( blob_size(&extraHeader)>0 ){
     fprintf(g.httpOut, "%s", blob_buffer(&extraHeader));
   }

Index: src/doc.c
==================================================================
--- src/doc.c
+++ src/doc.c
@@ -641,17 +641,29 @@
       }
     }
     if( isUV ){
       if( db_table_exists("repository","unversioned") ){
         Stmt q;
+        char *zHash=0;
+        sqlite3_int64 mtime=0;
         db_prepare(&q, "SELECT hash, mtime FROM unversioned"
                        " WHERE name=%Q", zName);
         if( db_step(&q)==SQLITE_ROW ){
-          etag_check(ETAG_HASH, db_column_text(&q,0));
-          etag_last_modified(db_column_int64(&q,1));
+          zHash = fossil_strdup(db_column_text(&q,0));
+          mtime = db_column_int64(&q,1);
         }
         db_finalize(&q);
+        if( zHash ){
+          /* Only call etag_check() if the unversioned file was found
+          ** and has a valid hash, as doc_page() is called repeatedly
+          ** to search for index documents (index.html, index.wiki,
+          ** index.md, and 404.md), causing an assertion failure in
+          ** etag_check(), due to zETag already initialized. */
+          if( zHash[0] )
+            etag_check(ETAG_HASH|ETAG_CEXP, zHash, mtime);
+          free(zHash);
+        }
         if( unversioned_content(zName, &filebody)==0 ){
           rid = 1;
           zDfltTitle = zName;
         }
       }
@@ -847,11 +859,11 @@
 */
 void logo_page(void){
   Blob logo;
   char *zMime;

-  etag_check(ETAG_CONFIG, 0);
+  etag_check(ETAG_CONFIG, 0, 0);
   zMime = db_get("logo-mimetype", "image/gif");
   blob_zero(&logo);
   db_blob(&logo, "SELECT value FROM config WHERE name='logo-image'");
   if( blob_size(&logo)==0 ){
     blob_init(&logo, (char*)aLogo, sizeof(aLogo));
@@ -881,11 +893,11 @@
 */
 void background_page(void){
   Blob bgimg;
   char *zMime;

-  etag_check(ETAG_CONFIG, 0);
+  etag_check(ETAG_CONFIG, 0, 0);
   zMime = db_get("background-mimetype", "image/gif");
   blob_zero(&bgimg);
   db_blob(&bgimg, "SELECT value FROM config WHERE name='background-image'");
   if( blob_size(&bgimg)==0 ){
     blob_init(&bgimg, (char*)aBackground, sizeof(aBackground));

Index: src/etag.c
==================================================================
--- src/etag.c
+++ src/etag.c
@@ -24,10 +24,11 @@
 **   (1)  The mtime on the Fossil executable
 **   (2)  The last change to the CONFIG table
 **   (3)  The last change to the EVENT table
 **   (4)  The value of the display cookie
 **   (5)  A hash value supplied by the page generator
+**   (6)  The "user.cexpire" field for logged-in users
 **
 ** Item (1) is always included in the ETag.  The other elements are
 ** optional.  Because (1) is always included as part of the ETag, all
 ** outstanding ETags can be invalidated by touching the fossil executable.
 **
@@ -60,20 +61,21 @@
 */
 #define ETAG_CONFIG   0x01 /* Output depends on the CONFIG table */
 #define ETAG_DATA     0x02 /* Output depends on the EVENT table */
 #define ETAG_COOKIE   0x04 /* Output depends on a display cookie value */
 #define ETAG_HASH     0x08 /* Output depends on a hash */
+#define ETAG_CEXP     0x10 /* Output depends on "user.cexpire" */
 #endif

 static char zETag[33];      /* The generated ETag */
 static int iMaxAge = 0;     /* The max-age parameter in the reply */
 static sqlite3_int64 iEtagMtime = 0;  /* Last-Modified time */

 /*
 ** Generate an ETag
 */
-void etag_check(unsigned eFlags, const char *zHash){
+void etag_check(unsigned eFlags, const char *zHash, sqlite3_int64 lmt){
   sqlite3_int64 mtime;
   const char *zIfNoneMatch;
   char zBuf[50];
   assert( zETag[0]==0 );  /* Only call this routine once! */

@@ -82,10 +84,22 @@

   /* Always include the mtime of the executable as part of the hash */
   mtime = file_mtime(g.nameOfExe, ExtFILE);
   sqlite3_snprintf(sizeof(zBuf),zBuf,"mtime: %lld\n", mtime);
   md5sum_step_text(zBuf, -1);
+
+  /* Include "user.cexpire" for logged-in users in the hash */
+  if ( (eFlags & ETAG_CEXP)!=0 && g.zLogin ){
+    char *zCExp = db_text(0, "SELECT cexpire FROM user WHERE uid=%d",
+                              g.userUid);
+    if ( zCExp ){
+      md5sum_step_text("cexp: ", -1);
+      md5sum_step_text(zCExp, -1);
+      md5sum_step_text("\n", 1);
+      fossil_free(zCExp);
+    }
+  }

   if( (eFlags & ETAG_HASH)!=0 && zHash ){
     md5sum_step_text("hash: ", -1);
     md5sum_step_text(zHash, -1);
     md5sum_step_text("\n", 1);
@@ -118,11 +132,23 @@
   memcpy(zETag, md5sum_finish(0), 33);

   /* Check to see if the generated ETag matches If-None-Match and
   ** generate a 304 reply if it does. */
   zIfNoneMatch = P("HTTP_IF_NONE_MATCH");
-  if( zIfNoneMatch==0 ) return;
+  if( zIfNoneMatch==0 ){
+    /* Prevent the If-Modified-Since cache handler to undermine cache
+    ** misses already cleared by the If-None-Match cache handler, and
+    ** call it only if there's no If-None-Match request header. */
+    if ( lmt ) etag_last_modified(lmt);
+    return;
+  }
+
+  /* Make sure the Last-Modified response header will be
+  ** sent, and contain the correct time stamp, even if
+  ** etag_last_modified() was not called. */
+  iEtagMtime = lmt;
+
   if( strcmp(zIfNoneMatch,zETag)!=0 ) return;

   /* If we get this far, it means that the content has
   ** not changed and we can do a 304 reply */
   cgi_reset_content();
@@ -147,11 +173,11 @@

   /* Check to see the If-Modified-Since constraint is satisfied */
   zIfModifiedSince = P("HTTP_IF_MODIFIED_SINCE");
   if( zIfModifiedSince==0 ) return;
   x = cgi_rfc822_parsedate(zIfModifiedSince);
-  if( x<=0 || x>mtime ) return;
+  if( x<=0 || x<mtime ) return;

 #if 0
   /* If the Fossil executable is more recent than If-Modified-Since,
   ** go ahead and regenerate the resource. */
   if( file_mtime(g.nameOfExe, ExtFILE)>x ) return;
@@ -203,8 +229,8 @@
   int iKey = 0;
   db_find_and_open_repository(0, 0);
   zKey = find_option("key",0,1);
   zHash = find_option("hash",0,1);
   if( zKey ) iKey = atoi(zKey);
-  etag_check(iKey, zHash);
+  etag_check(iKey, zHash, 0);
   fossil_print("%s\n", etag_tag());
 }

Index: src/style.c
==================================================================
--- src/style.c
+++ src/style.c
@@ -858,11 +858,11 @@
     cgi_set_content_type("text/plain");
   }
   if( zId && (nId = (int)strlen(zId))>=8 &&
strncmp(zId,MANIFEST_UUID,nId)==0 ){
     g.isConst = 1;
   }else{
-    etag_check(0,0);
+    etag_check(0,0,0);
   }
   blob_init(&out, zTxt, -1);
   cgi_set_content(&out);
 }

Index: src/tar.c
==================================================================
--- src/tar.c
+++ src/tar.c
@@ -772,11 +772,11 @@
   blob_appendf(&cacheKey, "/tarball/%z", rid_to_uuid(rid));
   blob_appendf(&cacheKey, "/%q", zName);
   if( zInclude ) blob_appendf(&cacheKey, ",in=%Q", zInclude);
   if( zExclude ) blob_appendf(&cacheKey, ",ex=%Q", zExclude);
   zKey = blob_str(&cacheKey);
-  etag_check(ETAG_HASH, zKey);
+  etag_check(ETAG_HASH, zKey, 0);

   if( P("debug")!=0 ){
     style_header("Tarball Generator Debug Screen");
     @ zName = "%h(zName)"<br />
     @ rid = %d(rid)<br />

Index: src/zip.c
==================================================================
--- src/zip.c
+++ src/zip.c
@@ -942,11 +942,11 @@
   blob_appendf(&cacheKey, "/%s/%z", g.zPath, rid_to_uuid(rid));
   blob_appendf(&cacheKey, "/%q", zName);
   if( zInclude ) blob_appendf(&cacheKey, ",in=%Q", zInclude);
   if( zExclude ) blob_appendf(&cacheKey, ",ex=%Q", zExclude);
   zKey = blob_str(&cacheKey);
-  etag_check(ETAG_HASH, zKey);
+  etag_check(ETAG_HASH, zKey, 0);

   if( P("debug")!=0 ){
     style_header("%s Archive Generator Debug Screen", zType);
     @ zName = "%h(zName)"<br />
     @ rid = %d(rid)<br />

===================== Patch for Fossil [a7056e64] ======================
_______________________________________________
fossil-users mailing list
fossil-users@lists.fossil-scm.org
http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users

Reply via email to