From: Amos Jeffries <squid3@treenet.co.nz>
Date: Thu, 8 Apr 2010 11:53:16 +0000 (+1200)
Subject: Some AuthUserReuqest polish and bug removal
X-Git-Tag: SQUID_3_2_0_1~167^2~32
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ea0695f2f95464d158b2a230ca4f260d1b7e5c00;p=thirdparty%2Fsquid.git

Some AuthUserReuqest polish and bug removal

* merge multiple authenticate and authenticateChildren members from
  child classes into the parent AuthUserRequest

* severe circular ref-count links between AuthUser object and
  AuthUserRequest. It appears to be unused and causes problems by merely
  existing.

* remove entry from client_side unsettign the auth credentials of a
  request simply because it was being logged.
  The log code woud be better pulling the credentials from the request
  directly when needed instead of cloning the text. RefCount in both request
  and AuthUser holds the info in place until log output is finished.
  But that polish is left for later.
---

diff --git a/include/RefCount.h b/include/RefCount.h
index 1f9abb9d1c..87341efaa0 100644
--- a/include/RefCount.h
+++ b/include/RefCount.h
@@ -118,7 +118,7 @@ private:
 struct RefCountable_ {
     RefCountable_():count_(0) {}
 
-    virtual ~RefCountable_() {}
+    virtual ~RefCountable_() { assert(count_ == 0); }
 
     /* Not private, to allow class hierarchies */
     void RefCountReference() const {
diff --git a/src/auth/Config.h b/src/auth/Config.h
index 8c4fd66669..71855b81d7 100644
--- a/src/auth/Config.h
+++ b/src/auth/Config.h
@@ -33,10 +33,12 @@
 #define SQUID_AUTHCONFIG_H
 
 #include "auth/UserRequest.h"
+#include "HelperChildConfig.h"
 
 class StoreEntry;
 class HttpReply;
 class HttpRequest;
+class wordlist;
 
 /* for http_hdr_type parameters-by-value */
 #include "HttpHeader.h"
@@ -60,7 +62,7 @@ public:
     static AuthUserRequest::Pointer CreateAuthUser(const char *proxy_auth);
 
     static AuthConfig *Find(const char *proxy_auth);
-    AuthConfig() {}
+    AuthConfig() : authenticateChildren(20), authenticate(NULL) {}
 
     virtual ~AuthConfig() {}
 
@@ -119,6 +121,10 @@ public:
     virtual void parse(AuthConfig *, int, char *) = 0;
     /** the http string id */
     virtual const char * type() const = 0;
+
+public:
+    HelperChildConfig authenticateChildren;    
+    wordlist *authenticate;
 };
 
 namespace Auth
diff --git a/src/auth/User.cc b/src/auth/User.cc
index 90f8d70de5..cceb296ad7 100644
--- a/src/auth/User.cc
+++ b/src/auth/User.cc
@@ -57,11 +57,14 @@ AuthUser::AuthUser (AuthConfig *aConfig) :
     proxy_auth_list.head = proxy_auth_list.tail = NULL;
     proxy_match_cache.head = proxy_match_cache.tail = NULL;
     ip_list.head = ip_list.tail = NULL;
+#if USER_REQUEST_LOOP_DEAD
     requests.head = requests.tail = NULL;
+#endif
     debugs(29, 5, "AuthUser::AuthUser: Initialised auth_user '" << this << "' with refcount '" << references << "'.");
 }
 
-/* Combine two user structs. ONLY to be called from within a scheme
+/**
+ * Combine two user structs. ONLY to be called from within a scheme
  * module. The scheme module is responsible for ensuring that the
  * two users _can_ be merged without invalidating all the request
  * scheme data. The scheme is also responsible for merging any user
@@ -76,17 +79,21 @@ AuthUser::absorb(AuthUser *from)
      * data
      */
     debugs(29, 5, "authenticateAuthUserMerge auth_user '" << from << "' into auth_user '" << this << "'.");
+#if USER_REQUEST_LOOP_DEAD
     dlink_node *link = from->requests.head;
 
     while (link) {
         AuthUserRequest::Pointer *auth_user_request = static_cast<AuthUserRequest::Pointer*>(link->data);
+        /* add to our list. replace if already present. */
+        addRequest(*auth_user_request);
+        AuthUserRequest::Pointer aur = *(auth_user_request);
+        aur->user(this);
+        /* remove from other list */
         dlink_node *tmplink = link;
         link = link->next;
         dlinkDelete(tmplink, &from->requests);
-        dlinkAddTail(auth_user_request, tmplink, &requests);
-        AuthUserRequest::Pointer aur = *(auth_user_request);
-        aur->user(this);
     }
