On Sat, 2011-10-08 at 21:18 +0200, Henrik Nordström wrote:
> fre 2011-10-07 klockan 19:18 +0100 skrev Andrew Beverley:
>
> > 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.
>
> Why are you allocating the DBT structures?
With the newer version of Berkeley DB, the structures need to be
initialised to null before use. I thought that in order to do so, I had
to allocate the memory. This was wrong, so I have now just made sure
that they are properly initiated before use in the new patch attached.
> Also you are not freeing them, causing a memory leak on each lookup.
>
Good point, although I've remove the malloc now as above.
I've made a couple of other minor changes - please find updated patch
attached for merge assuming you are happy with it. When using the DB
environment, I have hard-coded the name of the database file within the
directory. I can't see any problems with this, but please let me know if
you disagree (I did originally used program_name, but this doesn't work
when it contains the full path).
I have also removed the ->sync calls, as these are not required with the
improved synchronisation of the DB environment.
Finally, I noticed that in doc/manuals/* there is references to the
session helper. Should I be updating these files as well? If so, why is
the information repeated between that location and the session helper
manual in helpers/external_acl/session/?
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 within 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-09 13:17:47 +0000
@@ -1,11 +1,11 @@
-.if !'po4a'hide' .TH ext_session_acl 8 "19 September 2011"
+.if !'po4a'hide' .TH ext_session_acl 8 "9 October 2011"
.
.SH NAME
.if !'po4a'hide' .B ext_session_acl
.if !'po4a'hide' \-
Squid session tracking external acl helper.
.PP
-Version 1.1
+Version 1.2
.
.SH SYNOPSIS
.if !'po4a'hide' .B ext_session_acl
@@ -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-09 19:02:17 +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,45 +48,71 @@
#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, "session", NULL, DB_BTREE, DB_CREATE, 0666)) {
+ fprintf(stderr, "FATAL: %s: Failed to open db file '%s' in dir '%s'\n",
+ program_name, "session", 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;
+ DBT key = {0};
+ DBT data = {0};
key.data = (void *)details;
key.size = len;
- if (db->get(db, &key, &data, 0) == 0) {
+ if (db->get(db, NULL, &key, &data, 0) == 0) {
time_t 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);
return 0;
}
memcpy(×tamp, data.data, sizeof(timestamp));
@@ -95,22 +124,22 @@
static void session_login(const char *details, size_t len)
{
- DBT key, data;
+ DBT key = {0};
+ DBT data = {0};
+ 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);
- db->sync(db, 0);
+ db->put(db, NULL, &key, &data, 0);
}
static void session_logout(const char *details, size_t len)
{
- DBT key;
+ DBT key = {0};
key.data = (void *)details;
key.size = len;
- db->del(db, &key, 0);
+ db->del(db, NULL, &key, 0);
}
static void usage(void)
@@ -156,21 +185,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) {
=== modified file 'src/squid.8.in'
--- src/squid.8.in 2011-05-04 02:03:34 +0000
+++ src/squid.8.in 2011-10-09 13:22:56 +0000
@@ -265,7 +265,7 @@
.if !'po4a'hide' .B pam_auth "(8), "
.if !'po4a'hide' .B squid_ldap_auth "(8), "
.if !'po4a'hide' .B squid_ldap_group "(8), "
-.if !'po4a'hide' .B squid_session "(8), "
+.if !'po4a'hide' .B ext_session_acl "(8), "
.if !'po4a'hide' .B squid_unix_group "(8), "
.br
The Squid FAQ wiki
