Further to discussions, please find attached a patch for the session
helper to:

- Remove support for Berkeley DB 1.85
- Add support for the current Berkeley DB (db.h)
- Add support for a DB environment (fixes synchronisation between
multiple processes)
- Fix the active mode bug previously submitted (but not accepted)

I admit that I am rushing to submit this as I go away for the weekend,
so please let me know if it's not up to scratch! Works For Me (TM)
though.

Thanks,

Andy


This patch makes the following changes to the session helper:

- Removes support for Berkeley DB 1.85
- Adds support for the current Berkeley DB (db.h)
- Adds support for a DB environment (if a directory is specified as
  the path then an environment is created). This gives better
  synchronisation to multiple processes
- Fixes a bug with active mode where LOGIN/LOGOUT did not write to the DB

=== modified file 'helpers/external_acl/session/ext_session_acl.8'
--- helpers/external_acl/session/ext_session_acl.8	2011-09-21 00:19:19 +0000
+++ helpers/external_acl/session/ext_session_acl.8	2011-10-07 18:10:31 +0000
@@ -52,9 +52,15 @@
 .if !'po4a'hide' .TP
 .if !'po4a'hide' .B "\-b path"
 .B Path
-to persistent database. If not specified the session details
-will be kept in memory only and all sessions will reset each time
-Squid restarts its helpers (Squid restart or rotation of logs).
+to persistent database. If a file is specified then that single file is
+used as the database. If a path is specified, a Berkeley DB database
+environment is created within the directory. The advantage of the latter
+is better database support between multiple instances of the session
+helper. Using multiple instances of the session helper with a single
+database file will cause synchronisation problems between processes.
+If this option is not specified the session details will be kept in
+memory only and all sessions will reset each time Squid restarts its
+helpers (Squid restart or rotation of logs).
 .
 .if !'po4a'hide' .TP
 .if !'po4a'hide' .B \-a

=== modified file 'helpers/external_acl/session/ext_session_acl.cc'
--- helpers/external_acl/session/ext_session_acl.cc	2011-09-21 00:19:19 +0000
+++ helpers/external_acl/session/ext_session_acl.cc	2011-10-07 18:10:27 +0000
@@ -23,19 +23,22 @@
 #endif
 #include "helpers/defines.h"
 
-#include <sys/types.h>
-#include <sys/stat.h>
+#if HAVE_DB_H
+#include <db.h>
+#endif
 #include <fcntl.h>
+#if HAVE_GETOPT_H
+#include <getopt.h>
+#endif
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <time.h>
 #if HAVE_UNISTD_H
 #include <unistd.h>
 #endif