+#endif /* USER_REQUEST_LOOP_DEAD */
 
     references += from->references;
     from->references = 0;
@@ -95,7 +102,6 @@ AuthUser::absorb(AuthUser *from)
 
 AuthUser::~AuthUser()
 {
-    dlink_node *link, *tmplink;
     debugs(29, 5, "AuthUser::~AuthUser: Freeing auth_user '" << this << "' with refcount '" << references << "'.");
     assert(references == 0);
     /* were they linked in by username ? */
@@ -110,18 +116,21 @@ AuthUser::~AuthUser()
         delete usernamehash;
     }
 
+#if USER_REQUEST_LOOP_DEAD
     /* remove any outstanding requests */
-    link = requests.head;
+    dlink_node *link = requests.head;
 
     while (link) {
         debugs(29, 5, "AuthUser::~AuthUser: removing request entry '" << link->data << "'");
         AuthUserRequest::Pointer *auth_user_request = static_cast<AuthUserRequest::Pointer*>(link->data);
-        tmplink = link;
+        dlink_node *tmplink = link;
         link = link->next;
         dlinkDelete(tmplink, &requests);
+        tmplink->data = NULL;
         dlinkNodeDelete(tmplink);
         *auth_user_request = NULL;
     }
+#endif /* USER_REQUEST_LOOP_DEAD */
 
     /* free cached acl results */
     aclCacheMatchFlush(&proxy_match_cache);
@@ -166,7 +175,6 @@ AuthUser::CachedACLsReset()
         username = auth_user->username();
         /* free cached acl results */
         aclCacheMatchFlush(&auth_user->proxy_match_cache);
-
     }
 
     debugs(29, 3, "AuthUser::CachedACLsReset: Finished.");
diff --git a/src/auth/User.cci b/src/auth/User.cci
index 511a8af746..91c7988b51 100644
--- a/src/auth/User.cci
+++ b/src/auth/User.cci
@@ -57,15 +57,36 @@ AuthUser::username(char const*aString)
     }
 }
 
+#if USER_REQUEST_LOOP_DEAD
 void
 AuthUser::addRequest(AuthUserRequest::Pointer request)
 {
-    /* lock for the request link */
-    /* AYJ: 2009-12-12: WTF? locking the AuthUser because it has a reference to a request? */
-    /*    seems to me like this once-upon-a-time may have been intended to lock the AuthUserRequest */
+    /* lock for the request link (AKA lock because the request links to us) */
     lock();
 
     dlink_node *node = dlinkNodeNew();
 
     dlinkAdd(&request, node, &requests);
 }
+
+void
+AuthUser::doneRequest(AuthUserRequest::Pointer request)
+{
+    /* unlink from the auth_user struct */
+    dlink_node *link = requests.head;
+    AuthUserRequest::Pointer *auth_user_request = NULL;
+
+    while (link) {
+        auth_user_request = static_cast<AuthUserRequest::Pointer*>(link->data);
+        if (*auth_user_request == request) {
+            dlink_node *tmplink = link;
+            link = link->next;
+            dlinkDelete(tmplink, &requests);
+            tmplink->data = NULL;
+            dlinkNodeDelete(tmplink);
+            *auth_user_request = NULL;
+        }
+        link = link->next;
+    }
+}
+#endif /* USER_REQUEST_LOOP_DEAD */
diff --git a/src/auth/User.h b/src/auth/User.h
index af31988c16..ccad09a4a6 100644
--- a/src/auth/User.h
+++ b/src/auth/User.h
@@ -74,9 +74,6 @@ public:
     long expiretime;
     /** how many references are outstanding to this instance */
     size_t references;
-    /** the auth_user_request structures that link to this. Yes it could be a splaytree
-     * but how many requests will a single username have in parallel? */
-    dlink_list requests;
 
     static void cacheInit();
     static void CachedACLsReset();
@@ -85,10 +82,25 @@ public:
     virtual ~AuthUser();
     _SQUID_INLINE_ char const *username() const;
     _SQUID_INLINE_ void username(char const *);
+
+    /* Manage list of IPs using this username */
     void clearIp();
     void removeIp(IpAddress);
     void addIp(IpAddress);
