Build failed in Jenkins: 3.2-matrix » master #242
See http://build.squid-cache.org/job/3.2-matrix/./label=master/242/changes Changes: [Amos Jeffries] Do not double-escape %R on deny_info redirects [Amos Jeffries] Bug 3576: ICY streams being Transfer-Encoding:chunked [Amos Jeffries] Fix build with GCC 4.7 (and probably other C++11 compilers). User-defined literals introduced by C++11 break some previously valid code, resulting in errors when building with GCC v4.7. For example: error: unable to find string literal operator 'operator PRIu64' In particular, whitespace is now needed after a string literal and before something that could be a valid user-defined literal. See User-defined literals and whitespace section at [1] for more details. The patch adds spaces between string literals and macros. [1] http://gcc.gnu.org/gcc-4.7/porting_to.html [Amos Jeffries] Do not report the same job multiple times. [Amos Jeffries] Cleanup: disconnect Authentication and URL-rewrite callback handlers The authentication handlers were for some reason using RH (rewrite helper) callback typedef. But specifying it as a fatal error if the char* parameter was used in auth. Assign a new callback typedef AUTHCB for use by authentication callers. This allows auth callers to use different parameters (none) and to avoid possibly fatal mistakes when coding new auth modules. [Amos Jeffries] Account for Store disk client quota when bandwidth-limiting the server. It is not clear why the store client type matters when MemObject::mostBytesAllowed() is trying to find the maximum delay pool quota for reading from the next hop HTTP server. Whether the client(s) are reading from disk or RAM, the corresponding server-side bandwidth ought to be limited. This code was removed as a part of bug 3462 investigation, but it is not needed to fix bug 3462. -- [...truncated 6895 lines...] Making all in basic make[7]: Entering directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth/basic' make[7]: Nothing to be done for `all'. make[7]: Leaving directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth/basic' Making all in ntlm make[7]: Entering directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth/ntlm' make[7]: Nothing to be done for `all'. make[7]: Leaving directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth/ntlm' Making all in negotiate make[7]: Entering directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth/negotiate' make[7]: Nothing to be done for `all'. make[7]: Leaving directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth/negotiate' Making all in digest make[7]: Entering directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth/digest' make[7]: Nothing to be done for `all'. make[7]: Leaving directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth/digest' make[7]: Entering directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth' make[7]: Nothing to be done for `all-am'. make[7]: Leaving directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth' make[6]: Leaving directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth' make check-TESTS make[6]: Entering directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth' /bin/sh ../../../test-suite/testheaders.sh g++ -DHAVE_CONFIG_H -I../../.. -I../../../include -I../../../lib -I../../../src -I../../include -Wall -Wpointer-arith -Wwrite-strings -Wcomments -Werror -pipe -D_REENTRANT -g -O2 ../../../src/auth || exit 1 Testing ../../../src/auth ...Ok. PASS: testHeaders == All 1 tests passed == make[6]: Leaving directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth' make[5]: Leaving directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth' make[4]: Leaving directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/auth' Making check in ip make[4]: Entering directory `http://build.squid-cache.org/job/3.2-matrix/./label=master/ws/btlayer-00-default/squid-3.2.0.17-BZR/_build/src/ip' make testIpAddress make[5]:
[PATCH] ACL cleanup in 3.2
This is my attempted port of Alexs' ACL cleanups. They require the ACL
extensions changes (what remained in trunk anyway), so teh differences
there between trunk and 3.2 have been included in this patch.
So far they are testing out okay on builds, but there is a bit longer to
go on that.
If there are no objections I plan to release 3.2.0.18 and apply this
change to 3.2 branch afterward.
Amos
=== modified file 'src/ExternalACL.h'
--- src/ExternalACL.h 2009-03-08 19:34:36 +
+++ src/ExternalACL.h 2012-06-28 09:22:30 +
@@ -36,7 +36,8 @@
#include acl/Checklist.h
-class external_acl;
+/** \todo CLEANUP: kill this typedef. */
+typedef struct _external_acl_data external_acl_data;
class ExternalACLLookup : public ACLChecklist::AsyncState
{
@@ -45,14 +46,15 @@
static ExternalACLLookup *Instance();
virtual void checkForAsync(ACLChecklist *)const;
+// If possible, starts an asynchronous lookup of an external ACL.
+// Otherwise, asserts (or bails if background refresh is requested).
+static void Start(ACLChecklist *checklist, external_acl_data *acl, bool
bg);
+
private:
static ExternalACLLookup instance_;
static void LookupDone(void *data, void *result);
};
-/** \todo CLEANUP: kill this typedef. */
-typedef struct _external_acl_data external_acl_data;
-
#include acl/Acl.h
class ACLExternal : public ACL
@@ -61,7 +63,7 @@
public:
MEMPROXY_CLASS(ACLExternal);
-static void ExternalAclLookup(ACLChecklist * ch, ACLExternal *, EAH *
callback, void *callback_data);
+static void ExternalAclLookup(ACLChecklist * ch, ACLExternal *);
ACLExternal(char const *);
=== modified file 'src/ExternalACLEntry.cc'
--- src/ExternalACLEntry.cc 2012-02-05 06:09:46 +
+++ src/ExternalACLEntry.cc 2012-06-28 08:54:59 +
@@ -69,7 +69,7 @@
ExternalACLEntry::ExternalACLEntry()
{
lru.next = lru.prev = NULL;
-result = 0;
+result = ACCESS_DENIED;
date = 0;
def = NULL;
}
=== modified file 'src/ExternalACLEntry.h'
--- src/ExternalACLEntry.h 2011-02-11 12:53:06 +
+++ src/ExternalACLEntry.h 2012-06-28 08:54:59 +
@@ -44,7 +44,7 @@
#ifndef SQUID_EXTERNALACLENTRY_H
#define SQUID_EXTERNALACLENTRY_H
-
+#include acl/Acl.h
#include cbdata.h
/**
@@ -58,9 +58,9 @@
{
public:
-ExternalACLEntryData() : result (-1) {}
+ExternalACLEntryData() : result(ACCESS_DUNNO) {}
-int result;
+allow_t result;
#if USE_AUTH
// TODO use an AuthUser to hold this info
String user;
@@ -89,7 +89,7 @@
void update(ExternalACLEntryData const );
dlink_node lru;
-int result;
+allow_t result;
time_t date;
#if USE_AUTH
String user;
=== modified file 'src/acl/Acl.cc'
--- src/acl/Acl.cc 2012-06-04 10:23:57 +
+++ src/acl/Acl.cc 2012-06-28 09:22:40 +
@@ -317,16 +317,22 @@
ACLList::matches (ACLChecklist *checklist) const
{
assert (_acl);
+// XXX: AclMatchedName does not contain a matched ACL name when the acl
+// does not match (or contains stale name if no ACLs are checked). In
+// either case, we get misleading debugging and possibly incorrect error
+// messages. Unfortunately, deny_info's when none http_access
+// lines match exception essentially requires this mess.
+// TODO: Rework by using an acl-free deny_info for the no-match cases?
AclMatchedName = _acl-name;
debugs(28, 3, ACLList::matches: checking (op ? null_string : !)
_acl-name);
if (_acl-checklistMatches(checklist) != op) {
debugs(28, 4, ACLList::matches: result is false);
-return checklist-lastACLResult(false);
+return false;
}
debugs(28, 4, ACLList::matches: result is true);
-return checklist-lastACLResult(true);
+return true;
}
=== modified file 'src/acl/Acl.h'
--- src/acl/Acl.h 2011-07-21 11:09:47 +
+++ src/acl/Acl.h 2012-06-28 09:10:04 +
@@ -39,6 +39,10 @@
#include cbdata.h
#include dlink.h
+#if HAVE_OSTREAM
+#include ostream
+#endif
+
class ConfigParser;
class ACLChecklist;
@@ -105,12 +109,35 @@
/// \ingroup ACLAPI
typedef enum {
+// Authorization ACL result states
ACCESS_DENIED,
ACCESS_ALLOWED,
ACCESS_DUNNO,
-ACCESS_REQ_PROXY_AUTH
+
+// Authentication ACL result states
+ACCESS_AUTH_REQUIRED,// Missing Credentials
} allow_t;
+inline std::ostream
+operator (std::ostream o, const allow_t a)
+{
+switch (a) {
+case ACCESS_DENIED:
+o DENIED;
+break;
+case ACCESS_ALLOWED:
+o ALLOWED;
+break;
+case ACCESS_DUNNO:
+o DUNNO;
+break;
+case ACCESS_AUTH_REQUIRED:
+o AUTH_REQUIRED;
+break;
+}
+return o;
+}
+
/// \ingroup ACLAPI
class acl_access
{
=== modified file 'src/acl/Checklist.cc'
--- src/acl/Checklist.cc2012-02-05 06:09:46 +
+++
Jenkins build is back to normal : 3.2-matrix » master #243
See http://build.squid-cache.org/job/3.2-matrix/./label=master/243/changes
Geek fun in Squid's source code
from support_netbios.cc:
if (p == np) {
(stumbled into this while sweeping the sources changing postincrement
to preincrement operators).
--
/kinkie
Re: [PATCH] ACL cleanup in 3.2
On 06/28/2012 04:59 AM, Amos Jeffries wrote: This is my attempted port of Alexs' ACL cleanups. They require the ACL extensions changes (what remained in trunk anyway), so teh differences there between trunk and 3.2 have been included in this patch. So far they are testing out okay on builds, but there is a bit longer to go on that. If there are no objections I plan to release 3.2.0.18 and apply this change to 3.2 branch afterward. Hi Amos, Thank you for working on this! My response is not an objection to these changes going into v3.2, but based on initial feedback from folks running the cleaned up ACL code in trunk, there is one red flag that we may need to discuss. First some terminology. Any single ACL may match, mismatch, or fail (in various ways). That result can be negated using !acl syntax in the config. The negation of match is mismatch. The negation of mismatch is match. The negation of fail depends on the code: In old code: a negation of a failed ACL was a match. In current code: a negation of a failed ACL is a failure. This means that given a configuration like this: some_option allow !group the old code allows or applies some_option if the group ACL fails. After our changes, that option will never be allowed if the group ACL fails (other bugs notwithstanding). This change breaks old configurations that relied on negated failing ACLs to match. For example, some admins assumed that if there is no authentication information, the negated authentication-related ACL will match. In some cases, those configurations were just broken because they did not cover all the cases correctly: http_access allow !badGuys but there are apparently correctly-working misconfigurations that my changes break. I do not have enough information to post any meaningful statistics, but I suspect that in many cases the admins were thinking that missing credentials is the only ACL failure that can be negated into a match. However, I believe the code was negating more than that, and possibly any failure (e.g., a crashed authentication server) could be negated. IMO, it is wrong to convert failures into matches by negating them. It is often not logical (this is not Bob and I have no idea who this is are often two very different things!). I also predict that this approach will hinder ACL evolution because adding more operations and/or extending/optimizing existing ACLs will be difficult when failures are not propagated properly. However, even if we agree that failures should not be converted to matches by negation, there may be thousands of configurations out there that use this flawed principle. Yes, all of them can be fixed/corrected, but are we sure that forcing such corrections on users is the best way forward? Thank you, Alex. P.S. Please note that this discussion is only relevant in the negation context (!acl) because behavior is meant to be the same in a positive context.
Re: Geek fun in Squid's source code
On 29/06/2012 12:01 a.m., Kinkie wrote:
from support_netbios.cc:
if (p == np) {
(stumbled into this while sweeping the sources changing postincrement
to preincrement operators).
--
/kinkie
Why do you bring this up?
Set two pointers to the start of a string, increment one until some
delimiter. If they are both still identical the scan produced an empty
string.
-- Saves loading a constant into memory for comparison and
de-referencing one or both of the pointers to match it against.
What I do note about this is that the CR LF skip loop will make the
above logics break and permit binary usernames of CR@foo or
LF@foo through to the backend. I'm not sure if that is valid? or if
there is a missing (++np) operation to actually drop those octets from
the validation.
Amos
Re: Geek fun in Squid's source code
On Fri, Jun 29, 2012 at 12:53 PM, Amos Jeffries squ...@treenet.co.nz wrote:
On 29/06/2012 12:01 a.m., Kinkie wrote:
from support_netbios.cc:
if (p == np) {
(stumbled into this while sweeping the sources changing postincrement
to preincrement operators).
--
/kinkie
Why do you bring this up?
http://en.wikipedia.org/wiki/P_versus_NP_problem
Re: Geek fun in Squid's source code
On 29/06/2012 2:09 p.m., Kinkie wrote:
http://en.wikipedia.org/wiki/P_versus_NP_problem
Ah right.
Then there are the lesser grades of magics ...
src/stmem.cc:for_each (getNodes().begin(), getNodes().end(), foo);
Amos
On Jun 29, 2012 2:53 AM, Amos Jeffries squ...@treenet.co.nz
mailto:squ...@treenet.co.nz wrote:
On 29/06/2012 12:01 a.m., Kinkie wrote:
from support_netbios.cc:
if (p == np) {
(stumbled into this while sweeping the sources changing
postincrement
to preincrement operators).
--
/kinkie
Why do you bring this up?
Set two pointers to the start of a string, increment one until
some delimiter. If they are both still identical the scan produced
an empty string.
-- Saves loading a constant into memory for comparison and
de-referencing one or both of the pointers to match it against.
What I do note about this is that the CR LF skip loop will make
the above logics break and permit binary usernames of CR@foo
or LF@foo through to the backend. I'm not sure if that is
valid? or if there is a missing (++np) operation to actually drop
those octets from the validation.
Amos
