HI,
the low-hanging fruits from Coverity's analysis have been picked, now
working on somewhat more complex fixes.
The attached patch takes a hint from two benign coverity defects to:
- rework the berEncodeLoginData error handling path to be more readable
- refactor Digest auth's field lookup table to use a std::map instead of
abusing httpHeaderFieldsInfo; this improves readability and size of the
code, and has the added small bonus of lowering the lookup cost from linear
to logarithmic
the Digest code has been run-tested. I'd like feedback on its style, as
httpHeaderFieldsInfo is abused similarly elswehere and I'm considering to
apply it elsewhere as well; it can then be further refined to get O(1) via
a carefully-chosen hash (via std::unsorted_map)
--
Francesco
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: [email protected]
# target_branch: ../trunk
# testament_sha1: 175d89a001b38e47b5dd9f87b9ff22ca85d48573
# timestamp: 2015-07-26 18:42:47 +0200
# base_revision_id: [email protected]\
# cwsj97v5bn5nmc1y
#
# Begin patch
=== modified file 'helpers/digest_auth/eDirectory/edir_ldapext.cc'
--- helpers/digest_auth/eDirectory/edir_ldapext.cc 2015-01-13 07:25:36 +0000
+++ helpers/digest_auth/eDirectory/edir_ldapext.cc 2015-07-23 12:20:00 +0000
@@ -155,10 +155,9 @@
size_t putDataLen,
void *putData)
{
- int err = 0;
+ int presult = 0;
BerElement *requestBer = NULL;
- unsigned int i;
unsigned int elemCnt = methodIDLen / sizeof(unsigned int);
char *utf8ObjPtr=NULL;
@@ -174,45 +173,59 @@
utf8TagSize = strlen(utf8TagPtr)+1;
/* Allocate a BerElement for the request parameters. */
- if ((requestBer = ber_alloc()) == NULL) {
- err = LDAP_ENCODING_ERROR;
- return err;
- }
+ requestBer = ber_alloc();
+ if (requestBer == NULL)
+ return LDAP_ENCODING_ERROR;
/* BER encode the NMAS Version and the objectDN */
- err = (ber_printf(requestBer, "{io", NMAS_LDAP_EXT_VERSION, utf8ObjPtr, utf8ObjSize) < 0) ? LDAP_ENCODING_ERROR : 0;
+ presult = ber_printf(requestBer, "{io", NMAS_LDAP_EXT_VERSION, utf8ObjPtr, utf8ObjSize);
+ if (presult < 0) {
+ ber_free(requestBer, 1);
+ return LDAP_ENCODING_ERROR;
+ }
/* BER encode the MethodID Length and value */
- if (!err) {
- err = (ber_printf(requestBer, "{i{", methodIDLen) < 0) ? LDAP_ENCODING_ERROR : 0;
- }
-
- for (i = 0; !err && i < elemCnt; ++i) {
- err = (ber_printf(requestBer, "i", methodID[i]) < 0) ? LDAP_ENCODING_ERROR : 0;
- }
-
- if (!err) {
- err = (ber_printf(requestBer, "}}", 0) < 0) ? LDAP_ENCODING_ERROR : 0;
+ presult = ber_printf(requestBer, "{i{", methodIDLen);
+ if (presult < 0) {
+ ber_free(requestBer, 1);
+ return LDAP_ENCODING_ERROR;
+ }
+
+ for (unsigned int i = 0; i < elemCnt; ++i) {
+ presult = ber_printf(requestBer, "i", methodID[i]);
+ if (presult < 0) {
+ ber_free(requestBer, 1);
+ return LDAP_ENCODING_ERROR;
+ }
+ }
+
+ presult = ber_printf(requestBer, "}}", 0);
+ if (presult < 0) {
+ ber_free(requestBer, 1);
+ return LDAP_ENCODING_ERROR;
}
if (putData) {
/* BER Encode the the tag and data */
- err = (ber_printf(requestBer, "oio}", utf8TagPtr, utf8TagSize, putDataLen, putData, putDataLen) < 0) ? LDAP_ENCODING_ERROR : 0;
+ presult = ber_printf(requestBer, "oio}", utf8TagPtr, utf8TagSize, putDataLen, putData, putDataLen);
} else {
/* BER Encode the the tag */
- err = (ber_printf(requestBer, "o}", utf8TagPtr, utf8TagSize) < 0) ? LDAP_ENCODING_ERROR : 0;
+ presult = ber_printf(requestBer, "o}", utf8TagPtr, utf8TagSize);
+ }
+ if (presult < 0) {
+ ber_free(requestBer, 1);
+ return LDAP_ENCODING_ERROR;
}
/* Convert the BER we just built to a berval that we'll send with the extended request. */
- if (!err && (ber_tag_t)ber_flatten(requestBer, requestBV) == LBER_ERROR) {
- err = LDAP_ENCODING_ERROR;
- }
-
- if (requestBer) {
+ if (static_cast<ber_tag_t>(ber_flatten(requestBer, requestBV)) == LBER_ERROR) {
ber_free(requestBer, 1);
+ return LDAP_ENCODING_ERROR;
}
- return err;
+ ber_free(requestBer, 1);
+
+ return 0; /* no error */
}
/**********************************************************************
=== modified file 'src/auth/digest/Config.cc'
--- src/auth/digest/Config.cc 2015-06-05 05:56:36 +0000
+++ src/auth/digest/Config.cc 2015-07-23 20:04:46 +0000
@@ -34,6 +34,8 @@
#include "StrList.h"
#include "wordlist.h"
+#include <map>
+
/* digest_nonce_h still uses explicit alloc()/freeOne() MemPool calls.
* XXX: convert to MEMPROXY_CLASS() API
*/
@@ -63,20 +65,50 @@
DIGEST_ENUM_END
};
-static const HttpHeaderFieldAttrs DigestAttrs[DIGEST_ENUM_END] = {
- HttpHeaderFieldAttrs("username", (http_hdr_type)DIGEST_USERNAME),
- HttpHeaderFieldAttrs("realm", (http_hdr_type)DIGEST_REALM),
- HttpHeaderFieldAttrs("qop", (http_hdr_type)DIGEST_QOP),
- HttpHeaderFieldAttrs("algorithm", (http_hdr_type)DIGEST_ALGORITHM),
- HttpHeaderFieldAttrs("uri", (http_hdr_type)DIGEST_URI),
- HttpHeaderFieldAttrs("nonce", (http_hdr_type)DIGEST_NONCE),
- HttpHeaderFieldAttrs("nc", (http_hdr_type)DIGEST_NC),
- HttpHeaderFieldAttrs("cnonce", (http_hdr_type)DIGEST_CNONCE),
- HttpHeaderFieldAttrs("response", (http_hdr_type)DIGEST_RESPONSE),
-};
-
-class HttpHeaderFieldInfo;
-static HttpHeaderFieldInfo *DigestFieldsInfo = NULL;
+struct HttpDigestFieldAttrs {
+ const char *name;
+ http_digest_attr_type id;
+};
+
+static const HttpDigestFieldAttrs DigestAttrs[] = {
+ {"username", DIGEST_USERNAME},
+ {"realm", DIGEST_REALM},
+ {"qop", DIGEST_QOP},
+ {"algorithm", DIGEST_ALGORITHM},
+ {"uri", DIGEST_URI},
+ {"nonce", DIGEST_NONCE},
+ {"nc", DIGEST_NC},
+ {"cnonce", DIGEST_CNONCE},
+ {"response", DIGEST_RESPONSE}
+};
+
+/* a SBuf -> http_digest_attr_type lookup table.
+ * Implementaiton can be improved by using an unordered_map with custom hasher
+ * but the focus here is API correctness.
+ */
+class DigestFieldsLookupTable_t {
+public:
+ DigestFieldsLookupTable_t();
+ http_digest_attr_type lookup(const SBuf &key) const;
+private:
+ /* could be unordered_map but let's skip the requirement to hash for now */
+ typedef std::map<const SBuf, http_digest_attr_type> lookupTable_t;
+ lookupTable_t lookupTable;
+};
+DigestFieldsLookupTable_t::DigestFieldsLookupTable_t() {
+ for (auto i : DigestAttrs) {
+ lookupTable[SBuf(i.name)] = i.id;
+ }
+}
+inline http_digest_attr_type
+DigestFieldsLookupTable_t::lookup(const SBuf &key) const
+{
+ auto r = lookupTable.find(key);
+ if (r == lookupTable.end())
+ return DIGEST_ENUM_END; // original returns HDR_BAD_HDR(-1)
+ return r->second;
+}
+DigestFieldsLookupTable_t DigestFieldsLookupTable;
/*
*
@@ -545,7 +577,6 @@
Auth::Digest::Config::init(Auth::Config *)
{
if (authenticateProgram) {
- DigestFieldsInfo = httpHeaderBuildFieldsInfo(DigestAttrs, DIGEST_ENUM_END);
authenticateDigestNonceSetup();
authdigest_initialised = 1;
@@ -581,11 +612,6 @@
if (digestauthenticators)
helperShutdown(digestauthenticators);
- if (DigestFieldsInfo) {
- httpHeaderDestroyFieldsInfo(DigestFieldsInfo, DIGEST_ENUM_END);
- DigestFieldsInfo = NULL;
- }
-
if (!shutting_down)
return;
@@ -815,7 +841,7 @@
}
/* find type */
- http_digest_attr_type t = (http_digest_attr_type)httpHeaderIdByName(item, nlen, DigestFieldsInfo, DIGEST_ENUM_END);
+ const http_digest_attr_type t = DigestFieldsLookupTable.lookup(keyName);
switch (t) {
case DIGEST_USERNAME:
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWfyKnjgAClZfgERQWf///3/n
/0q////6YBB9WR1Zq6+Jet5u9m4Aha0M8nnO41zQ273h48+xS+ypHXvau9gemvCSRAEnpiZNNNGm
mSpvVPUGmQA2p6mTJ6j1Bp6gCSgAEJtRJtJhU9qZhSeETQNqAaPUAPUZAyJkag0U2SnqfqE0Gmmj
RoAAAANGgAJERCYkZMT0oelPyp6nlNPTU8k9QPUPU0DR6QAGgiomlDRkAepp6TTRoNBoAAAAAAAi
kTQCGIhNoRo08iJ5JqeFPKbU9R6I0Bo9TR6lBLyMADfr5vaMivNlYFWmWNe17c9WTtsgofpd/pOe
48SBmb9zOmnktJXOeQw9DICymp2bTNZCoWCQfRnrc+GWAF83XsJRfhMzGMzjaVJao8v4d2jPaI4v
PZswyzONsfKyCzAqxBZl9m4YBxxxcgGymaS1rWVnKdBOpSzCTpsHaUcFaC03fMLvhf5urya3cIQl
YJIKG5i9hux2mFBSyYtI/2IMCoznD2Bxg0DYhtDGB1ox/nsbP2QHFbquIUjahpzaLzVNtsfJcoBp
vzeWVu2VktZOaJVfLSKaqqHmoE4hHsVJlAtj0HOo5kyPECVQgFIDrDI8W2RdFZUb8zxRAG+IPGM+
2BTDVkE/2/OLWGmigHoDgoNwwoQz7bKTRWHIEDjl1cohHoTxUZ3DTbuGfsj7f975wjWEPEViPuzP
PLz4Ol4jjk8kdNqsHRTFEbSR0+IZr209roBIoxac8On85CPLbao6P5nt7AkBZD2HhXPlnFa+a/PE
MC99Gvq6Q6ucKedUE9Jw927zpk98hpYYbjhp5nX8GyMpGJGnRWAdVGg1rUvWxEy3JENtttwu1mRt
EPe9PjWnTZqpR0xK8KEMBE8CAThYvMIuPs2Pqg2FD+IbtbO9EH7eGnr8/ucM3bs1bNk4Z6TppGp9
zuQ+hmbrGxpeoyA6AH1nGJsYQLOmomTNnolX+cUIJ6Ia1uz+w+8OwfO8sj30hQYYDvvLBPAo8hr4
m8VedxHkHWONSHeODWMCcWqiG7gcmWZFteBowpadXCJES8AmJJdy4dZ9W0Wv0jNfLyt3xERERERG
jWGrveWAH0MMngwd0KGrmUaQZSggUoIIiLoENBdCH1hC0hugZ3laE0JWSBWN52XM0LZNkfwEPCeh
/DKZASSh132ID4gitpZbjgIM3tMyg2wsLkfrPLDhixBjdUAXo6BQvruxstG0aRsz+sAwutIAONml
tpGKc0i+sDczVS7SBWSc5Uk1eIrkvixqBhi7kH89P1As94uFMNk7ntxtJUFUVHcEFciLTaaXm1za
6aicxpNIZaZVgBA5DFCxl2j8x2DQtprBXfkJOjkUkVkamA1EBjrjN2mFAlZGQ2qZ7HpgpklzcVlz
AhQ6vRqH6zYCsCUfQ9Uw2mE11cxfkBvCNdinqTDWDBAYSO3bnlKU6BWJZDZRC9DRCGg3oSBWjSsk
UYlGwqZZ2UMUKrGDdB3CW1IpoE4QEZYXTClxpJaTkcUA2525bwXAY1QJDVnbAh6yS050BqA6Sdot
qwfmTI1EVEWMsGze0q7eFLySa5qQTg0RpdqCTb0RgmoD+xKtUEsnc8xDHEay0zXgORhMdR2gwWRm
D92Z8xgqboubDpjZRrU19ScONbsIDUG8hHOSDuQR0Vo7swPu3tXStiboAG+yWIvFMRxuF2F5YCQr
cxS3EPnz5d3BBGqkbMTTfFhGFZlNT70EdoaM1rJWbwKVcIx1WZBPeIIoSuId11G8eQjJ3oM6D1SP
Gn/g1No8S/WHUTSiuQ3JYHsbpwNMtUhEGjIMSEm6NmMESdARmkNAbuodXqU943aNkeiYjJRc0odi
FEARAEQoMqTtuac8KFc4ETmLnvVBBxTfGP+LjdF+FOY2yoTHhOgI4Sy7U0SShEhs2FpAa+h2OJXG
x1hchIDOQCZbbCDqpfOCAzZTFHN0jkK0R8ajeQO1HZsLaz4TSmkVoL2EwrPoDkSNW7ZE0GVBqDOT
jwNKtladjw8Mbcpa6upBu8uXJUUDYfw3G14EdA3KDgQBXC0n5Y05LqOIVbikJjUVRmZzybqSTmbW
mdArADXl6EyTNSLi6c6UnulHsETccidZphINCYUGu6T7hoFgrm7INEBey2eaLkrXYDSPAvLUuMuo
klCLGce1NcVpGV4YJaZ0nrJRc6Rax0XW0BT6BWFUvD0D/RPBiuKx5GwwfPKXFsdoWexisMtKVRia
InmAicYYlglI0KjoxTSgEvSE1LhwthpBjPUtTjtOHLJyBWNC241ybLiQH6ZgZ9InN6hOlz2ZPNiO
AFxqd+xdXMaSsMhjlG8ZJoNYxNQLgtGYlBkAQNgxQTEKQgW3CYuPu+PizeBRjJKwLRQhA3ZLrXwp
bDORKevsCGKUL7RgNjZIrdLE8ybCHD6W224KBb4B94QcC0u6Bn1VIr0B2fY+J8DVuA/dkLqGL3cs
MRBCkQF38KXwr/xb8E/Cwksx2aL+5lMBCEbH/MJeSUXkNH799CjUrNvSTlW3X1PPZc1mhdtSTxgP
bhCeQkQd0UkrUgLsFYJG4tSlq+8Yt0hny1t2nchLV50mhN2GgX5n4MP01Z0/JBPszImFyEO3Bdod
6TWB0JauyxkB0A1+4ZBKWgcj/jADcdpjyZDA8OoppNsAQHLA+QwAaCr5TcnCM7CLKwHHHE0NYp9u
CBmF6Zh2Rh0dMbYZEeuJ7fmHmNsG9fOEgnvWqzSivDgJ8T6DyCVLk0YjgMkCYcSBzsrAoBvop4yr
3x8rp/gXAj6qnYfHaLgo4/OSKeZH7cH1/L7Ppv+u5vMOaA32Erz/LOrXOVuzO4L+cfEgbEJ79Dyn
DEqDtYIhKxV8KmnBfWKoOJVaCXePO/3zqRLQPa4VARViKDuVqq+7z1O0Wm63e/8OQ48x6745R8ZD
HGz4C6Edz7yiOCU/or4chrDO/WHzeL566faMQj+vtVxSNGfwAR5HQHU8XC/Mwd0WsfogvEY8SvAx
gmh49EoClas7chJK3oZyWCNm4mLgfebOthswjE4tuM4BW1m4SSoQM8XCH0Jbj5BWBKQ3iueuhpm7
o+BRCqQWSHJZxcl8IQmbb3OFrtG+8ckkuautFwxyxNkfXlMDY1CP7tQ9frxM1LtWQBFZsKDjNtx1
nFxbIF3K+I5HpnCAXLesh10NGVee87L8a+8R7j2LzAKourbINIuLWr97qHwbJ26oW6sFPQbw4EGg
TO4axHiSwbASSgFdH9vHhSVeMc+BjwKFCKOQgdFptFEstvVNUk8mPRVs/A0nKCtoqZyMbLS2ltoO
C84oSWtXszIjEzkVSJUIBgLC7C5h0+Dxi2Qq8eMERHnmdrQC4Ty9ybiKa6rlW0IpBTBZs26b0Dsr
RTatXYW3igxyxdXCJ9O3lHl/AowXMBcMluaoTufgvTIFihER1CEgDqCSEkC33K6dOA8QClR8y64Y
Nlst48B0HwHWePIe1A7wCBAkNQPLQJk1OcIFM0PmYu4WOKWDgagIPujxRYMcOBqPH1nqn3nuh7PL
u6jxqNwVsM1FecsWZ4aVog0JuUTWE/pnV85iPoZnU8uA/kIbp1r8ofAPWHl9Yvqi/7ExBkZDEoEm
GfPAWs1xLmkyG0lpYKkS8UHe80jiXiUfGdeEjmGJn4sDOoEgzNlRMVa3KD8xssohgF4mxC4yg80W
HJBEe+N+8Fk3jBeeb9CfwvXrPOOpfQh+IX1uEEucbzy9wgFsYk1sWUh1UV/gGoNAUQdQS4wU5hWA
mvqqB9NoOsO0/WmpA29N1lSKz3z14jPqHsGsfrcKd1+acuPtFPlqI6PP6x0ovsDcPgJ+TkFZibNK
Emwb7m7iEEKola6uMxxT8LLFFdBA5heo7ICCBwDIHCGQOEMgcAyI4C0YoezrQejvAfIhWKeG8agH
b1TQrtorsCEPaHOBAQkBAEvB82o9wXjXyW0OI8UMRnnSZdeGx9g18K6oraCYtXvGRgnG0qRLffYl
C9LXHLzGx6OtHldomLCQX2hYPKOZA+gcxoq4Vgj7FYsBtKlkRYEgvImFaECipy35HrgOBaFwK7xw
gQUfpgXooEKh3iOoUcwxJLtWPSaihPO4nCljeh8ISQwIRAB6JCGgccB6Mmkw8tOsm6tc/nlsQauk
cLC0ZYCmIT9Q5vcTU2CamAiISLvqQOTEGCBeZirr8R7CKFa80g4OzvccK+c8j0+gHNqPeDigrJNG
JZoECZdYwEAEFBPoDZaNoSQqfAyv3olyXauovjoFMb0GHphlWAjBBlzXqTxF5gg+WgAoUAgVwrka
sKJEqZIwZ4oFgILMJOgfAe4ZFVeJ5LajyjcE8MBG3omLK48YNXUvbGeyOUMRMIVcEwmkS0TWp6ow
JhjFWQkxGehXWNsFne22EQoaElqm7yQxCMButObzYd+DEjZhSPwQOR8R7xXBUPu0JPF4B0YJ+yT+
9gMbbbZsCcLdIkUDnWnjWljY2m02N8R9hVwTCaxgJJSJEhV3nkuDVYgVV1gz9xvQ0puO0b4GL86z
hAe6HYUkUmziWq6RRqGtkNAtPV8RLGlkGIzkKXsCBUFwFAtCuqgmtYaVjtWwDWNg3DywBUSNRFo+
OUg2DoD9ENLbjXbCBgNIBrXUg7f7MhOYKSRLLyMagDVgP+1fvEmJ+iJopzIQFa0q4Rq6+YRX1bGm
/mAeCTAgMUHC2YW0FIdh1+pBq7zeptkVyv8DNqHnEsQ+X3PimHaJd3GokS0EOxCY9IpxeIliFVn5
BWhsIPkobXiDOmLVhYgJEPI0MVXKciYJqC+RjJBtD2t2XkHQIeUTl1LNCDyOodLkND0wDXnIUk0Q
4+mO7ekZvuku8l9z+MSFRq/4u5IpwoSH5FTxwA==
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