+
+#if USER_REQUEST_LOOP_DEAD
+protected:
+    /* manage list of active authentication requests for this username */
+    /** the auth_user_request structures that link to this. Yes it could be a splaytree
+     * but how many requests will a single username have in parallel? */
+    dlink_list requests;
+
+    /* AYJ: why? do we need this here? it forms the core of a circular refcount. */
+
+public:
     _SQUID_INLINE_ void addRequest(AuthUserRequest::Pointer);
+    _SQUID_INLINE_ void doneRequest(AuthUserRequest::Pointer);
+#endif /* USER_REQUEST_LOOP_DEAD */
 
     void lock();
     void unlock();
@@ -96,10 +108,10 @@ public:
     void addToNameCache();
 
 protected:
-    AuthUser (AuthConfig *);
+    AuthUser(AuthConfig *);
 
 private:
-    static void cacheCleanup (void *unused);
+    static void cacheCleanup(void *unused);
 
     /**
      * DPW 2007-05-08
diff --git a/src/auth/UserRequest.cc b/src/auth/UserRequest.cc
index 75a281202a..493ae4a048 100644
--- a/src/auth/UserRequest.cc
+++ b/src/auth/UserRequest.cc
@@ -135,32 +135,20 @@ AuthUserRequest::AuthUserRequest():_auth_user(NULL), message(NULL),
 
 AuthUserRequest::~AuthUserRequest()
 {
-    dlink_node *link;
+    assert(RefCountCount()==0);
     debugs(29, 5, "AuthUserRequest::~AuthUserRequest: freeing request " << this);
 
     if (user()) {
+#if USER_REQUEST_LOOP_DEAD
         /* AYJ: something strange: in order to be deleted this object must not be
          * referenced anywhere. Including the AuthUser list of requests.
          * I expect the following loop to NEVER find a pointer to this request object.
          */
+        user()->doneRequest(this);
+#endif /* USER_REQUEST_LOOP_DEAD */
 
-        /* unlink from the auth_user struct */
-        link = user()->requests.head;
-
-        while (link) {
-
-            assert( static_cast<AuthUserRequest::Pointer*>(link->data)->getRaw() != this );
-
-            if (static_cast<AuthUserRequest::Pointer*>(link->data)->getRaw() == this) {
-                dlinkDelete(link, &user()->requests);
-                dlinkNodeDelete(link);
-            }
-            link = link->next;
-        }
-
-        /* unlock the request structure's lock */
+        /* unlock the request structure's lock and release it */
         user()->unlock();
-
         user(NULL);
     }
 
diff --git a/src/auth/basic/auth_basic.cc b/src/auth/basic/auth_basic.cc
index 550119e383..d73c952c13 100644
--- a/src/auth/basic/auth_basic.cc
+++ b/src/auth/basic/auth_basic.cc
@@ -212,8 +212,6 @@ AuthBasicConfig::dump(StoreEntry * entry, const char *name, AuthConfig * scheme)
 }
 
 AuthBasicConfig::AuthBasicConfig() :
-        authenticateChildren(20),
-        authenticate(NULL),
         credentialsTTL( 2*60*60 ),
         casesensitive(0),
         utf8(0)
@@ -387,6 +385,10 @@ BasicUser::valid() const
     return true;
 }
 
+/**
+ * Generate a duplicate of the bad credentials before clearing the working copy.
+ * apparently for logging, but WTF?!
+ */
 void
 BasicUser::makeLoggingInstance(AuthUserRequest::Pointer auth_user_request)
 {
@@ -401,8 +403,10 @@ BasicUser::makeLoggingInstance(AuthUserRequest::Pointer auth_user_request)
         username(NULL);
         /* set the auth_user type */
         basic_auth->auth_type = AUTH_BROKEN;
+#if USER_REQUEST_LOOP_DEAD
         /* link the request to the user */
         basic_auth->addRequest(auth_user_request);
+#endif /* USER_REQUEST_LOOP_DEAD */
     }
 }
 
@@ -494,7 +498,9 @@ AuthBasicConfig::decode(char const *proxy_auth)
 
     /* link the request to the in-cache user */
     auth_user_request->user(basic_auth);
+#if USER_REQUEST_LOOP_DEAD
     basic_auth->addRequest(auth_user_request);
+#endif /* USER_REQUEST_LOOP_DEAD */
     return auth_user_request;
 }
 
