Re: [PATCH] annotation support for external ACL helpers

[Eliezer Croitoru]

Now I just had small hole in time to look.
The mentioned logic seems pretty reasonable to me.

Eliezer

On 23/11/13 13:02, Amos Jeffries wrote:

  entryData.tag = label;
@@ -1603,6 +1605,18 @@
  {
  ACLFilledChecklist*checklist = Filled(static_cast(data));
  checklist->extacl_entry = cbdataReference((external_acl_entry *)result);
+
+// attach the helper kv-pair to the transaction
+if (HttpRequest * req = checklist->request) {
+// XXX: we have no access to the transaction / AccessLogEntry so cant 
SyncNotes().
+// workaround by using anything already set in HttpRequest
+// OR use new and rely on a later Sync copying these to AccessLogEntry
+if (!req->notes)
+req->notes = new NotePairs;
+
+req->notes->appendNewOnly(&checklist->extacl_entry->notes);
+}
+
  checklist->resumeNonBlockingCheck(ExternalACLLookup::Instance());
  }

Re: [PATCH] annotation support for external ACL helpers

[Amos Jeffries]

On 24/11/2013 12:02 a.m., Amos Jeffries wrote:
> This completes the annotation support for common helper interfaces by
> making custom key=value pairs sent by external ACL helpers in to
> NotePair objects and attaching to the active request.
> 
> The other side of this - sending values to the helper is deferred until
> the helper format can be converted to logformat codes.
> 
> Amos
> 

In absence any objections and in order to get the next beta out for
further testing of all these I have applied this patch as-is.

Amos

[PATCH] annotation support for external ACL helpers

[Amos Jeffries]

This completes the annotation support for common helper interfaces by
making custom key=value pairs sent by external ACL helpers in to
NotePair objects and attaching to the active request.

The other side of this - sending values to the helper is deferred until
the helper format can be converted to logformat codes.

Amos
=== modified file 'src/ExternalACLEntry.cc'
--- src/ExternalACLEntry.cc 2013-06-27 15:58:46 +
+++ src/ExternalACLEntry.cc 2013-10-31 09:52:52 +
@@ -49,7 +49,8 @@
 
 CBDATA_CLASS_INIT(ExternalACLEntry);
 
-ExternalACLEntry::ExternalACLEntry()
+ExternalACLEntry::ExternalACLEntry() :
+notes()
 {
 lru.next = lru.prev = NULL;
 result = ACCESS_DENIED;
@@ -67,6 +68,11 @@
 {
 date = squid_curtime;
 result = someData.result;
+
+// replace all notes. not combine
+notes.entries.clean();
+notes.append(&someData.notes);
+
 #if USE_AUTH
 user = someData.user;
 password = someData.password;

=== modified file 'src/ExternalACLEntry.h'
--- src/ExternalACLEntry.h  2013-06-27 15:58:46 +
+++ src/ExternalACLEntry.h  2013-10-31 09:49:06 +
@@ -45,6 +45,7 @@
 #include "acl/Acl.h"
 #include "cbdata.h"
 #include "hash.h"
+#include "Notes.h"
 #include "SquidString.h"
 
 class external_acl;
@@ -62,6 +63,10 @@
 ExternalACLEntryData() : result(ACCESS_DUNNO) {}
 
 allow_t result;
+
+/// list of all kv-pairs returned by the helper
+NotePairs notes;
+
 #if USE_AUTH
 // TODO use an AuthUser to hold this info
 String user;
@@ -88,6 +93,10 @@
 dlink_node lru;
 allow_t result;
 time_t date;
+
+/// list of all kv-pairs returned by the helper
+NotePairs notes;
+
 #if USE_AUTH
 String user;
 String password;

=== modified file 'src/external_acl.cc'
--- src/external_acl.cc 2013-07-21 19:24:35 +
+++ src/external_acl.cc 2013-10-29 18:11:21 +
@@ -1380,6 +1380,8 @@
 
 // XXX: make entryData store a proper HelperReply object instead of 
copying.
 
+entryData.notes.append(&reply.notes);
+
 const char *label = reply.notes.findFirst("tag");
 if (label != NULL && *label != '\0')
 entryData.tag = label;
@@ -1603,6 +1605,18 @@
 {
 ACLFilledChecklist *checklist = Filled(static_cast(data));
 checklist->extacl_entry = cbdataReference((external_acl_entry *)result);
+
+// attach the helper kv-pair to the transaction
+if (HttpRequest * req = checklist->request) {
+// XXX: we have no access to the transaction / AccessLogEntry so cant 
SyncNotes().
+// workaround by using anything already set in HttpRequest
+// OR use new and rely on a later Sync copying these to AccessLogEntry
+if (!req->notes)
+req->notes = new NotePairs;
+
+req->notes->appendNewOnly(&checklist->extacl_entry->notes);
+}
+
 checklist->resumeNonBlockingCheck(ExternalACLLookup::Instance());
 }