--------------------- PatchSet 10261 Date: 2007/12/16 16:14:22 Author: adri Branch: s27_adri Tag: (none) Log: more groundwork to ditch String-based NUL terminated strings. * rework strwordtok() to take a length parameter; this is untested - leave a note to migrate this into libcore; and then write some unit testing code! * rework external_acl.c code to remove strBuf() Members: ADRIAN_TODO:1.1.2.10->1.1.2.11 libbuf/String.c:1.1.2.9->1.1.2.10 libbuf/String.h:1.1.2.15->1.1.2.16 libcore/tools.h:1.1.2.2->1.1.2.3 src/external_acl.c:1.26->1.26.12.1 src/protos.h:1.146.2.4.4.22->1.146.2.4.4.23 src/tools.c:1.62.2.3.4.3->1.62.2.3.4.4 Index: squid/ADRIAN_TODO =================================================================== RCS file: /cvsroot/squid-sf//squid/Attic/ADRIAN_TODO,v retrieving revision 1.1.2.10 retrieving revision 1.1.2.11 diff -u -r1.1.2.10 -r1.1.2.11 --- squid/ADRIAN_TODO 16 Dec 2007 14:25:57 -0000 1.1.2.10 +++ squid/ADRIAN_TODO 16 Dec 2007 16:14:22 -0000 1.1.2.11 @@ -63,3 +63,6 @@ that are NUL-terminated; please write replacement strotoll() which takes a len parameter; this will avoid the temporary copy which is being done now. +* unit check start time - move strwordtok() out of src and into libcore; + write a unit check for strwordtok() + Index: squid/libbuf/String.c =================================================================== RCS file: /cvsroot/squid-sf//squid/libbuf/Attic/String.c,v retrieving revision 1.1.2.9 retrieving revision 1.1.2.10 diff -u -r1.1.2.9 -r1.1.2.10 --- squid/libbuf/String.c 14 Dec 2007 07:36:15 -0000 1.1.2.9 +++ squid/libbuf/String.c 16 Dec 2007 16:14:22 -0000 1.1.2.10 @@ -1,6 +1,6 @@ /* - * $Id: String.c,v 1.1.2.9 2007/12/14 07:36:15 adri Exp $ + * $Id: String.c,v 1.1.2.10 2007/12/16 16:14:22 adri Exp $ * * DEBUG: section 67 String * AUTHOR: Duane Wessels @@ -37,6 +37,8 @@ #include #include #include +#include +#include #include "../include/config.h" #include "../include/util.h" Index: squid/libbuf/String.h =================================================================== RCS file: /cvsroot/squid-sf//squid/libbuf/Attic/String.h,v retrieving revision 1.1.2.15 retrieving revision 1.1.2.16 diff -u -r1.1.2.15 -r1.1.2.16 --- squid/libbuf/String.h 14 Dec 2007 07:09:37 -0000 1.1.2.15 +++ squid/libbuf/String.h 16 Dec 2007 16:14:22 -0000 1.1.2.16 @@ -157,6 +157,23 @@ stringInit(s, str); } +static int inline +strToOffset(String s, off_t *off, int base) +{ + off_t o; + char *e; + + char buf[32]; + memcpy(buf, strBuf2(s), XMIN(strLen2(s), 31)); + buf[(XMIN(strLen2(s), 31))] = '\0'; + + errno = 0; + o = (off_t) strtoll(buf, &e, base); + if (errno == EINVAL || errno == ERANGE) + return 0; + *off = o; + return 1; +} #endif /* __LIBBUF_STRING__H_ */ Index: squid/libcore/tools.h =================================================================== RCS file: /cvsroot/squid-sf//squid/libcore/Attic/tools.h,v retrieving revision 1.1.2.2 retrieving revision 1.1.2.3 diff -u -r1.1.2.2 -r1.1.2.3 --- squid/libcore/tools.h 6 Dec 2007 06:28:33 -0000 1.1.2.2 +++ squid/libcore/tools.h 16 Dec 2007 16:14:23 -0000 1.1.2.3 @@ -5,7 +5,7 @@ * * Adrian Chadd * - * $Id: tools.h,v 1.1.2.2 2007/12/06 06:28:33 adri Exp $ + * $Id: tools.h,v 1.1.2.3 2007/12/16 16:14:23 adri Exp $ */ #ifndef __LIBCORE_TOOLS_H__ #define __LIBCORE_TOOLS_H__ @@ -52,4 +52,6 @@ extern double gb_to_double(const gb_t * g); extern const char * gb_to_str(const gb_t * g); +#define XMIN(x,y) ((x)<(y)? (x) : (y)) + #endif /* __LIBCORE_TOOLS_H__ */ Index: squid/src/external_acl.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/external_acl.c,v retrieving revision 1.26 retrieving revision 1.26.12.1 diff -u -r1.26 -r1.26.12.1 --- squid/src/external_acl.c 22 May 2007 01:52:00 -0000 1.26 +++ squid/src/external_acl.c 16 Dec 2007 16:14:23 -0000 1.26.12.1 @@ -1,6 +1,6 @@ /* - * $Id: external_acl.c,v 1.26 2007/05/22 01:52:00 squidadm Exp $ + * $Id: external_acl.c,v 1.26.12.1 2007/12/16 16:14:23 adri Exp $ * * DEBUG: section 82 External ACL * AUTHOR: Henrik Nordstrom, MARA Systems AB @@ -629,9 +629,11 @@ memBufReset(&mb); for (format = acl_data->def->format; format; format = format->next) { const char *str = NULL; + int len = -1; switch (format->type) { case EXT_ACL_LOGIN: str = authenticateUserRequestUsername(request->auth_user_request); + len = strlen(str); break; #if USE_IDENT case EXT_ACL_IDENT: @@ -640,89 +642,110 @@ ch->state[ACL_IDENT] = ACL_LOOKUP_NEEDED; return NULL; } + len = strlen(str); break; #endif case EXT_ACL_SRC: str = inet_ntoa(ch->src_addr); + len = strlen(str); break; case EXT_ACL_SRCPORT: - snprintf(buf, sizeof(buf), "%d", request->client_port); + len = snprintf(buf, sizeof(buf), "%d", request->client_port); str = buf; break; case EXT_ACL_MYADDR: str = inet_ntoa(request->my_addr); + len = strlen(str); break; case EXT_ACL_MYPORT: - snprintf(buf, sizeof(buf), "%d", request->my_port); + len = snprintf(buf, sizeof(buf), "%d", request->my_port); str = buf; break; case EXT_ACL_URI: str = urlCanonical(request); + len = strlen(str); break; case EXT_ACL_DST: str = request->host; + len = strlen(str); break; case EXT_ACL_PROTO: str = ProtocolStr[request->protocol]; + len = strlen(str); break; case EXT_ACL_PORT: - snprintf(buf, sizeof(buf), "%d", request->port); + len = snprintf(buf, sizeof(buf), "%d", request->port); str = buf; break; case EXT_ACL_PATH: - str = strBuf(request->urlpath); + str = strBuf2(request->urlpath); + len = strLen2(request->urlpath); break; case EXT_ACL_METHOD: str = RequestMethods[request->method].str; + len = strlen(str); break; case EXT_ACL_HEADER: sb = httpHeaderGetByName(&request->header, format->header); - str = strBuf(sb); + str = strBuf2(sb); + len = strLen2(sb); break; case EXT_ACL_HEADER_ID: sb = httpHeaderGetStrOrList(&request->header, format->header_id); - str = strBuf(sb); + str = strBuf2(sb); + len = strLen2(sb); break; case EXT_ACL_HEADER_MEMBER: sb = httpHeaderGetByNameListMember(&request->header, format->header, format->member, format->separator); - str = strBuf(sb); + str = strBuf2(sb); + len = strLen2(sb); break; case EXT_ACL_HEADER_ID_MEMBER: sb = httpHeaderGetListMember(&request->header, format->header_id, format->member, format->separator); - str = strBuf(sb); + str = strBuf2(sb); + len = strLen2(sb); break; #if USE_SSL case EXT_ACL_USER_CERT_RAW: if (cbdataValid(ch->conn)) { SSL *ssl = fd_table[ch->conn->fd].ssl; - if (ssl) + if (ssl) { str = sslGetUserCertificatePEM(ssl); + len = strlen(str); + } } break; case EXT_ACL_USER_CERTCHAIN_RAW: if (cbdataValid(ch->conn)) { SSL *ssl = fd_table[ch->conn->fd].ssl; - if (ssl) + if (ssl) { str = sslGetUserCertificateChainPEM(ssl); + len = strlen(str); + } } break; case EXT_ACL_USER_CERT: if (cbdataValid(ch->conn)) { SSL *ssl = fd_table[ch->conn->fd].ssl; - if (ssl) + if (ssl) { str = sslGetUserAttribute(ssl, format->header); + len = strlen(str); + } } break; case EXT_ACL_CA_CERT: if (cbdataValid(ch->conn)) { SSL *ssl = fd_table[ch->conn->fd].ssl; - if (ssl) + if (ssl) { str = sslGetCAAttribute(ssl, format->header); + len = strlen(str); + } } break; #endif case EXT_ACL_EXT_USER: str = request->extacl_user; + len = strlen(str); break; case EXT_ACL_DATA: data_used = 1; @@ -734,17 +757,19 @@ stringAppend(&sb, quoted, strlen(quoted)); } else { static MemBuf mb2 = MemBufNULL; - strwordquote(&mb2, arg->key); + strwordquote(&mb2, arg->key, strlen(arg->key)); stringAppend(&sb, mb2.buf, mb2.size); memBufClean(&mb2); } first = 0; } - str = strBuf(sb); + str = strBuf2(sb); + len = strLen2(sb); break; case EXT_ACL_ACL: str = acl_data->name; + len = strlen(str); break; case EXT_ACL_UNKNOWN: @@ -753,17 +778,22 @@ break; } if (str) - if (!*str) + if (!*str) { str = NULL; - if (!str) + len = -1; + } + if (!str) { str = "-"; + len = 1; + } + assert(len > -1); if (!first) memBufAppend(&mb, " ", 1); if (acl_data->def->quote == QUOTE_METHOD_URL) { - const char *quoted = rfc1738_escape(str); + const char *quoted = rfc1738_escape_str(str, len); memBufAppend(&mb, quoted, strlen(quoted)); } else { - strwordquote(&mb, str); + strwordquote(&mb, str, strlen(str)); } stringClean(&sb); first = 0; @@ -776,7 +806,7 @@ const char *quoted = rfc1738_escape(arg->key); memBufAppend(&mb, quoted, strlen(quoted)); } else { - strwordquote(&mb, arg->key); + strwordquote(&mb, arg->key, strlen(arg->key)); } first = 0; } Index: squid/src/protos.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/protos.h,v retrieving revision 1.146.2.4.4.22 retrieving revision 1.146.2.4.4.23 diff -u -r1.146.2.4.4.22 -r1.146.2.4.4.23 --- squid/src/protos.h 16 Dec 2007 14:52:31 -0000 1.146.2.4.4.22 +++ squid/src/protos.h 16 Dec 2007 16:14:23 -0000 1.146.2.4.4.23 @@ -1,6 +1,6 @@ /* - * $Id: protos.h,v 1.146.2.4.4.22 2007/12/16 14:52:31 adri Exp $ + * $Id: protos.h,v 1.146.2.4.4.23 2007/12/16 16:14:23 adri Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -1190,7 +1190,7 @@ extern int parse_sockaddr(char *s, struct sockaddr_in *addr); char *strwordtok(char *buf, char **t); -void strwordquote(MemBuf * mb, const char *str); +void strwordquote(MemBuf * mb, const char *str, int len); void setUmask(mode_t mask); int xusleep(unsigned int usec); Index: squid/src/tools.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/tools.c,v retrieving revision 1.62.2.3.4.3 retrieving revision 1.62.2.3.4.4 diff -u -r1.62.2.3.4.3 -r1.62.2.3.4.4 --- squid/src/tools.c 6 Dec 2007 06:34:46 -0000 1.62.2.3.4.3 +++ squid/src/tools.c 16 Dec 2007 16:14:25 -0000 1.62.2.3.4.4 @@ -1,6 +1,6 @@ /* - * $Id: tools.c,v 1.62.2.3.4.3 2007/12/06 06:34:46 adri Exp $ + * $Id: tools.c,v 1.62.2.3.4.4 2007/12/16 16:14:25 adri Exp $ * * DEBUG: section 21 Misc Functions * AUTHOR: Harvest Derived @@ -1185,34 +1185,41 @@ /* * Inverse of strwordtok. Quotes a word if needed + * XXX is this len-safe? strchr and strcspn are both assuming NUL-terminated + * XXX strings! */ void -strwordquote(MemBuf * mb, const char *str) +strwordquote(MemBuf * mb, const char *str, int len) { int quoted = 0; - if (strchr(str, ' ')) { + int i; + const char *c; + /* XXX off-by-one? [ahc] */ + if ((c = strchr(str, ' ')) && (c - str < len)) { quoted = 1; memBufAppend(mb, "\"", 1); } - while (*str) { - int l = strcspn(str, "\"\\"); - memBufAppend(mb, str, l); - str += l; - switch (*str) { + for (i = 0; i < len; i++) { + int l = strcspn(str + i, "\"\\"); + if (i + l < len) { + memBufAppend(mb, str, l); + i += l; + } + switch (*(str + i)) { case '\n': memBufAppend(mb, "\\n", 2); - str++; + i++; break; case '\r': memBufAppend(mb, "\\r", 2); - str++; + i++; break; case '\0': break; default: memBufAppend(mb, "\\", 1); - memBufAppend(mb, str, 1); - str++; + memBufAppend(mb, str + i, 1); + i++; break; } }