diff --git a/src/auth/basic/auth_basic.h b/src/auth/basic/auth_basic.h
index 29f9dc11f6..e9f12d0089 100644
--- a/src/auth/basic/auth_basic.h
+++ b/src/auth/basic/auth_basic.h
@@ -67,8 +67,6 @@ MEMPROXY_CLASS_INLINE(BasicUser);
 
 typedef class BasicUser basic_data;
 
-#include "HelperChildConfig.h"
-
 /* configuration runtime data */
 
 class AuthBasicConfig : public AuthConfig
@@ -87,8 +85,6 @@ public:
     virtual void parse(AuthConfig *, int, char *);
     virtual void registerWithCacheManager(void);
     virtual const char * type() const;
-    HelperChildConfig authenticateChildren;
-    wordlist *authenticate;
     char *basicAuthRealm;
     time_t credentialsTTL;
     int casesensitive;
diff --git a/src/auth/basic/basicUserRequest.h b/src/auth/basic/basicUserRequest.h
index 7e1975ad25..fabd599247 100644
--- a/src/auth/basic/basicUserRequest.h
+++ b/src/auth/basic/basicUserRequest.h
@@ -16,7 +16,7 @@ public:
     MEMPROXY_CLASS(AuthBasicUserRequest);
 
     AuthBasicUserRequest() {};
-    virtual ~AuthBasicUserRequest() {};
+    virtual ~AuthBasicUserRequest() { assert(RefCountCount()==0); };
 
     virtual int authenticated() const;
     virtual void authenticate(HttpRequest * request, ConnStateData *conn, http_hdr_type type);
diff --git a/src/auth/digest/auth_digest.cc b/src/auth/digest/auth_digest.cc
index 865a142d99..8a163e500e 100644
--- a/src/auth/digest/auth_digest.cc
+++ b/src/auth/digest/auth_digest.cc
@@ -63,7 +63,7 @@ static hash_table *digest_nonce_cache;
 static int authdigest_initialised = 0;
 static MemAllocator *digest_nonce_pool = NULL;
 
-CBDATA_TYPE(DigestAuthenticateStateData);
+// CBDATA_TYPE(DigestAuthenticateStateData);
 
 enum http_digest_attr_type {
     DIGEST_USERNAME,
@@ -676,7 +676,7 @@ AuthDigestConfig::done()
     safe_free(digestAuthRealm);
 }
 