-#include <string.h>
-#include <time.h>
-#if HAVE_GETOPT_H
-#include <getopt.h>
-#endif
 
 /* At this point all Bit Types are already defined, so we must
    protect from multiple type definition on platform where
@@ -45,48 +48,79 @@
 #define        __BIT_TYPES_DEFINED__
 #endif
 
-#if HAVE_DB_185_H
-#include <db_185.h>
-#elif HAVE_DB_H
-#include <db.h>
-#endif
-
 static int session_ttl = 3600;
 static int fixed_timeout = 0;
 char *db_path = NULL;
 const char *program_name;
 
 DB *db = NULL;
+DB_ENV *db_env = NULL;
 
 static void init_db(void)
 {
-    db = dbopen(db_path, O_CREAT | O_RDWR, 0666, DB_BTREE, NULL);
-    if (!db) {
-        fprintf(stderr, "FATAL: %s: Failed to open session db '%s'\n", program_name, db_path);
-        exit(1);
+    struct stat st_buf;
+
+    if (db_path) {
+        if (!stat(db_path, &st_buf)) {
+            if (S_ISDIR (st_buf.st_mode)) {
+                /* If directory then open database environment. This prevents sync problems
+                    between different processes. Otherwise fallback to single file */
+                db_env_create(&db_env, 0);
+                if (db_env->open(db_env, db_path, DB_CREATE | DB_INIT_MPOOL | DB_INIT_LOCK , 0666)) {
+                    fprintf(stderr, "FATAL: %s: Failed to open database environment in '%s'\n", program_name, db_path);
+                    db_env->close(db_env, 0);
+                    exit(1);
+                }
+                db_create(&db, db_env, 0);
+            }
+        }
+    }
+    
+    if (db_env) {
+        if (db->open(db, NULL, program_name, NULL, DB_BTREE, DB_CREATE, 0666)) {
+            fprintf(stderr, "FATAL: %s: Failed to open db file '%s' in dir '%s'\n",
+                program_name, program_name, db_path);
+            db_env->close(db_env, 0);
+            exit(1);
+        }
+    } else {
+        db_create(&db, NULL, 0);
+        if (db->open(db, NULL, db_path, NULL, DB_BTREE, DB_CREATE, 0666)) {
+            fprintf(stderr, "FATAL: %s: Failed to open session db '%s'\n", program_name, db_path);
+            exit(1);
+        }
     }
 }
 
 static void shutdown_db(void)
 {
-    db->close(db);
+    db->close(db, 0);
+    if (db_env) {
+        db_env->close(db_env, 0);
+    }
 }
 
 int session_is_active = 0;
 
 static int session_active(const char *details, size_t len)
 {
-    DBT key, data;
-    key.data = (void *)details;
-    key.size = len;
-    if (db->get(db, &key, &data, 0) == 0) {
+    DBT *key;
+    key = (DBT*)malloc(sizeof(DBT));
+    DBT *data;
+    data = (DBT*)malloc(sizeof(DBT));
+    memset(key, 0, sizeof(DBT));
+    memset(data, 0, sizeof(DBT));
+    key->data = (void *)details;
+    key->size = len;
+    if (db->get(db, NULL, key, data, 0) == 0) {
         time_t timestamp;
-        if (data.size != sizeof(timestamp)) {
+        if (data->size != sizeof(timestamp)) {
             fprintf(stderr, "ERROR: %s: CORRUPTED DATABASE (%s)\n", program_name, details);
-            db->del(db, &key, 0);
+            db->del(db, NULL, key, 0);
+            db->sync(db, 0);
             return 0;
         }
-        memcpy(&timestamp, data.data, sizeof(timestamp));
+        memcpy(&timestamp, data->data, sizeof(timestamp));
         if (timestamp + session_ttl >= time(NULL))
             return 1;
     }
@@ -95,13 +129,18 @@
 
 static void session_login(const char *details, size_t len)
 {
-    DBT key, data;
+    DBT *key;
+    key = (DBT*)malloc(sizeof(DBT));
+    DBT *data;
+    data = (DBT*)malloc(sizeof(DBT));
+    memset(key, 0, sizeof(DBT));
+    memset(data, 0, sizeof(DBT));
+    key->data = (void *)details;
+    key->size = len;
     time_t now = time(NULL);
-    key.data = (void *)details;
-    key.size = len;
-    data.data = &now;
-    data.size = sizeof(now);
-    db->put(db, &key, &data, 0);
+    data->data = &now;
+    data->size = sizeof(now);
+    db->put(db, NULL, key, data, 0);
     db->sync(db, 0);
 }
 
@@ -110,7 +149,8 @@
     DBT key;
     key.data = (void *)details;
     key.size = len;
-    db->del(db, &key, 0);
+    db->del(db, NULL, &key, 0);
+    db->sync(db, 0);
 }
 
 static void usage(void)
@@ -156,21 +196,24 @@
     while (fgets(request, HELPER_INPUT_BUFFER, stdin)) {
         int action = 0;
         const char *channel_id = strtok(request, " ");
-        const char *detail = strtok(NULL, "\n");
+        char *detail = strtok(NULL, "\n");
         if (detail == NULL) {
             // Only 1 paramater supplied. We are expecting at least 2 (including the channel ID)
             fprintf(stderr, "FATAL: %s is concurrent and requires the concurrency option to be specified.\n", program_name);
+            shutdown_db();
             exit(1);
         }
-        const char *lastdetail = strrchr(detail, ' ');
+        char *lastdetail = strrchr(detail, ' ');
         size_t detail_len = strlen(detail);
         if (lastdetail) {
             if (strcmp(lastdetail, " LOGIN") == 0) {
                 action = 1;
                 detail_len = (size_t)(lastdetail-detail);
+                *lastdetail = '\0';
             } else if (strcmp(lastdetail, " LOGOUT") == 0) {
                 action = -1;
                 detail_len = (size_t)(lastdetail-detail);
+                *lastdetail = '\0';
             }
         }
         if (action == -1) {

Reply via email to