Hi all,
I bring back this thread because we have a related project we need to
close.
Also I believe that the notes interface and usage is currently a little
broken so I believe we should fix it.
There are thinks which will never work well if we continue using notes
like this.
For example assume that a url-rewriter return a note1 note. So assume
the following configuration:
url_rewrite_program /usr/local/squid/bin/line-helper.sh OK note1=value1
url_rewrite_children 1
request_header_add X-Cookie "cookie=%{note1}note"
This is not expected to work, because we are copying the Notes to the
AccessLogEntry just before logging the request. The formatter gets the
notes to print from the AccessLogEntry object.
Because the %note needs to print both ICAP/eCAP and helper notes we need
to find a common way to use Notes, and store ICAP/eCAP and helper notes
to the same structure.
This is will help us to access the notes any time we need them, without
conversions from NotePairs to Notes or the reverse.
In the future we may need to add more squid features which operates on
both ICAP/eCAP and helpers notes, for example a new ACL.
I am attaching a patch which stores the helper notes to a NotePairs
class (as ICAP/eCAP does).
This patch:
- Converts the HttpRequest::helperNotes to a NotePairs object
- Adds three members to NotePairs:
1) NotePairs::findFirst which return the first matched note for a key
2) NotePairs::find which return as value a comma separated list of
values of the notes matching the key.
3) - NotePairs::add to add a key/value pair
- removes the uneeded methods from Note and Notes classes.
- Try to hide any references to the HttpHeaders
I do not believe that this patch breaks anything. Moreover I believe
that this patch makes the code a little faster and better.
Maybe requires a little more testing, but if we decide that it is in the
right direction I will do it.
Next steps:
- Remove the AccessLogEntry::notes
- Rename HttpRequest::helperNotes to HttpRequest::notes
- Store ICAP/eCAP notes to HttpRequest::notes
- Probably rewrite the NotePairs to not a new classes which is not
based on the HttpHeader
- Rename the Note/Notes/NotePairs classes to avoid confusion in the
future.
Regards,
Christos
On 01/28/2013 01:51 PM, Tsantilas Christos wrote:
> To summarize, lets try to see what we currently have and then see what
> we need.
>
> Currently:
> 1) the helpers may return multiple kv-pairs
>
> 2) the helper may use non valid characters for HTTP headers for key
> names. This is work well now, but we are worrying that in the future it
> may not work, because the HttpHeader API will be changed.
>
> 3) the helper currently can return a quoted value eg group="J, Smith"
> OR a url encoded value eg group=J%2C%20Smith.
>
> 4) the helper may return multiple kv-pairs with the same key
>
> 5) In most cases(?) used only the first returned value for a key. For
> example if multiple "tag=" kv-pairs returned by the helper only the
> first will be used in tag ACL.
>
> 6) If more than one kv-pairs with the same key returned then in
> access-log we will print them as a list. For example if the helper
> returned 'OK user="J, Smith" group="group1,A" group=group2' the
> formating code "%note{group}" will print:
> "group1,A,group2".
>
> 7) In the above helper reply the formating code %note will print:
> "user: J, Smith\r\ngroup: group1,A\r\ngroup: group2"
> I must note here that the two "group" kv-pair keys will be printed as
> separate entries.
>
>
=== modified file 'src/HelperReply.cc'
--- src/HelperReply.cc 2013-02-14 05:42:57 +0000
+++ src/HelperReply.cc 2013-03-14 16:13:31 +0000
@@ -126,76 +126,75 @@
}
void
HelperReply::parseResponseKeys()
{
// parse a "key=value" pair off the 'other()' buffer.
while (other().hasContent()) {
char *p = modifiableOther().content();
while (*p && *p != '=' && *p != ' ') ++p;
if (*p != '=')
return; // done. Not a key.
// whitespace between key and value is prohibited.
// workaround strwordtok() which skips whitespace prefix.
if (xisspace(*(p+1)))
return; // done. Not a key.
*p = '\0';
++p;
- const String key(other().content());
+ const char *key = other().content();
// the value may be a quoted string or a token
const bool urlDecode = (*p != '"'); // check before moving p.
char *v = strwordtok(NULL, &p);
if (v != NULL && urlDecode && (p-v) > 2) // 1-octet %-escaped requires 3 bytes
rfc1738_unescape(v);
- const String value(v?v:""); // value can be empty, but must not be NULL
- notes.add(key, value);
+ notes.add(key, v ? v : ""); // value can be empty, but must not be NULL
modifiableOther().consume(p - other().content());
modifiableOther().consumeWhitespacePrefix();
}
}
std::ostream &
operator <<(std::ostream &os, const HelperReply &r)
{
os << "{result=";
switch (r.result) {
case HelperReply::Okay:
os << "OK";
break;
case HelperReply::Error:
os << "ERR";
break;
case HelperReply::BrokenHelper:
os << "BH";
break;
case HelperReply::TT:
os << "TT";
break;
case HelperReply::Unknown:
os << "Unknown";
break;
}
// dump the helper key=pair "notes" list
- if (r.notes.notes.size() > 0) {
+ if (r.notes.entries.size() > 0) {
os << ", notes={";
- for (Notes::NotesList::const_iterator m = r.notes.notes.begin(); m != r.notes.notes.end(); ++m) {
- for (Note::Values::iterator v = (*m)->values.begin(); v != (*m)->values.end(); ++v) {
- os << ',' << (*m)->key << '=' << ConfigParser::QuoteString((*v)->value);
- }
+ NotePairs::Pos pos = NotePairs::InitPos;
+ while (NotePairs::Entry *e = r.notes.getEntry(&pos)) {
+ os << ',' << e->name << '=' << ConfigParser::QuoteString(e->value);
}
os << "}";
}
if (r.other().hasContent())
os << ", other: \"" << r.other().content() << '\"';
os << '}';
return os;
}
+
=== modified file 'src/HelperReply.h'
--- src/HelperReply.h 2012-11-27 21:19:46 +0000
+++ src/HelperReply.h 2013-03-08 15:14:52 +0000
@@ -48,35 +48,35 @@
* token are URL-decoded.
* quoted-string are \-escape decoded and the quotes are stripped.
*/
// XXX: buf should be const but we may need strwordtok() and rfc1738_unescape()
void parse(char *buf, size_t len);
public:
/// The helper response 'result' field.
enum Result_ {
Unknown, // no result code received, or unknown result code
Okay, // "OK" indicating success/positive result
Error, // "ERR" indicating success/negative result
BrokenHelper, // "BH" indicating failure due to helper internal problems.
// result codes for backward compatibility with NTLM/Negotiate
// TODO: migrate to a variant of the above results with kv-pair parameters
TT
} result;
// list of key=value pairs the helper produced
- Notes notes;
+ NotePairs notes;
/// for stateful replies the responding helper 'server' needs to be preserved across callbacks
CbcPointer<helper_stateful_server> whichServer;
private:
void parseResponseKeys();
/// the remainder of the line
MemBuf other_;
};
std::ostream &operator <<(std::ostream &os, const HelperReply &r);
#endif /* _SQUID_SRC_HELPERREPLY_H */
=== modified file 'src/HttpRequest.cc'
--- src/HttpRequest.cc 2013-02-17 02:24:54 +0000
+++ src/HttpRequest.cc 2013-03-08 16:46:59 +0000
@@ -215,42 +215,42 @@
copy->port = port;
// urlPath handled in ctor
copy->canonical = canonical ? xstrdup(canonical) : NULL;
// range handled in hdrCacheInit()
copy->ims = ims;
copy->imslen = imslen;
copy->hier = hier; // Is it safe to copy? Should we?
copy->errType = errType;
// XXX: what to do with copy->peer_login?
copy->lastmod = lastmod;
copy->vary_headers = vary_headers ? xstrdup(vary_headers) : NULL;
// XXX: what to do with copy->peer_domain?
copy->myportname = myportname;
if (helperNotes) {
- copy->helperNotes = new Notes;
- copy->helperNotes->notes = helperNotes->notes;
+ copy->helperNotes = new NotePairs;
+ copy->helperNotes->append(helperNotes);
}
copy->tag = tag;
#if USE_AUTH
copy->extacl_user = extacl_user;
copy->extacl_passwd = extacl_passwd;
#endif
copy->extacl_log = extacl_log;
copy->extacl_message = extacl_message;
assert(copy->inheritProperties(this));
return copy;
}
bool
HttpRequest::inheritProperties(const HttpMsg *aMsg)
{
const HttpRequest* aReq = dynamic_cast<const HttpRequest*>(aMsg);
if (!aReq)
return false;
=== modified file 'src/HttpRequest.h'
--- src/HttpRequest.h 2013-02-12 11:34:35 +0000
+++ src/HttpRequest.h 2013-03-08 16:12:22 +0000
@@ -186,41 +186,41 @@
HierarchyLogEntry hier;
int dnsWait; ///< sum of DNS lookup delays in milliseconds, for %dt
err_type errType;
int errDetail; ///< errType-specific detail about the transaction error
char *peer_login; /* Configured peer login:password */
char *peer_host; /* Selected peer host*/
time_t lastmod; /* Used on refreshes */
const char *vary_headers; /* Used when varying entities are detected. Changes how the store key is calculated */
char *peer_domain; /* Configured peer forceddomain */
String myportname; // Internal tag name= value from port this requests arrived in.
- Notes *helperNotes; ///< collection of meta notes associated with this request by helper lookups.
+ NotePairs *helperNotes; ///< collection of meta notes associated with this request by helper lookups.
String tag; /* Internal tag for this request */
String extacl_user; /* User name returned by extacl lookup */
String extacl_passwd; /* Password returned by extacl lookup */
String extacl_log; /* String to be used for access.log purposes */
String extacl_message; /* String to be used for error page purposes */
#if FOLLOW_X_FORWARDED_FOR
String x_forwarded_for_iterator; /* XXX a list of IP addresses */
#endif /* FOLLOW_X_FORWARDED_FOR */
public:
bool multipartRangeRequest() const;
bool parseFirstLine(const char *start, const char *end);
=== modified file 'src/Notes.cc'
--- src/Notes.cc 2013-02-12 11:34:35 +0000
+++ src/Notes.cc 2013-03-14 16:39:33 +0000
@@ -23,127 +23,89 @@
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA.
*
*/
#include "squid.h"
#include "globals.h"
#include "acl/FilledChecklist.h"
#include "acl/Gadgets.h"
#include "ConfigParser.h"
#include "HttpRequest.h"
#include "HttpReply.h"
#include "SquidConfig.h"
#include "Store.h"
#include <algorithm>
#include <string>
+const int NotePairs::InitPos = HttpHeaderInitPos;
+
Note::Value::~Value()
{
aclDestroyAclList(&aclList);
}
Note::Value::Pointer
Note::addValue(const String &value)
{
Value::Pointer v = new Value(value);
values.push_back(v);
return v;
}
const char *
Note::match(HttpRequest *request, HttpReply *reply)
{
typedef Values::iterator VLI;
ACLFilledChecklist ch(NULL, request, NULL);
ch.reply = reply;
if (reply)
HTTPMSGLOCK(ch.reply);
for (VLI i = values.begin(); i != values.end(); ++i ) {
const int ret= ch.fastCheck((*i)->aclList);
debugs(93, 5, HERE << "Check for header name: " << key << ": " << (*i)->value
<<", HttpRequest: " << request << " HttpReply: " << reply << " matched: " << ret);
if (ret == ACCESS_ALLOWED)
return (*i)->value.termedBuf();
}
return NULL;
}
Note::Pointer
-Notes::find(const String ¬eKey) const
+Notes::add(const String ¬eKey)
{
- typedef Notes::NotesList::const_iterator AMLI;
+ typedef Notes::NotesList::iterator AMLI;
for (AMLI i = notes.begin(); i != notes.end(); ++i) {
if ((*i)->key == noteKey)
return (*i);
}
- return Note::Pointer();
-}
-
-void
-Notes::add(const String ¬eKey, const String ¬eValue)
-{
- Note::Pointer key = add(noteKey);
- key->addValue(noteValue);
-}
-
-Note::Pointer
-Notes::add(const String ¬eKey)
-{
- Note::Pointer note = find(noteKey);
- if (note == NULL) {
- note = new Note(noteKey);
- notes.push_back(note);
- }
+ Note::Pointer note = new Note(noteKey);
+ notes.push_back(note);
return note;
}
-void
-Notes::add(const Notes &src)
-{
- typedef Notes::NotesList::const_iterator AMLI;
- typedef Note::Values::iterator VLI;
-
- for (AMLI i = src.notes.begin(); i != src.notes.end(); ++i) {
-
- // ensure we have a key by that name to fill out values for...
- // NP: not sharing pointers at the key level since merging other helpers
- // details later would affect this src objects keys, which is a bad idea.
- Note::Pointer ourKey = add((*i)->key);
-
- // known key names, merge the values lists...
- for (VLI v = (*i)->values.begin(); v != (*i)->values.end(); ++v ) {
- // 2012-11-29: values are read-only and Pointer can safely be shared
- // for now we share pointers to save memory and gain speed.
- // If that ever ceases to be true, convert this to a full copy.
- ourKey->values.push_back(*v);
- // TODO: prune/skip duplicates ?
- }
- }
-}
-
Note::Pointer
Notes::parse(ConfigParser &parser)
{
String key, value;
ConfigParser::ParseString(&key);
ConfigParser::ParseQuotedString(&value);
Note::Pointer note = add(key);
Note::Value::Pointer noteValue = note->addValue(value);
aclParseAclList(parser, ¬eValue->aclList);
if (blacklisted) {
for (int i = 0; blacklisted[i] != NULL; ++i) {
if (note->key.caseCmp(blacklisted[i]) == 0) {
fatalf("%s:%d: meta key \"%s\" is a reserved %s name",
cfg_filename, config_lineno, note->key.termedBuf(),
descr ? descr : "");
}
}
}
@@ -153,20 +115,47 @@
void
Notes::dump(StoreEntry *entry, const char *key)
{
typedef Notes::NotesList::iterator AMLI;
for (AMLI m = notes.begin(); m != notes.end(); ++m) {
typedef Note::Values::iterator VLI;
for (VLI v =(*m)->values.begin(); v != (*m)->values.end(); ++v ) {
storeAppendPrintf(entry, "%s " SQUIDSTRINGPH " %s",
key, SQUIDSTRINGPRINT((*m)->key), ConfigParser::QuoteString((*v)->value));
dump_acl_list(entry, (*v)->aclList);
storeAppendPrintf(entry, "\n");
}
}
}
void
Notes::clean()
{
notes.clean();
}
+
+const char *
+NotePairs::find(const char *noteKey) const
+{
+ static String value;
+ if (getByNameIfPresent(noteKey, value))
+ return value.termedBuf();
+
+ return NULL;
+}
+
+const char *
+NotePairs::findFirst(const char *noteKey) const
+{
+ NotePairs::Pos pos = NotePairs::InitPos;
+ while (const NotePairs::Entry *e = getEntry(&pos)) {
+ if (e->name.cmp(noteKey) == 0)
+ e->value.termedBuf();
+ }
+ return NULL;
+}
+
+void
+NotePairs::add(const char *key, const char *note)
+{
+ addEntry(new HttpHeaderEntry(HDR_OTHER, key, note));
+}
=== modified file 'src/Notes.h'
--- src/Notes.h 2012-11-30 11:08:47 +0000
+++ src/Notes.h 2013-03-14 16:30:51 +0000
@@ -32,116 +32,94 @@
explicit Value(const String &aVal) : value(aVal), aclList(NULL) {}
~Value();
};
typedef Vector<Value::Pointer> Values;
explicit Note(const String &aKey): key(aKey) {}
/**
* Adds a value to the note and returns a pointer to the
* related Value object.
*/
Value::Pointer addValue(const String &value);
/**
* Walks through the possible values list of the note and selects
* the first value which matches the given HttpRequest and HttpReply
* or NULL if none matches.
*/
const char *match(HttpRequest *request, HttpReply *reply);
- /**
- * Returns the first value for this key or an empty string.
- */
- const char *firstValue() const { return (values.size()>0&&values[0]->value.defined()?values[0]->value.termedBuf():""); }
-
String key; ///< The note key
Values values; ///< The possible values list for the note
};
class ConfigParser;
/**
* Used to store a notes list.
*/
class Notes
{
public:
typedef Vector<Note::Pointer> NotesList;
typedef NotesList::iterator iterator; ///< iterates over the notes list
typedef NotesList::const_iterator const_iterator; ///< iterates over the notes list
Notes(const char *aDescr, const char **metasBlacklist): descr(aDescr), blacklisted(metasBlacklist) {}
Notes(): descr(NULL), blacklisted(NULL) {}
~Notes() { notes.clean(); }
/**
* Parse a notes line and returns a pointer to the
* parsed Note object.
*/
Note::Pointer parse(ConfigParser &parser);
/**
* Dump the notes list to the given StoreEntry object.
*/
void dump(StoreEntry *entry, const char *name);
void clean(); /// clean the notes list
/// points to the first argument
iterator begin() { return notes.begin(); }
/// points to the end of list
iterator end() { return notes.end(); }
/// return true if the notes list is empty
bool empty() { return notes.empty(); }
- /**
- * Adds a note key and value to the notes list.
- * If the key name already exists in list, add the given value to its set of values.
- */
- void add(const String ¬eKey, const String ¬eValue);
-
- /**
- * Adds a set of notes from another notes list to this set.
- * Creating entries for any new keys needed.
- * If the key name already exists in list, add the given value to its set of values.
- *
- * WARNING:
- * The list entries are all of shared Pointer type. Altering the src object(s) after
- * using this function will update both Notes lists. Likewise, altering this
- * destination NotesList will affect any relevant copies of src still in use.
- */
- void add(const Notes &src);
-
- /**
- * Returns a pointer to an existing Note with given key name or nil.
- */
- Note::Pointer find(const String ¬eKey) const;
-
NotesList notes; ///< The Note::Pointer objects array list
const char *descr; ///< A short description for notes list
const char **blacklisted; ///< Null terminated list of blacklisted note keys
private:
/**
* Adds a note to the notes list and returns a pointer to the
* related Note object. If the note key already exists in list,
* returns a pointer to the existing object.
*/
Note::Pointer add(const String ¬eKey);
};
class NotePairs : public HttpHeader
{
public:
+ typedef HttpHeaderPos Pos;
+ typedef HttpHeaderEntry Entry;
+
NotePairs() : HttpHeader(hoNote) {}
- /// convert a NotesList into a NotesPairs
- /// appending to any existing entries already present
- void append(const Notes::NotesList &src) {
- for (Notes::NotesList::const_iterator m = src.begin(); m != src.end(); ++m)
- for (Note::Values::iterator v =(*m)->values.begin(); v != (*m)->values.end(); ++v)
- putExt((*m)->key.termedBuf(), (*v)->value.termedBuf());
- }
-
- void append(const NotePairs *src) {
- HttpHeader::append(dynamic_cast<const HttpHeader*>(src));
- }
+ /**
+ * Returns the first value for this key or an empty string.
+ */
+ const char *findFirst(const char *noteKey) const;
+ /**
+ * Returns a comma separated list of notes with key 'noteKey'.
+ */
+ const char *find(const char *noteKey) const;
+ /**
+ * Adds the key/note note pair.
+ */
+ void add(const char *key, const char *note);
+
+ static const int InitPos;
};
#endif
=== modified file 'src/auth/digest/UserRequest.cc'
--- src/auth/digest/UserRequest.cc 2013-01-25 16:53:16 +0000
+++ src/auth/digest/UserRequest.cc 2013-03-08 15:51:21 +0000
@@ -290,67 +290,67 @@
// the HA1 will be found in content() for these responses.
if (!oldHelperWarningDone) {
debugs(29, DBG_IMPORTANT, "WARNING: Digest auth helper returned old format HA1 response. It needs to be upgraded.");
oldHelperWarningDone=true;
}
/* allow this because the digest_request pointer is purely local */
Auth::Digest::User *digest_user = dynamic_cast<Auth::Digest::User *>(auth_user_request->user().getRaw());
assert(digest_user != NULL);
CvtBin(reply.other().content(), digest_user->HA1);
digest_user->HA1created = 1;
}
break;
case HelperReply::Okay: {
/* allow this because the digest_request pointer is purely local */
Auth::Digest::User *digest_user = dynamic_cast<Auth::Digest::User *>(auth_user_request->user().getRaw());
assert(digest_user != NULL);
- Note::Pointer ha1Note = reply.notes.find("ha1");
+ const char *ha1Note = reply.notes.findFirst("ha1");
if (ha1Note != NULL) {
- CvtBin(ha1Note->firstValue(), digest_user->HA1);
+ CvtBin(ha1Note, digest_user->HA1);
digest_user->HA1created = 1;
} else {
debugs(29, DBG_IMPORTANT, "ERROR: Digest auth helper did not produce a HA1. Using the wrong helper program? received: " << reply);
}
}
break;
case HelperReply::TT:
debugs(29, DBG_IMPORTANT, "ERROR: Digest auth does not support the result code received. Using the wrong helper program? received: " << reply);
// fall through to next case. Handle this as an ERR response.
case HelperReply::BrokenHelper:
// TODO retry the broken lookup on another helper?
// fall through to next case for now. Handle this as an ERR response silently.
case HelperReply::Error: {
/* allow this because the digest_request pointer is purely local */
Auth::Digest::UserRequest *digest_request = dynamic_cast<Auth::Digest::UserRequest *>(auth_user_request.getRaw());
assert(digest_request);
digest_request->user()->credentials(Auth::Failed);
digest_request->flags.invalid_password = true;
- Note::Pointer msgNote = reply.notes.find("message");
+ const char *msgNote = reply.notes.find("message");
if (msgNote != NULL) {
- digest_request->setDenyMessage(msgNote->firstValue());
+ digest_request->setDenyMessage(msgNote);
} else if (reply.other().hasContent()) {
// old helpers did send ERR result but a bare message string instead of message= key name.
digest_request->setDenyMessage(reply.other().content());
if (!oldHelperWarningDone) {
debugs(29, DBG_IMPORTANT, "WARNING: Digest auth helper returned old format ERR response. It needs to be upgraded.");
oldHelperWarningDone=true;
}
}
}
break;
}
void *cbdata = NULL;
if (cbdataReferenceValidDone(replyData->data, &cbdata))
replyData->handler(cbdata);
delete replyData;
}
=== modified file 'src/auth/negotiate/UserRequest.cc'
--- src/auth/negotiate/UserRequest.cc 2013-02-14 06:31:38 +0000
+++ src/auth/negotiate/UserRequest.cc 2013-03-08 15:49:47 +0000
@@ -251,131 +251,131 @@
assert(lm_request != NULL);
assert(lm_request->waiting);
lm_request->waiting = 0;
safe_free(lm_request->client_blob);
assert(auth_user_request->user() != NULL);
assert(auth_user_request->user()->auth_type == Auth::AUTH_NEGOTIATE);
if (lm_request->authserver == NULL)
lm_request->authserver = reply.whichServer.get(); // XXX: no locking?
else
assert(reply.whichServer == lm_request->authserver);
switch (reply.result) {
case HelperReply::TT:
/* we have been given a blob to send to the client */
safe_free(lm_request->server_blob);
lm_request->request->flags.mustKeepalive = true;
if (lm_request->request->flags.proxyKeepalive) {
- Note::Pointer tokenNote = reply.notes.find("token");
- lm_request->server_blob = xstrdup(tokenNote->firstValue());
+ const char *tokenNote = reply.notes.findFirst("token");
+ lm_request->server_blob = xstrdup(tokenNote);
auth_user_request->user()->credentials(Auth::Handshake);
auth_user_request->denyMessage("Authentication in progress");
- debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << tokenNote->firstValue() << "'");
+ debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << tokenNote << "'");
} else {
auth_user_request->user()->credentials(Auth::Failed);
auth_user_request->denyMessage("Negotiate authentication requires a persistent connection");
}
break;
case HelperReply::Okay: {
- Note::Pointer userNote = reply.notes.find("user");
- Note::Pointer tokenNote = reply.notes.find("token");
+ const char *userNote = reply.notes.findFirst("user");
+ const char *tokenNote = reply.notes.findFirst("token");
if (userNote == NULL || tokenNote == NULL) {
// XXX: handle a success with no username better
/* protocol error */
fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply.other().content());
break;
}
/* we're finished, release the helper */
- auth_user_request->user()->username(userNote->firstValue());
+ auth_user_request->user()->username(userNote);
auth_user_request->denyMessage("Login successful");
safe_free(lm_request->server_blob);
- lm_request->server_blob = xstrdup(tokenNote->firstValue());
+ lm_request->server_blob = xstrdup(tokenNote);
lm_request->releaseAuthServer();
/* connection is authenticated */
debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username());
/* see if this is an existing user with a different proxy_auth
* string */
AuthUserHashPointer *usernamehash = static_cast<AuthUserHashPointer *>(hash_lookup(proxy_auth_username_cache, auth_user_request->user()->username()));
Auth::User::Pointer local_auth_user = lm_request->user();
while (usernamehash && (usernamehash->user()->auth_type != Auth::AUTH_NEGOTIATE ||
strcmp(usernamehash->user()->username(), auth_user_request->user()->username()) != 0))
usernamehash = static_cast<AuthUserHashPointer *>(usernamehash->next);
if (usernamehash) {
/* we can't seamlessly recheck the username due to the
* challenge-response nature of the protocol.
* Just free the temporary auth_user after merging as
* much of it new state into the existing one as possible */
usernamehash->user()->absorb(local_auth_user);
/* from here on we are working with the original cached credentials. */
local_auth_user = usernamehash->user();
auth_user_request->user(local_auth_user);
} else {
/* store user in hash's */
local_auth_user->addToNameCache();
}
/* set these to now because this is either a new login from an
* existing user or a new user */
local_auth_user->expiretime = current_time.tv_sec;
auth_user_request->user()->credentials(Auth::Ok);
debugs(29, 4, HERE << "Successfully validated user via Negotiate. Username '" << auth_user_request->user()->username() << "'");
}
break;
case HelperReply::Error: {
- Note::Pointer messageNote = reply.notes.find("message");
- Note::Pointer tokenNote = reply.notes.find("token");
+ const char *messageNote = reply.notes.find("message");
+ const char *tokenNote = reply.notes.findFirst("token");
/* authentication failure (wrong password, etc.) */
if (messageNote != NULL)
- auth_user_request->denyMessage(messageNote->firstValue());
+ auth_user_request->denyMessage(messageNote);
else
auth_user_request->denyMessage("Negotiate Authentication denied with no reason given");
auth_user_request->user()->credentials(Auth::Failed);
safe_free(lm_request->server_blob);
if (tokenNote != NULL)
- lm_request->server_blob = xstrdup(tokenNote->firstValue());
+ lm_request->server_blob = xstrdup(tokenNote);
lm_request->releaseAuthServer();
debugs(29, 4, HERE << "Failed validating user via Negotiate. Error returned '" << reply << "'");
}
break;
case HelperReply::Unknown:
debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication Helper '" << reply.whichServer << "' crashed!.");
/* continue to the next case */
case HelperReply::BrokenHelper: {
/* TODO kick off a refresh process. This can occur after a YR or after
* a KK. If after a YR release the helper and resubmit the request via
* Authenticate Negotiate start.
* If after a KK deny the user's request w/ 407 and mark the helper as
* Needing YR. */
- Note::Pointer errNote = reply.notes.find("message");
+ const char *errNote = reply.notes.find("message");
if (reply.result == HelperReply::Unknown)
auth_user_request->denyMessage("Internal Error");
else if (errNote != NULL)
- auth_user_request->denyMessage(errNote->firstValue());
+ auth_user_request->denyMessage(errNote);
else
auth_user_request->denyMessage("Negotiate Authentication failed with no reason given");
auth_user_request->user()->credentials(Auth::Failed);
safe_free(lm_request->server_blob);
lm_request->releaseAuthServer();
debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication validating user. Error returned " << reply);
} // break;
}
if (lm_request->request) {
HTTPMSGUNLOCK(lm_request->request);
lm_request->request = NULL;
}
r->handler(r->data);
delete r;
}
void
Auth::Negotiate::UserRequest::addAuthenticationInfoHeader(HttpReply * rep, int accel)
{
=== modified file 'src/auth/ntlm/UserRequest.cc'
--- src/auth/ntlm/UserRequest.cc 2013-01-28 16:56:05 +0000
+++ src/auth/ntlm/UserRequest.cc 2013-03-08 15:45:40 +0000
@@ -244,116 +244,116 @@
assert(lm_request != NULL);
assert(lm_request->waiting);
lm_request->waiting = 0;
safe_free(lm_request->client_blob);
assert(auth_user_request->user() != NULL);
assert(auth_user_request->user()->auth_type == Auth::AUTH_NTLM);
if (lm_request->authserver == NULL)
lm_request->authserver = reply.whichServer.get(); // XXX: no locking?
else
assert(reply.whichServer == lm_request->authserver);
switch (reply.result) {
case HelperReply::TT:
/* we have been given a blob to send to the client */
safe_free(lm_request->server_blob);
lm_request->request->flags.mustKeepalive = true;
if (lm_request->request->flags.proxyKeepalive) {
- Note::Pointer serverBlob = reply.notes.find("token");
- lm_request->server_blob = xstrdup(serverBlob->firstValue());
+ const char *serverBlob = reply.notes.findFirst("token");
+ lm_request->server_blob = xstrdup(serverBlob);
auth_user_request->user()->credentials(Auth::Handshake);
auth_user_request->denyMessage("Authentication in progress");
- debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << serverBlob->firstValue() << "'");
+ debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << serverBlob << "'");
} else {
auth_user_request->user()->credentials(Auth::Failed);
auth_user_request->denyMessage("NTLM authentication requires a persistent connection");
}
break;
case HelperReply::Okay: {
/* we're finished, release the helper */
- Note::Pointer userLabel = reply.notes.find("user");
- auth_user_request->user()->username(userLabel->firstValue());
+ const char *userLabel = reply.notes.findFirst("user");
+ auth_user_request->user()->username(userLabel);
auth_user_request->denyMessage("Login successful");
safe_free(lm_request->server_blob);
lm_request->releaseAuthServer();
- debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << userLabel->firstValue() << "'");
+ debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << userLabel << "'");
/* connection is authenticated */
debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username());
/* see if this is an existing user with a different proxy_auth
* string */
AuthUserHashPointer *usernamehash = static_cast<AuthUserHashPointer *>(hash_lookup(proxy_auth_username_cache, auth_user_request->user()->username()));
Auth::User::Pointer local_auth_user = lm_request->user();
while (usernamehash && (usernamehash->user()->auth_type != Auth::AUTH_NTLM ||
strcmp(usernamehash->user()->username(), auth_user_request->user()->username()) != 0))
usernamehash = static_cast<AuthUserHashPointer *>(usernamehash->next);
if (usernamehash) {
/* we can't seamlessly recheck the username due to the
* challenge-response nature of the protocol.
* Just free the temporary auth_user after merging as
* much of it new state into the existing one as possible */
usernamehash->user()->absorb(local_auth_user);
/* from here on we are working with the original cached credentials. */
local_auth_user = usernamehash->user();
auth_user_request->user(local_auth_user);
} else {
/* store user in hash's */
local_auth_user->addToNameCache();
}
/* set these to now because this is either a new login from an
* existing user or a new user */
local_auth_user->expiretime = current_time.tv_sec;
auth_user_request->user()->credentials(Auth::Ok);
debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << auth_user_request->user()->username() << "'");
}
break;
case HelperReply::Error: {
/* authentication failure (wrong password, etc.) */
- Note::Pointer errNote = reply.notes.find("message");
+ const char *errNote = reply.notes.find("message");
if (errNote != NULL)
- auth_user_request->denyMessage(errNote->firstValue());
+ auth_user_request->denyMessage(errNote);
else
auth_user_request->denyMessage("NTLM Authentication denied with no reason given");
auth_user_request->user()->credentials(Auth::Failed);
safe_free(lm_request->server_blob);
lm_request->releaseAuthServer();
- debugs(29, 4, HERE << "Failed validating user via NTLM. Error returned '" << errNote->firstValue() << "'");
+ debugs(29, 4, HERE << "Failed validating user via NTLM. Error returned '" << errNote << "'");
}
break;
case HelperReply::Unknown:
debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication Helper '" << reply.whichServer << "' crashed!.");
/* continue to the next case */
case HelperReply::BrokenHelper: {
/* TODO kick off a refresh process. This can occur after a YR or after
* a KK. If after a YR release the helper and resubmit the request via
* Authenticate NTLM start.
* If after a KK deny the user's request w/ 407 and mark the helper as
* Needing YR. */
- Note::Pointer errNote = reply.notes.find("message");
+ const char *errNote = reply.notes.find("message");
if (reply.result == HelperReply::Unknown)
auth_user_request->denyMessage("Internal Error");
else if (errNote != NULL)
- auth_user_request->denyMessage(errNote->firstValue());
+ auth_user_request->denyMessage(errNote);
else
auth_user_request->denyMessage("NTLM Authentication failed with no reason given");
auth_user_request->user()->credentials(Auth::Failed);
safe_free(lm_request->server_blob);
lm_request->releaseAuthServer();
debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication validating user. Error returned '" << reply << "'");
}
break;
}
if (lm_request->request) {
HTTPMSGUNLOCK(lm_request->request);
lm_request->request = NULL;
}
r->handler(r->data);
delete r;
}
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2013-02-12 11:34:35 +0000
+++ src/client_side.cc 2013-03-08 16:18:46 +0000
@@ -599,41 +599,41 @@
ah->lastMeta.packInto(&p);
aLogEntry->adapt.last_meta = xstrdup(mb.buf);
aLogEntry->notes.append(&ah->metaHeaders);
}
#endif
packerClean(&p);
mb.clean();
}
#if ICAP_CLIENT
const Adaptation::Icap::History::Pointer ih = request->icapHistory();
if (ih != NULL)
aLogEntry->icap.processingTime = ih->processingTime();
#endif
aLogEntry->http.method = request->method;
aLogEntry->http.version = request->http_ver;
aLogEntry->hier = request->hier;
if (request->helperNotes)
- aLogEntry->notes.append(request->helperNotes->notes);
+ aLogEntry->notes.append(request->helperNotes);
if (request->content_length > 0) // negative when no body or unknown length
aLogEntry->cache.requestSize += request->content_length;
aLogEntry->cache.extuser = request->extacl_user.termedBuf();
#if USE_AUTH
if (request->auth_user_request != NULL) {
if (request->auth_user_request->username())
aLogEntry->cache.authuser = xstrdup(request->auth_user_request->username());
}
#endif
// Adapted request, if any, inherits and then collects all the stats, but
// the virgin request gets logged instead; copy the stats to log them.
// TODO: avoid losses by keeping these stats in a shared history object?
if (aLogEntry->request) {
aLogEntry->request->dnsWait = request->dnsWait;
aLogEntry->request->errType = request->errType;
aLogEntry->request->errDetail = request->errDetail;
}
}
=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc 2013-02-12 11:34:35 +0000
+++ src/client_side_request.cc 2013-03-08 16:26:59 +0000
@@ -1244,194 +1244,192 @@
ClientRequestContext *calloutContext = (ClientRequestContext *)data;
if (!calloutContext->httpStateIsValid())
return;
calloutContext->clientStoreIdDone(result);
}
void
ClientRequestContext::clientRedirectDone(const HelperReply &reply)
{
HttpRequest *old_request = http->request;
debugs(85, 5, HERE << "'" << http->uri << "' result=" << reply);
assert(redirect_state == REDIRECT_PENDING);
redirect_state = REDIRECT_DONE;
// copy the URL rewriter response Notes to the HTTP request for logging
// do it early to ensure that no matter what the outcome the notes are present.
// TODO put them straight into the transaction state record (ALE?) eventually
if (!old_request->helperNotes)
- old_request->helperNotes = new Notes;
- old_request->helperNotes->add(reply.notes);
+ old_request->helperNotes = new NotePairs;
+ old_request->helperNotes->append(&reply.notes);
switch (reply.result) {
case HelperReply::Unknown:
case HelperReply::TT:
// Handler in redirect.cc should have already mapped Unknown
// IF it contained valid entry for the old URL-rewrite helper protocol
debugs(85, DBG_IMPORTANT, "ERROR: URL rewrite helper returned invalid result code. Wrong helper? " << reply);
break;
case HelperReply::BrokenHelper:
debugs(85, DBG_IMPORTANT, "ERROR: URL rewrite helper: " << reply << ", attempt #" << (redirect_fail_count+1) << " of 2");
if (redirect_fail_count < 2) { // XXX: make this configurable ?
++redirect_fail_count;
// reset state flag to try redirector again from scratch.
redirect_done = false;
}
break;
case HelperReply::Error:
// no change to be done.
break;
case HelperReply::Okay: {
// #1: redirect with a specific status code OK status=NNN url="..."
// #2: redirect with a default status code OK url="..."
// #3: re-write the URL OK rewrite-url="..."
- Note::Pointer statusNote = reply.notes.find("status");
- Note::Pointer urlNote = reply.notes.find("url");
+ const char *statusNote = reply.notes.findFirst("status");
+ const char *urlNote = reply.notes.findFirst("url");
if (urlNote != NULL) {
// HTTP protocol redirect to be done.
// TODO: change default redirect status for appropriate requests
// Squid defaults to 302 status for now for better compatibility with old clients.
// HTTP/1.0 client should get 302 (HTTP_MOVED_TEMPORARILY)
// HTTP/1.1 client contacting reverse-proxy should get 307 (HTTP_TEMPORARY_REDIRECT)
// HTTP/1.1 client being diverted by forward-proxy should get 303 (HTTP_SEE_OTHER)
http_status status = HTTP_MOVED_TEMPORARILY;
- if (statusNote != NULL) {
- const char * result = statusNote->firstValue();
- status = (http_status) atoi(result);
- }
+ if (statusNote != NULL)
+ status = (http_status) atoi(statusNote);
if (status == HTTP_MOVED_PERMANENTLY
|| status == HTTP_MOVED_TEMPORARILY
|| status == HTTP_SEE_OTHER
|| status == HTTP_PERMANENT_REDIRECT
|| status == HTTP_TEMPORARY_REDIRECT) {
http->redirect.status = status;
- http->redirect.location = xstrdup(urlNote->firstValue());
+ http->redirect.location = xstrdup(urlNote);
// TODO: validate the URL produced here is RFC 2616 compliant absolute URI
} else {
- debugs(85, DBG_CRITICAL, "ERROR: URL-rewrite produces invalid " << status << " redirect Location: " << urlNote->firstValue());
+ debugs(85, DBG_CRITICAL, "ERROR: URL-rewrite produces invalid " << status << " redirect Location: " << urlNote);
}
} else {
// URL-rewrite wanted. Ew.
- urlNote = reply.notes.find("rewrite-url");
+ urlNote = reply.notes.findFirst("rewrite-url");
// prevent broken helpers causing too much damage. If old URL == new URL skip the re-write.
- if (urlNote != NULL && strcmp(urlNote->firstValue(), http->uri)) {
+ if (urlNote != NULL && strcmp(urlNote, http->uri)) {
// XXX: validate the URL properly *without* generating a whole new request object right here.
// XXX: the clone() should be done only AFTER we know the new URL is valid.
HttpRequest *new_request = old_request->clone();
- if (urlParse(old_request->method, const_cast<char*>(urlNote->firstValue()), new_request)) {
+ if (urlParse(old_request->method, const_cast<char*>(urlNote), new_request)) {
debugs(61,2, HERE << "URL-rewriter diverts URL from " << urlCanonical(old_request) << " to " << urlCanonical(new_request));
// update the new request to flag the re-writing was done on it
new_request->flags.redirected = true;
// unlink bodypipe from the old request. Not needed there any longer.
if (old_request->body_pipe != NULL) {
old_request->body_pipe = NULL;
debugs(61,2, HERE << "URL-rewriter diverts body_pipe " << new_request->body_pipe <<
" from request " << old_request << " to " << new_request);
}
// update the current working ClientHttpRequest fields
safe_free(http->uri);
http->uri = xstrdup(urlCanonical(new_request));
HTTPMSGUNLOCK(old_request);
http->request = new_request;
HTTPMSGLOCK(http->request);
} else {
debugs(85, DBG_CRITICAL, "ERROR: URL-rewrite produces invalid request: " <<
- old_request->method << " " << urlNote->firstValue() << " " << old_request->http_ver);
+ old_request->method << " " << urlNote << " " << old_request->http_ver);
delete new_request;
}
}
}
}
break;
}
/* FIXME PIPELINE: This is innacurate during pipelining */
if (http->getConn() != NULL && Comm::IsConnOpen(http->getConn()->clientConnection))
fd_note(http->getConn()->clientConnection->fd, http->uri);
assert(http->uri);
http->doCallouts();
}
/**
* This method handles the different replies from StoreID helper.
*/
void
ClientRequestContext::clientStoreIdDone(const HelperReply &reply)
{
HttpRequest *old_request = http->request;
debugs(85, 5, "'" << http->uri << "' result=" << reply);
assert(store_id_state == REDIRECT_PENDING);
store_id_state = REDIRECT_DONE;
// copy the helper response Notes to the HTTP request for logging
// do it early to ensure that no matter what the outcome the notes are present.
// TODO put them straight into the transaction state record (ALE?) eventually
if (!old_request->helperNotes)
- old_request->helperNotes = new Notes;
- old_request->helperNotes->add(reply.notes);
+ old_request->helperNotes = new NotePairs;
+ old_request->helperNotes->append(&reply.notes);
switch (reply.result) {
case HelperReply::Unknown:
case HelperReply::TT:
// Handler in redirect.cc should have already mapped Unknown
// IF it contained valid entry for the old helper protocol
debugs(85, DBG_IMPORTANT, "ERROR: storeID helper returned invalid result code. Wrong helper? " << reply);
break;
case HelperReply::BrokenHelper:
debugs(85, DBG_IMPORTANT, "ERROR: storeID helper: " << reply << ", attempt #" << (store_id_fail_count+1) << " of 2");
if (store_id_fail_count < 2) { // XXX: make this configurable ?
++store_id_fail_count;
// reset state flag to try StoreID again from scratch.
store_id_done = false;
}
break;
case HelperReply::Error:
// no change to be done.
break;
case HelperReply::Okay: {
- Note::Pointer urlNote = reply.notes.find("store-id");
+ const char *urlNote = reply.notes.findFirst("store-id");
// prevent broken helpers causing too much damage. If old URL == new URL skip the re-write.
- if (urlNote != NULL && strcmp(urlNote->firstValue(), http->uri) ) {
+ if (urlNote != NULL && strcmp(urlNote, http->uri) ) {
// Debug section required for some very specific cases.
- debugs(85, 9, "Setting storeID with: " << urlNote->firstValue() );
- http->request->store_id = urlNote->firstValue();
- http->store_id = urlNote->firstValue();
+ debugs(85, 9, "Setting storeID with: " << urlNote );
+ http->request->store_id = urlNote;
+ http->store_id = urlNote;
}
}
break;
}
http->doCallouts();
}
/** Test cache allow/deny configuration
* Sets flags.cachable=1 if caching is not denied.
*/
void
ClientRequestContext::checkNoCache()
{
if (Config.accessList.noCache) {
acl_checklist = clientAclChecklistCreate(Config.accessList.noCache, http);
acl_checklist->nonBlockingCheck(checkNoCacheDoneWrapper, this);
} else {
/* unless otherwise specified, we try to cache. */
checkNoCacheDone(ACCESS_ALLOWED);
=== modified file 'src/external_acl.cc'
--- src/external_acl.cc 2013-01-27 17:35:07 +0000
+++ src/external_acl.cc 2013-03-08 16:33:26 +0000
@@ -1311,60 +1311,60 @@
* value needs to be URL-encoded or enclosed in double quotes (")
* with \-escaping on any whitespace, quotes, or slashes (\).
*/
static void
externalAclHandleReply(void *data, const HelperReply &reply)
{
externalAclState *state = static_cast<externalAclState *>(data);
externalAclState *next;
ExternalACLEntryData entryData;
entryData.result = ACCESS_DENIED;
external_acl_entry *entry = NULL;
debugs(82, 2, HERE << "reply=" << reply);
if (reply.result == HelperReply::Okay)
entryData.result = ACCESS_ALLOWED;
// XXX: handle other non-DENIED results better
// XXX: make entryData store a proper HelperReply object instead of copying.
- Note::Pointer label = reply.notes.find("tag");
- if (label != NULL && label->values[0]->value.size() > 0)
- entryData.tag = label->values[0]->value;
-
- label = reply.notes.find("message");
- if (label != NULL && label->values[0]->value.size() > 0)
- entryData.message = label->values[0]->value;
-
- label = reply.notes.find("log");
- if (label != NULL && label->values[0]->value.size() > 0)
- entryData.log = label->values[0]->value;
+ const char *label = reply.notes.findFirst("tag");
+ if (label != NULL && *label != '\0')
+ entryData.tag = label;
+
+ label = reply.notes.findFirst("message");
+ if (label != NULL && *label != '\0')
+ entryData.message = label;
+
+ label = reply.notes.findFirst("log");
+ if (label != NULL && *label != '\0')
+ entryData.log = label;
#if USE_AUTH
- label = reply.notes.find("user");
- if (label != NULL && label->values[0]->value.size() > 0)
- entryData.user = label->values[0]->value;
-
- label = reply.notes.find("password");
- if (label != NULL && label->values[0]->value.size() > 0)
- entryData.password = label->values[0]->value;
+ label = reply.notes.findFirst("user");
+ if (label != NULL && *label != '\0')
+ entryData.user = label;
+
+ label = reply.notes.findFirst("password");
+ if (label != NULL && *label != '\0')
+ entryData.password = label;
#endif
dlinkDelete(&state->list, &state->def->queue);
if (cbdataReferenceValid(state->def)) {
// only cache OK and ERR results.
if (reply.result == HelperReply::Okay || reply.result == HelperReply::Error)
entry = external_acl_cache_add(state->def, state->key, entryData);
else {
external_acl_entry *oldentry = (external_acl_entry *)hash_lookup(state->def->cache, state->key);
if (oldentry)
external_acl_cache_delete(state->def, oldentry);
}
}
do {
void *cbdata;
cbdataReferenceDone(state->def);
=== modified file 'src/format/Format.cc'
--- src/format/Format.cc 2013-02-11 23:11:12 +0000
+++ src/format/Format.cc 2013-03-14 16:20:56 +0000
@@ -1029,42 +1029,42 @@
out = tmp;
}
}
break;
case LFT_SSL_USER_CERT_ISSUER:
if (X509 *cert = al->cache.sslClientCert.get()) {
if (X509_NAME *issuer = X509_get_issuer_name(cert)) {
X509_NAME_oneline(issuer, tmp, sizeof(tmp));
out = tmp;
}
}
break;
#endif
case LFT_NOTE:
if (fmt->data.string) {
sb = al->notes.getByName(fmt->data.string);
out = sb.termedBuf();
quote = 1;
} else {
- HttpHeaderPos pos = HttpHeaderInitPos;
- while (const HttpHeaderEntry *e = al->notes.getEntry(&pos)) {
+ NotePairs::Pos pos = NotePairs::InitPos;
+ while (const NotePairs::Entry *e = al->notes.getEntry(&pos)) {
sb.append(e->name);
sb.append(": ");
sb.append(e->value);
sb.append("\r\n");
}
out = sb.termedBuf();
quote = 1;
}
break;
case LFT_PERCENT:
out = "%";
break;
}
if (dooff) {
snprintf(tmp, sizeof(tmp), "%0*" PRId64, fmt->zero && fmt->widthMin >= 0 ? fmt->widthMin : 0, outoff);
out = tmp;