-AuthDigestConfig::AuthDigestConfig() : authenticateChildren(20), authenticate(NULL)
+AuthDigestConfig::AuthDigestConfig()
 {
     /* TODO: move into initialisation list */
     /* 5 minutes */
@@ -828,7 +828,9 @@ authDigestLogUsername(char *username, AuthUserRequest::Pointer auth_user_request
     /* link the request to the user */
     auth_user_request->user(digest_user);
     digest_user->lock();
+#if USER_REQUEST_LOOP_DEAD
     digest_user->addRequest(auth_user_request);
+#endif
     return auth_user_request;
 }
 
@@ -1090,8 +1092,9 @@ AuthDigestConfig::decode(char const *proxy_auth)
 
     digest_user->lock();
     digest_request->user(digest_user);
-
+#if USER_REQUEST_LOOP_DEAD
     digest_user->addRequest(digest_request);
+#endif
 
     debugs(29, 9, "username = '" << digest_user->username() << "'\nrealm = '" <<
            digest_request->realm << "'\nqop = '" << digest_request->qop <<
diff --git a/src/auth/digest/auth_digest.h b/src/auth/digest/auth_digest.h
index 8b021ddb2f..dfbe62ae6a 100644
--- a/src/auth/digest/auth_digest.h
+++ b/src/auth/digest/auth_digest.h
@@ -74,8 +74,6 @@ extern int authDigestNonceIsValid(digest_nonce_h * nonce, char nc[9]);
 extern const char *authenticateDigestNonceNonceb64(const digest_nonce_h * nonce);
 extern const int authDigestNonceLastRequest(digest_nonce_h * nonce);
 
-#include "HelperChildConfig.h"
-
 /* configuration runtime data */
 
 class AuthDigestConfig : public AuthConfig
@@ -93,9 +91,7 @@ public:
     virtual void parse(AuthConfig *, int, char *);
     virtual void registerWithCacheManager(void);
     virtual const char * type() const;
-    HelperChildConfig authenticateChildren;
     char *digestAuthRealm;
-    wordlist *authenticate;
     time_t nonceGCInterval;
     time_t noncemaxduration;
     unsigned int noncemaxuses;
diff --git a/src/auth/digest/digestUserRequest.cc b/src/auth/digest/digestUserRequest.cc
index de45320b3b..e3c06ce530 100644
--- a/src/auth/digest/digestUserRequest.cc
+++ b/src/auth/digest/digestUserRequest.cc
@@ -26,6 +26,8 @@ AuthDigestUserRequest::AuthDigestUserRequest() :
  */
 AuthDigestUserRequest::~AuthDigestUserRequest()
 {
+    assert(RefCountCount()==0);
+
     safe_free(nonceb64);
     safe_free(cnonce);
     safe_free(realm);
diff --git a/src/auth/negotiate/auth_negotiate.cc b/src/auth/negotiate/auth_negotiate.cc
index b8ae8ed092..6e9a6ff630 100644
--- a/src/auth/negotiate/auth_negotiate.cc
+++ b/src/auth/negotiate/auth_negotiate.cc
@@ -116,7 +116,7 @@ AuthNegotiateConfig::dump(StoreEntry * entry, const char *name, AuthConfig * sch
 
 }
 
-AuthNegotiateConfig::AuthNegotiateConfig() : authenticateChildren(20), keep_alive(1), authenticate(NULL)
+AuthNegotiateConfig::AuthNegotiateConfig() : keep_alive(1)
 { }
 
 void
@@ -312,7 +312,9 @@ AuthNegotiateConfig::decode(char const *proxy_auth)
 
     auth_user_request->user(newUser);
     auth_user_request->user()->auth_type = AUTH_NEGOTIATE;
+#if USER_REQUEST_LOOP_DEAD
     auth_user_request->user()->addRequest(auth_user_request);
+#endif
 
     /* all we have to do is identify that it's Negotiate - the helper does the rest */
     debugs(29, 9, "AuthNegotiateConfig::decode: Negotiate authentication");
diff --git a/src/auth/negotiate/auth_negotiate.h b/src/auth/negotiate/auth_negotiate.h
index 22f9b37dad..d0cdd31038 100644
--- a/src/auth/negotiate/auth_negotiate.h
+++ b/src/auth/negotiate/auth_negotiate.h
@@ -35,8 +35,6 @@ public:
 
 MEMPROXY_CLASS_INLINE(NegotiateUser);
 
-#include "HelperChildConfig.h"
-
 extern statefulhelper *negotiateauthenticators;
 
 /* configuration runtime data */
@@ -57,9 +55,7 @@ public:
     virtual void parse(AuthConfig *, int, char *);
     virtual void registerWithCacheManager(void);
     virtual const char * type() const;
-    HelperChildConfig authenticateChildren;
     int keep_alive;
-    wordlist *authenticate;
 };
 
 extern AuthNegotiateConfig negotiateConfig;
diff --git a/src/auth/negotiate/negotiateUserRequest.cc b/src/auth/negotiate/negotiateUserRequest.cc
index 9935dc8ef0..dbf4511131 100644
--- a/src/auth/negotiate/negotiateUserRequest.cc
+++ b/src/auth/negotiate/negotiateUserRequest.cc
@@ -25,6 +25,7 @@ AuthNegotiateUserRequest::AuthNegotiateUserRequest() :
 
 AuthNegotiateUserRequest::~AuthNegotiateUserRequest()
 {
+    assert(RefCountCount()==0);
     safe_free(server_blob);
     safe_free(client_blob);
 
diff --git a/src/auth/ntlm/auth_ntlm.cc b/src/auth/ntlm/auth_ntlm.cc
index 36eb0a5b39..dbc1515e29 100644
--- a/src/auth/ntlm/auth_ntlm.cc
+++ b/src/auth/ntlm/auth_ntlm.cc
@@ -104,7 +104,7 @@ AuthNTLMConfig::dump(StoreEntry * entry, const char *name, AuthConfig * scheme)
 
 }
 
-AuthNTLMConfig::AuthNTLMConfig() : authenticateChildren(20), keep_alive(1), authenticate(NULL)
+AuthNTLMConfig::AuthNTLMConfig() : keep_alive(1)
 { }
 
 void
@@ -283,7 +283,9 @@ AuthNTLMConfig::decode(char const *proxy_auth)
 
     auth_user_request->user(newUser);
     auth_user_request->user()->auth_type = AUTH_NTLM;
+#if USER_REQUEST_LOOP_DEAD
     auth_user_request->user()->addRequest(auth_user_request);
+#endif
 
     /* all we have to do is identify that it's NTLM - the helper does the rest */
     debugs(29, 9, "AuthNTLMConfig::decode: NTLM authentication");
diff --git a/src/auth/ntlm/auth_ntlm.h b/src/auth/ntlm/auth_ntlm.h
index 58f532d997..d61eece5e1 100644
--- a/src/auth/ntlm/auth_ntlm.h
+++ b/src/auth/ntlm/auth_ntlm.h
@@ -28,8 +28,6 @@ MEMPROXY_CLASS_INLINE(NTLMUser);
 
 typedef class NTLMUser ntlm_user_t;
 
-#include "HelperChildConfig.h"
-
 /* configuration runtime data */
 
 class AuthNTLMConfig : public AuthConfig
@@ -47,9 +45,7 @@ public:
     virtual void parse(AuthConfig *, int, char *);
     virtual void registerWithCacheManager(void);
     virtual const char * type() const;
-    HelperChildConfig authenticateChildren;
     int keep_alive;
-    wordlist *authenticate;
 };
 
 typedef class AuthNTLMConfig auth_ntlm_config;
diff --git a/src/auth/ntlm/ntlmUserRequest.cc b/src/auth/ntlm/ntlmUserRequest.cc
index 34317dbb7e..0e41ccf7a8 100644
--- a/src/auth/ntlm/ntlmUserRequest.cc
+++ b/src/auth/ntlm/ntlmUserRequest.cc
@@ -19,6 +19,7 @@ AuthNTLMUserRequest::AuthNTLMUserRequest() :
 
 AuthNTLMUserRequest::~AuthNTLMUserRequest()
 {
+    assert(RefCountCount()==0);
     safe_free(server_blob);
     safe_free(client_blob);
 
diff --git a/src/client_side.cc b/src/client_side.cc
index 60defc884f..ec53114f4f 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -504,7 +504,7 @@ prepareLogWithRequestDetails(HttpRequest * request, AccessLogEntry * aLogEntry)
         if (request->auth_user_request->username())
             aLogEntry->cache.authuser = xstrdup(request->auth_user_request->username());
 
-        request->auth_user_request = NULL;
+// WTF??        request->auth_user_request = NULL;
     }
 }
 
diff --git a/src/tests/testAuth.cc b/src/tests/testAuth.cc
index b4ea356396..449cb689c5 100644
--- a/src/tests/testAuth.cc
+++ b/src/tests/testAuth.cc
@@ -202,7 +202,9 @@ testAuthBasicUserRequest::username()
     BasicUser *basic_auth=new BasicUser(AuthConfig::Find("basic"));
     basic_auth->username("John");
     temp->user(basic_auth);
+#if USER_REQUEST_LOOP_DEAD
     basic_auth->addRequest(temp);
+#endif
     CPPUNIT_ASSERT_EQUAL(0, strcmp("John", temp->username()));
 }
 #endif /* HAVE_AUTH_MODULE_BASIC */
@@ -226,7 +228,9 @@ testAuthDigestUserRequest::username()
     DigestUser *duser=new DigestUser(AuthConfig::Find("digest"));
     duser->username("John");
     temp->user(duser);
+#if USER_REQUEST_LOOP_DEAD
     duser->addRequest(temp);
+#endif
     CPPUNIT_ASSERT_EQUAL(0, strcmp("John", temp->username()));
 }
 #endif /* HAVE_AUTH_MODULE_DIGEST */
@@ -250,7 +254,9 @@ testAuthNTLMUserRequest::username()
     NTLMUser *nuser=new NTLMUser(AuthConfig::Find("ntlm"));
     nuser->username("John");
     temp->user(nuser);
+#if USER_REQUEST_LOOP_DEAD
     nuser->addRequest(temp);
+#endif
     CPPUNIT_ASSERT_EQUAL(0, strcmp("John", temp->username()));
 }
 #endif /* HAVE_AUTH_MODULE_NTLM */
@@ -274,7 +280,9 @@ testAuthNegotiateUserRequest::username()
     NegotiateUser *nuser=new NegotiateUser(AuthConfig::Find("negotiate"));
     nuser->username("John");
     temp->user(nuser);
+#if USER_REQUEST_LOOP_DEAD
     nuser->addRequest(temp);
+#endif
     CPPUNIT_ASSERT_EQUAL(0, strcmp("John", temp->username()));
 }