On 22/11/2013 8:20 a.m., Alex Rousskov wrote:
> On 11/18/2013 05:13 AM, Amos Jeffries wrote:
>> On 16/10/2013 7:12 a.m., Alex Rousskov wrote:
>>> On 10/11/2013 11:03 PM, Amos Jeffries wrote:
>>>> On 10/10/2013 10:12 p.m., Amos Jeffries wrote:
>>>>> Replace String class internals with an SBuf.
>>> FWIW, you made the review process more time consuming and less accurate
>>> [for me] by moving implementation from .cc and .cci into .h. As you
>>> know, bzr does not track such changes well. I only found a bug or two,
>>> but I am not confident the move did not hide more problems from me.
>>
>> Okay, undone that moving now. The attached patch is more easily comparible.
>
> Thank you. I am using the patch from your recent posting on the same thread.
>
>
>> - String(char const *);
>> + explicit String(char const *);
>
> Most of the non-String.* changes in the patch are due to this change.
> Most of those changes are not in a performance-sensitive code. And many
> are due to some _other_ code using char* types. Furthermore:
>
>> String &operator =(char const *);
>
> The above operator is inconsistent with an explicit char* constructor.
> If you do not allow implicit construction, you should not allow
> assignment either because it is just one more form of an implicit char*
> conversion.
>
> Please consider postponing adding "explicit" to the old constructor
> (which is unrelated to the patch scope AFAICT) until more stuff is
> converted to SBuf.
>
>
> The above comment does not apply to the explicit SBuf constructor. I am
> not sure "explicit" is warranted there, but I do not have enough
> ammunition to object to it: My overall motivation here is to make the
> eventual transition to SBuf smoother. Explicit constructors for an
> essentially temporary String class hurt that goal: We are forced to add
> String() wrappers first and then will be forced to remove them when the
> String class is gone...
>
>
>>> * Manipulation of defined_ in the assignment operator is questionable
>>> because it differs from similar manipulation in the constructor. If the
>>> code is correct, a comment explaining the difference is warranted.
>>
>> Done. Both use size()>0 now.
>
> There is still a difference with:
>
>> +String::String(char const *aString) :
>> + buf_(aString),
>> + defined_(aString != NULL)
>
> Above, defined_ for "" is true. In the new SBuf constructor, defined_
> for "" is false. If the code is correct, a comment explaining the
> difference is warranted.
>
>
>> + if (str) {
>> + append(str);
>> + // XXX: empty string "" sent to append means no change,
>
> The comment is misleading a bit: Appending an empty string sets defined_
> to true in the patched code because patched String::append() sets
> defined_ to true unconditionally.
>
>
>> char const *
>> String::termedBuf() const
>> {
>> - return buf_;
>> + if (!defined()) return NULL;
>
> Since an empty String could have been defined() in the unpatched code, I
> suggest relaxing this to always return c_str(), even for undefined
> Strings. I think it will be safer this way, but you should double check
> that I did not miss any cases where it would break something.
>
I've gone through the non-String code use of String::defined() and tried
to find other ways of avoiding the defined()/NULL testing for ambiguous
cases of String/SBuf differences.
If you can check this attached patch is okay for merge to trunk we can
drop the nasty defined_ tracking the conversion patch has to do.
Amos
=== modified file 'src/HttpHdrCc.h'
--- src/HttpHdrCc.h 2013-03-26 10:33:10 +0000
+++ src/HttpHdrCc.h 2013-11-22 14:52:28 +0000
@@ -61,53 +61,53 @@
min_fresh(MIN_FRESH_UNKNOWN) {}
/// reset data-members to default state
void clear();
/// parse a header-string and fill in appropriate values.
bool parse(const String & s);
//manipulation for Cache-Control: public header
bool hasPublic() const {return isSet(CC_PUBLIC);}
bool Public() const {return isSet(CC_PUBLIC);}
void Public(bool v) {setMask(CC_PUBLIC,v);}
void clearPublic() {setMask(CC_PUBLIC,false);}
//manipulation for Cache-Control: private header
bool hasPrivate() const {return isSet(CC_PRIVATE);}
const String &Private() const {return private_;}
void Private(String &v) {
setMask(CC_PRIVATE,true);
// uses append for multi-line headers
- if (private_.defined())
+ if (private_.size() > 0)
private_.append(",");
private_.append(v);
}
void clearPrivate() {setMask(CC_PRIVATE,false); private_.clean();}
//manipulation for Cache-Control: no-cache header
bool hasNoCache() const {return isSet(CC_NO_CACHE);}
const String &noCache() const {return no_cache;}
void noCache(String &v) {
setMask(CC_NO_CACHE,true);
// uses append for multi-line headers
- if (no_cache.defined())
+ if (no_cache.size() > 0)
no_cache.append(",");
no_cache.append(v);
}
void clearNoCache() {setMask(CC_NO_CACHE,false); no_cache.clean();}
//manipulation for Cache-Control: no-store header
bool hasNoStore() const {return isSet(CC_NO_STORE);}
bool noStore() const {return isSet(CC_NO_STORE);}
void noStore(bool v) {setMask(CC_NO_STORE,v);}
void clearNoStore() {setMask(CC_NO_STORE,false);}
//manipulation for Cache-Control: no-transform header
bool hasNoTransform() const {return isSet(CC_NO_TRANSFORM);}
bool noTransform() const {return isSet(CC_NO_TRANSFORM);}
void noTransform(bool v) {setMask(CC_NO_TRANSFORM,v);}
void clearNoTransform() {setMask(CC_NO_TRANSFORM,false);}
//manipulation for Cache-Control: must-revalidate header
bool hasMustRevalidate() const {return isSet(CC_MUST_REVALIDATE);}
bool mustRevalidate() const {return isSet(CC_MUST_REVALIDATE);}
=== modified file 'src/HttpHeaderTools.cc'
--- src/HttpHeaderTools.cc 2013-10-25 00:13:46 +0000
+++ src/HttpHeaderTools.cc 2013-11-22 14:55:11 +0000
@@ -277,41 +277,41 @@
}
end = pos;
while (end < (start+len) && *end != '\\' && *end != '\"' && (unsigned
char)*end > 0x1F && *end != 0x7F)
++end;
if (((unsigned char)*end <= 0x1F && *end != '\r' && *end != '\n') ||
*end == 0x7F) {
debugs(66, 2, HERE << "failed to parse a quoted-string header
field with CTL octet " << (start-pos)
<< " bytes into '" << start << "'");
val->clean();
return 0;
}
val->append(pos, end-pos);
pos = end;
}
if (*pos != '\"') {
debugs(66, 2, HERE << "failed to parse a quoted-string header field
which did not end with \" ");
val->clean();
return 0;
}
/* Make sure it's defined even if empty "" */
- if (!val->defined())
+ if (!val->termedBuf())
val->limitInit("", 0);
return 1;
}
/**
* Checks the anonymizer (header_access) configuration.
*
* \retval 0 Header is explicitly blocked for removal
* \retval 1 Header is explicitly allowed
* \retval 1 Header has been replaced, the current version can be used.
* \retval 1 Header has no access controls to test
*/
static int
httpHdrMangle(HttpHeaderEntry * e, HttpRequest * request, int req_or_rep)
{
int retval;
/* check with anonymizer tables */
HeaderManglers *hms = NULL;
assert(e);
=== modified file 'src/SquidString.h'
--- src/SquidString.h 2013-10-04 13:55:21 +0000
+++ src/SquidString.h 2013-11-22 15:04:22 +0000
@@ -55,48 +55,40 @@
typedef size_t size_type; //storage size intentionally unspecified
const static size_type npos = std::string::npos;
String &operator =(char const *);
String &operator =(String const &);
bool operator ==(String const &) const;
bool operator !=(String const &) const;
/**
* Retrieve a single character in the string.
\param pos Position of character to retrieve.
*/
_SQUID_INLINE_ char operator [](unsigned int pos) const;
_SQUID_INLINE_ size_type size() const;
/// variant of size() suited to be used for printf-alikes.
/// throws when size() > MAXINT
int psize() const;
/**
- * \retval true the String has some contents
- */
- _SQUID_INLINE_ bool defined() const;
- /**
- * \retval true the String does not hold any contents
- */
- _SQUID_INLINE_ bool undefined() const;
- /**
* Returns a raw pointer to the underlying backing store. The caller has
been
* verified not to make any assumptions about null-termination
*/
_SQUID_INLINE_ char const * rawBuf() const;
/**
* Returns a raw pointer to the underlying backing store.
* The caller requires it to be null-terminated.
*/
_SQUID_INLINE_ char const * termedBuf() const;
void limitInit(const char *str, int len); // TODO: rename to assign()
void clean();
void reset(char const *str);
void append(char const *buf, int len);
void append(char const *buf);
void append(char const);
void append(String const &);
void absorb(String &old);
const char * pos(char const *aString) const;
const char * pos(char const ch) const;
///offset from string start of the first occurrence of ch
@@ -104,40 +96,43 @@
size_type find(char const ch) const;
size_type find(char const *aString) const;
const char * rpos(char const ch) const;
size_type rfind(char const ch) const;
_SQUID_INLINE_ int cmp(char const *) const;
_SQUID_INLINE_ int cmp(char const *, size_type count) const;
_SQUID_INLINE_ int cmp(String const &) const;
_SQUID_INLINE_ int caseCmp(char const *) const;
_SQUID_INLINE_ int caseCmp(char const *, size_type count) const;
_SQUID_INLINE_ int caseCmp(String const &) const;
String substr(size_type from, size_type to) const;
_SQUID_INLINE_ void cut(size_type newLength);
private:
void allocAndFill(const char *str, int len);
void allocBuffer(size_type sz);
void setBuffer(char *buf, size_type sz);
+ bool defined() const {return buf_!=NULL;}
+ bool undefined() const {return !defined();}
+
_SQUID_INLINE_ bool nilCmp(bool, bool, int &) const;
/* never reference these directly! */
size_type size_; /* buffer size; 64K limit */
size_type len_; /* current length */
char *buf_;
_SQUID_INLINE_ void set(char const *loc, char const ch);
_SQUID_INLINE_ void cutPointer(char const *loc);
};
_SQUID_INLINE_ std::ostream & operator<<(std::ostream& os, String const
&aString);
_SQUID_INLINE_ bool operator<(const String &a, const String &b);
#if _USE_INLINE_
#include "String.cci"
=== modified file 'src/String.cci'
--- src/String.cci 2012-09-22 01:28:35 +0000
+++ src/String.cci 2013-11-22 15:01:15 +0000
@@ -36,50 +36,40 @@
#include <stdint.h>
#else /* HAVE_STDINT_H */
#ifndef INT_MAX
#define INT_MAX 1<<31 //hack but a safe bet
#endif /* INT_MAX */
#endif /* HAVE_STDINT_H */
String::String() : size_(0), len_(0), buf_(NULL)
{
#if DEBUGSTRINGS
StringRegistry::Instance().add(this);
#endif
}
String::size_type
String::size() const
{
return len_;
}
-bool String::defined() const
-{
- return buf_!=NULL;
-}
-
-bool String::undefined() const
-{
- return buf_==NULL;
-}
-
char const *
String::rawBuf() const
{
return buf_;
}
char const *
String::termedBuf() const
{
return buf_;
}
char
String::operator [](unsigned int aPos) const
{
assert(aPos < size_);
return buf_[aPos];
}
=== modified file 'src/acl/HttpHeaderData.cc'
--- src/acl/HttpHeaderData.cc 2013-10-31 19:13:17 +0000
+++ src/acl/HttpHeaderData.cc 2013-11-22 14:41:12 +0000
@@ -58,47 +58,42 @@
}
bool
ACLHTTPHeaderData::match(HttpHeader* hdr)
{
if (hdr == NULL)
return false;
debugs(28, 3, "aclHeaderData::match: checking '" << hdrName << "'");
String value;
if (hdrId != HDR_BAD_HDR) {
if (!hdr->has(hdrId))
return false;
value = hdr->getStrOrList(hdrId);
} else {
if (!hdr->getByNameIfPresent(hdrName.termedBuf(), value))
return false;
}
- // By now, we know the header is present, but:
- // HttpHeader::get*() return an undefined String for empty header values;
- // String::termedBuf() returns NULL for undefined Strings; and
- // ACLRegexData::match() always fails on NULL strings.
- // This makes it possible to detect an empty header value using regex:
- const char *cvalue = value.defined() ? value.termedBuf() : "";
- return regex_rule->match(cvalue);
+ const SBuf cvalue(value);
+ return regex_rule->match(cvalue.c_str());
}
wordlist *
ACLHTTPHeaderData::dump()
{
wordlist *W = NULL;
wordlistAdd(&W, hdrName.termedBuf());
wordlist * regex_dump = regex_rule->dump();
wordlistAddWl(&W, regex_dump);
wordlistDestroy(®ex_dump);
return W;
}
void
ACLHTTPHeaderData::parse()
{
char* t = strtokFile();
assert (t != NULL);
hdrName = t;
hdrId = httpHeaderIdByNameDef(hdrName.rawBuf(), hdrName.size());
=== modified file 'src/adaptation/ecap/MessageRep.cc'
--- src/adaptation/ecap/MessageRep.cc 2013-10-25 00:13:46 +0000
+++ src/adaptation/ecap/MessageRep.cc 2013-11-22 14:39:07 +0000
@@ -22,41 +22,41 @@
{
}
bool
Adaptation::Ecap::HeaderRep::hasAny(const Name &name) const
{
const http_hdr_type squidId = TranslateHeaderId(name);
// XXX: optimize to remove getByName: we do not need the value here
return squidId == HDR_OTHER ?
theHeader.getByName(name.image().c_str()).size() > 0:
(bool)theHeader.has(squidId);
}
Adaptation::Ecap::HeaderRep::Value
Adaptation::Ecap::HeaderRep::value(const Name &name) const
{
const http_hdr_type squidId = TranslateHeaderId(name);
const String value = squidId == HDR_OTHER ?
theHeader.getByName(name.image().c_str()) :
theHeader.getStrOrList(squidId);
- return value.defined() ?
+ return value.size() > 0 ?
Value::FromTempString(value.termedBuf()) : Value();
}
void
Adaptation::Ecap::HeaderRep::add(const Name &name, const Value &value)
{
const http_hdr_type squidId = TranslateHeaderId(name); // HDR_OTHER OK
HttpHeaderEntry *e = new HttpHeaderEntry(squidId, name.image().c_str(),
value.toString().c_str());
theHeader.addEntry(e);
if (squidId == HDR_CONTENT_LENGTH)
theMessage.content_length = theHeader.getInt64(HDR_CONTENT_LENGTH);
}
void
Adaptation::Ecap::HeaderRep::removeAny(const Name &name)
{
const http_hdr_type squidId = TranslateHeaderId(name);
if (squidId == HDR_OTHER)
=== modified file 'src/adaptation/ecap/XactionRep.cc'
--- src/adaptation/ecap/XactionRep.cc 2013-10-25 00:13:46 +0000
+++ src/adaptation/ecap/XactionRep.cc 2013-11-22 15:22:04 +0000
@@ -126,41 +126,41 @@
client_addr = request->client_addr;
if (!client_addr.isAnyAddr() && !client_addr.isNoAddr()) {
char ntoabuf[MAX_IPSTRLEN] = "";
client_addr.toStr(ntoabuf,MAX_IPSTRLEN);
return libecap::Area::FromTempBuffer(ntoabuf, strlen(ntoabuf));
}
}
return libecap::Area();
}
const libecap::Area
Adaptation::Ecap::XactionRep::usernameValue() const
{
#if USE_AUTH
const HttpRequest *request = dynamic_cast<const HttpRequest*>(theCauseRep ?
theCauseRep->raw().header :
theVirginRep.raw().header);
Must(request);
if (request->auth_user_request != NULL) {
if (char const *name = request->auth_user_request->username())
return libecap::Area::FromTempBuffer(name, strlen(name));
- else if (request->extacl_user.defined() && request->extacl_user.size())
+ else if (request->extacl_user.size() > 0)
return libecap::Area::FromTempBuffer(request->extacl_user.rawBuf(),
request->extacl_user.size());
}
#endif
return libecap::Area();
}
const libecap::Area
Adaptation::Ecap::XactionRep::masterxSharedValue(const libecap::Name &name)
const
{
const HttpRequest *request = dynamic_cast<const HttpRequest*>(theCauseRep ?
theCauseRep->raw().header :
theVirginRep.raw().header);
Must(request);
if (name.known()) { // must check to avoid empty names matching unset cfg
Adaptation::History::Pointer ah = request->adaptHistory(false);
if (ah != NULL) {
String name, value;
if (ah->getXxRecord(name, value))
return libecap::Area::FromTempBuffer(value.rawBuf(),
value.size());
}
=== modified file 'src/adaptation/icap/ModXact.cc'
--- src/adaptation/icap/ModXact.cc 2013-11-11 12:09:44 +0000
+++ src/adaptation/icap/ModXact.cc 2013-11-22 14:37:06 +0000
@@ -1335,41 +1335,41 @@
const Adaptation::ServiceConfig &s = service().cfg();
buf.Printf("%s " SQUIDSTRINGPH " ICAP/1.0\r\n", s.methodStr(),
SQUIDSTRINGPRINT(s.uri));
buf.Printf("Host: " SQUIDSTRINGPH ":%d\r\n", SQUIDSTRINGPRINT(s.host),
s.port);
buf.Printf("Date: %s\r\n", mkrfc1123(squid_curtime));
if (!TheConfig.reuse_connections)
buf.Printf("Connection: close\r\n");
const HttpRequest *request = &virginRequest();
// we must forward "Proxy-Authenticate" and "Proxy-Authorization"
// as ICAP headers.
if (virgin.header->header.has(HDR_PROXY_AUTHENTICATE)) {
String vh=virgin.header->header.getByName("Proxy-Authenticate");
buf.Printf("Proxy-Authenticate: " SQUIDSTRINGPH
"\r\n",SQUIDSTRINGPRINT(vh));
}
if (virgin.header->header.has(HDR_PROXY_AUTHORIZATION)) {
String vh=virgin.header->header.getByName("Proxy-Authorization");
buf.Printf("Proxy-Authorization: " SQUIDSTRINGPH "\r\n",
SQUIDSTRINGPRINT(vh));
- } else if (request->extacl_user.defined() && request->extacl_user.size()
&& request->extacl_passwd.defined() && request->extacl_passwd.size()) {
+ } else if (request->extacl_user.size() > 0 &&
request->extacl_passwd.size() > 0) {
char loginbuf[256];
snprintf(loginbuf, sizeof(loginbuf), SQUIDSTRINGPH ":" SQUIDSTRINGPH,
SQUIDSTRINGPRINT(request->extacl_user),
SQUIDSTRINGPRINT(request->extacl_passwd));
buf.Printf("Proxy-Authorization: Basic %s\r\n",
old_base64_encode(loginbuf));
}
// share the cross-transactional database records if needed
if (Adaptation::Config::masterx_shared_name) {
Adaptation::History::Pointer ah = request->adaptHistory(false);
if (ah != NULL) {
String name, value;
if (ah->getXxRecord(name, value)) {
buf.Printf(SQUIDSTRINGPH ": " SQUIDSTRINGPH "\r\n",
SQUIDSTRINGPRINT(name), SQUIDSTRINGPRINT(value));
}
}
}
buf.Printf("Encapsulated: ");
@@ -1492,41 +1492,41 @@
else if (allow204out)
allowHeader = "Allow: 204\r\n";
else if (allow206)
allowHeader = "Allow: 206\r\n";
if (allowHeader) { // may be nil if only allow204in is true
buf.append(allowHeader, strlen(allowHeader));
debugs(93,5, HERE << "Will write " << allowHeader);
}
}
void Adaptation::Icap::ModXact::makeUsernameHeader(const HttpRequest *request,
MemBuf &buf)
{
#if USE_AUTH
if (request->auth_user_request != NULL) {
char const *name = request->auth_user_request->username();
if (name) {
const char *value = TheConfig.client_username_encode ?
old_base64_encode(name) : name;
buf.Printf("%s: %s\r\n", TheConfig.client_username_header, value);
}
- } else if (request->extacl_user.defined() && request->extacl_user.size()) {
+ } else if (request->extacl_user.size() > 0) {
const char *value = TheConfig.client_username_encode ?
old_base64_encode(request->extacl_user.termedBuf()) :
request->extacl_user.termedBuf();
buf.Printf("%s: %s\r\n", TheConfig.client_username_header, value);
}
#endif
}
void Adaptation::Icap::ModXact::encapsulateHead(MemBuf &icapBuf, const char
*section, MemBuf &httpBuf, const HttpMsg *head)
{
// update ICAP header
icapBuf.Printf("%s=%d, ", section, (int) httpBuf.contentSize());
// begin cloning
HttpMsg::Pointer headClone;
if (const HttpRequest* old_request = dynamic_cast<const
HttpRequest*>(head)) {
HttpRequest::Pointer new_request(new HttpRequest);
Must(old_request->canonical);
urlParse(old_request->method, old_request->canonical,
new_request.getRaw());
new_request->http_ver = old_request->http_ver;
headClone = new_request.getRaw();
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2013-11-11 12:09:44 +0000
+++ src/client_side.cc 2013-11-22 14:51:16 +0000
@@ -3772,41 +3772,41 @@
} else {
Ssl::CrtdMessage reply_message(Ssl::CrtdMessage::REPLY);
if (reply_message.parse(reply.other().content(),
reply.other().contentSize()) != Ssl::CrtdMessage::OK) {
debugs(33, 5, HERE << "Reply from ssl_crtd for " <<
sslConnectHostOrIp << " is incorrect");
} else {
if (reply.result != HelperReply::Okay) {
debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp
<< " cannot be generated. ssl_crtd response: " << reply_message.getBody());
} else {
debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp
<< " was successfully recieved from ssl_crtd");
SSL_CTX *ctx =
Ssl::generateSslContextUsingPkeyAndCertFromMemory(reply_message.getBody().c_str(),
*port);
getSslContextDone(ctx, true);
return;
}
}
}
getSslContextDone(NULL);
}
void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties
&certProperties)
{
- certProperties.commonName = sslCommonName.defined() ?
sslCommonName.termedBuf() : sslConnectHostOrIp.termedBuf();
+ certProperties.commonName = sslCommonName.size() > 0 ?
sslCommonName.termedBuf() : sslConnectHostOrIp.termedBuf();
// fake certificate adaptation requires bump-server-first mode
if (!sslServerBump) {
assert(port->signingCert.get());
certProperties.signWithX509.resetAndLock(port->signingCert.get());
if (port->signPkey.get())
certProperties.signWithPkey.resetAndLock(port->signPkey.get());
certProperties.signAlgorithm = Ssl::algSignTrusted;
return;
}
// In case of an error while connecting to the secure server, use a fake
// trusted certificate, with no mimicked fields and no adaptation
// algorithms. There is nothing we can mimic so we want to minimize the
// number of warnings the user will have to see to get to the error page.
assert(sslServerBump->entry);
if (sslServerBump->entry->isEmpty()) {
if (X509 *mimicCert = sslServerBump->serverCert.get())
certProperties.mimicCert.resetAndLock(mimicCert);
@@ -3867,41 +3867,41 @@
assert(port->signingCert.get());
certProperties.signWithX509.resetAndLock(port->signingCert.get());
if (port->signPkey.get())
certProperties.signWithPkey.resetAndLock(port->signPkey.get());
}
signAlgorithm = certProperties.signAlgorithm;
}
void
ConnStateData::getSslContextStart()
{
assert(areAllContextsForThisConnection());
freeAllContexts();
/* careful: freeAllContexts() above frees request, host, etc. */
if (port->generateHostCertificates) {
Ssl::CertificateProperties certProperties;
buildSslCertGenerationParams(certProperties);
sslBumpCertKey = certProperties.dbKey().c_str();
- assert(sslBumpCertKey.defined() && sslBumpCertKey[0] != '\0');
+ assert(sslBumpCertKey.size() > 0 && sslBumpCertKey[0] != '\0');
debugs(33, 5, HERE << "Finding SSL certificate for " << sslBumpCertKey
<< " in cache");
Ssl::LocalContextStorage &
ssl_ctx_cache(Ssl::TheGlobalContextStorage.getLocalStorage(port->s));
SSL_CTX * dynCtx = NULL;
Ssl::SSL_CTX_Pointer *cachedCtx =
ssl_ctx_cache.get(sslBumpCertKey.termedBuf());
if (cachedCtx && (dynCtx = cachedCtx->get())) {
debugs(33, 5, HERE << "SSL certificate for " << sslBumpCertKey <<
" have found in cache");
if (Ssl::verifySslCertificate(dynCtx, certProperties)) {
debugs(33, 5, HERE << "Cached SSL certificate for " <<
sslBumpCertKey << " is valid");
getSslContextDone(dynCtx);
return;
} else {
debugs(33, 5, HERE << "Cached SSL certificate for " <<
sslBumpCertKey << " is out of date. Delete this certificate from cache");
ssl_ctx_cache.del(sslBumpCertKey.termedBuf());
}
} else {
debugs(33, 5, HERE << "SSL certificate for " << sslBumpCertKey <<
" haven't found in cache");
}
#if USE_SSL_CRTD
@@ -3934,41 +3934,41 @@
ConnStateData::getSslContextDone(SSL_CTX * sslContext, bool isNew)
{
// Try to add generated ssl context to storage.
if (port->generateHostCertificates && isNew) {
if (signAlgorithm == Ssl::algSignTrusted) {
// Add signing certificate to the certificates chain
X509 *cert = port->signingCert.get();
if (SSL_CTX_add_extra_chain_cert(sslContext, cert)) {
// increase the certificate lock
CRYPTO_add(&(cert->references),1,CRYPTO_LOCK_X509);
} else {
const int ssl_error = ERR_get_error();
debugs(33, DBG_IMPORTANT, "WARNING: can not add signing
certificate to SSL context chain: " << ERR_error_string(ssl_error, NULL));
}
Ssl::addChainToSslContext(sslContext, port->certsToChain.get());
}
//else it is self-signed or untrusted do not attrach any certificate
Ssl::LocalContextStorage &
ssl_ctx_cache(Ssl::TheGlobalContextStorage.getLocalStorage(port->s));
- assert(sslBumpCertKey.defined() && sslBumpCertKey[0] != '\0');
+ assert(sslBumpCertKey.size() > 0 && sslBumpCertKey[0] != '\0');
if (sslContext) {
if (!ssl_ctx_cache.add(sslBumpCertKey.termedBuf(), new
Ssl::SSL_CTX_Pointer(sslContext))) {
// If it is not in storage delete after using. Else storage
deleted it.
fd_table[clientConnection->fd].dynamicSslContext = sslContext;
}
} else {
debugs(33, 2, HERE << "Failed to generate SSL cert for " <<
sslConnectHostOrIp);
}
}
// If generated ssl context = NULL, try to use static ssl context.
if (!sslContext) {
if (!port->staticSslContext) {
debugs(83, DBG_IMPORTANT, "Closing SSL " <<
clientConnection->remote << " as lacking SSL context");
clientConnection->close();
return;
} else {
debugs(33, 5, HERE << "Using static ssl context.");
sslContext = port->staticSslContext.get();
}
=== modified file 'src/errorpage.cc'
--- src/errorpage.cc 2013-10-25 00:13:46 +0000
+++ src/errorpage.cc 2013-11-22 14:51:57 +0000
@@ -822,41 +822,41 @@
break;
case 'B':
if (building_deny_info_url) break;
p = request ? ftpUrlWith2f(request) : "[no URL]";
break;
case 'c':
if (building_deny_info_url) break;
p = errorPageName(type);
break;
case 'D':
if (!allowRecursion)
p = "%D"; // if recursion is not allowed, do not convert
#if USE_SSL
// currently only SSL error details implemented
else if (detail) {
detail->useRequest(request);
const String &errDetail = detail->toString();
- if (errDetail.defined()) {
+ if (errDetail.size() > 0) {
MemBuf *detail_mb = ConvertText(errDetail.termedBuf(), false);
mb.append(detail_mb->content(), detail_mb->contentSize());
delete detail_mb;
do_quote = 0;
}
}
#endif
if (!mb.contentSize())
mb.Printf("[No Error Detail]");
break;
case 'e':
mb.Printf("%d", xerrno);
break;
case 'E':
if (xerrno)
mb.Printf("(%d) %s", xerrno, strerror(xerrno));
else
mb.Printf("[No Error]");
=== modified file 'src/http.cc'
--- src/http.cc 2013-11-18 15:55:05 +0000
+++ src/http.cc 2013-11-22 15:18:38 +0000
@@ -345,41 +345,41 @@
// NP: reverse-proxy traffic our parent server has instructed us never to
cache
if (surrogateNoStore) {
debugs(22, 3, HERE << "NO because Surrogate-Control:no-store");
return 0;
}
// RFC 2616: HTTP/1.1 Cache-Control conditions
if (!ignoreCacheControl) {
// XXX: check to see if the request headers alone were enough to
prevent caching earlier
// (ie no-store request header) no need to check those all again here
if so.
// for now we are not reliably doing that so we waste CPU re-checking
request CC
// RFC 2616 section 14.9.2 - MUST NOT cache any response with request
CC:no-store
if (request && request->cache_control &&
request->cache_control->noStore() &&
!REFRESH_OVERRIDE(ignore_no_store)) {
debugs(22, 3, HERE << "NO because client request
Cache-Control:no-store");
return 0;
}
// NP: request CC:no-cache only means cache READ is forbidden. STORE
is permitted.
- if (rep->cache_control && rep->cache_control->hasNoCache() &&
rep->cache_control->noCache().defined()) {
+ if (rep->cache_control && rep->cache_control->hasNoCache() &&
rep->cache_control->noCache().size() > 0) {
/* TODO: we are allowed to cache when no-cache= has parameters.
* Provided we strip away any of the listed headers unless they
are revalidated
* successfully (ie, must revalidate AND these headers are
prohibited on stale replies).
* That is a bit tricky for squid right now so we avoid caching
entirely.
*/
debugs(22, 3, HERE << "NO because server reply
Cache-Control:no-cache has parameters");
return 0;
}
// NP: request CC:private is undefined. We ignore.
// NP: other request CC flags are limiters on HIT/MISS. We don't care
about here.
// RFC 2616 section 14.9.2 - MUST NOT cache any response with
CC:no-store
if (rep->cache_control && rep->cache_control->noStore() &&
!REFRESH_OVERRIDE(ignore_no_store)) {
debugs(22, 3, HERE << "NO because server reply
Cache-Control:no-store");
return 0;
}
// RFC 2616 section 14.9.1 - MUST NOT cache any response with
CC:private in a shared cache like Squid.
@@ -410,41 +410,41 @@
debugs(22, 3, HERE << "NO because Authenticated and ignoring
Cache-Control");
return 0;
}
bool mayStore = false;
// HTTPbis pt6 section 3.2: a response CC:public is present
if (rep->cache_control->Public()) {
debugs(22, 3, HERE << "Authenticated but server reply
Cache-Control:public");
mayStore = true;
// HTTPbis pt6 section 3.2: a response CC:must-revalidate is
present
} else if (rep->cache_control->mustRevalidate() &&
!REFRESH_OVERRIDE(ignore_must_revalidate)) {
debugs(22, 3, HERE << "Authenticated but server reply
Cache-Control:public");
mayStore = true;
#if USE_HTTP_VIOLATIONS
// NP: given the must-revalidate exception we should also be able
to exempt no-cache.
// HTTPbis WG verdict on this is that it is omitted from the spec
due to being 'unexpected' by
// some. The caching+revalidate is not exactly unsafe though with
Squids interpretation of no-cache
// (without parameters) as equivalent to must-revalidate in the
reply.
- } else if (rep->cache_control->hasNoCache() &&
!rep->cache_control->noCache().defined() &&
!REFRESH_OVERRIDE(ignore_must_revalidate)) {
+ } else if (rep->cache_control->hasNoCache() &&
rep->cache_control->noCache().size() > 0 &&
!REFRESH_OVERRIDE(ignore_must_revalidate)) {
debugs(22, 3, HERE << "Authenticated but server reply
Cache-Control:no-cache (equivalent to must-revalidate)");
mayStore = true;
#endif
// HTTPbis pt6 section 3.2: a response CC:s-maxage is present
} else if (rep->cache_control->sMaxAge()) {
debugs(22, 3, HERE << "Authenticated but server reply
Cache-Control:s-maxage");
mayStore = true;
}
if (!mayStore) {
debugs(22, 3, HERE << "NO because Authenticated transaction");
return 0;
}
// NP: response CC:no-cache is equivalent to
CC:must-revalidate,max-age=0. We MAY cache, and do so.
// NP: other request CC flags are limiters on HIT/MISS/REFRESH. We
don't care about here.
}
/* HACK: The "multipart/x-mixed-replace" content type is used for
@@ -1689,41 +1689,41 @@
StoreEntry * entry,
const AccessLogEntryPointer &al,
HttpHeader * hdr_out,
const HttpStateFlags &flags)
{
/* building buffer for complex strings */
#define BBUF_SZ (MAX_URL+32)
LOCAL_ARRAY(char, bbuf, BBUF_SZ);
LOCAL_ARRAY(char, ntoabuf, MAX_IPSTRLEN);
const HttpHeader *hdr_in = &request->header;
const HttpHeaderEntry *e = NULL;
HttpHeaderPos pos = HttpHeaderInitPos;
assert (hdr_out->owner == hoRequest);
/* use our IMS header if the cached entry has Last-Modified time */
if (request->lastmod > -1)
hdr_out->putTime(HDR_IF_MODIFIED_SINCE, request->lastmod);
// Add our own If-None-Match field if the cached entry has a strong ETag.
// copyOneHeaderFromClientsideRequestToUpstreamRequest() adds client ones.
- if (request->etag.defined()) {
+ if (request->etag.size() > 0) {
hdr_out->addEntry(new HttpHeaderEntry(HDR_IF_NONE_MATCH, NULL,
request->etag.termedBuf()));
}
bool we_do_ranges = decideIfWeDoRanges (request);
String strConnection (hdr_in->getList(HDR_CONNECTION));
while ((e = hdr_in->getEntry(&pos)))
copyOneHeaderFromClientsideRequestToUpstreamRequest(e, strConnection,
request, hdr_out, we_do_ranges, flags);
/* Abstraction break: We should interpret multipart/byterange responses
* into offset-length data, and this works around our inability to do so.
*/
if (!we_do_ranges && request->multipartRangeRequest()) {
/* don't cache the result */
request->flags.cachable = false;
/* pretend it's not a range request */
delete request->range;
request->range = NULL;
=== modified file 'src/redirect.cc'
--- src/redirect.cc 2013-10-26 15:56:57 +0000
+++ src/redirect.cc 2013-11-22 14:37:53 +0000
@@ -242,42 +242,41 @@
Http::StatusCode status;
char claddr[MAX_IPSTRLEN];
char myaddr[MAX_IPSTRLEN];
/** TODO: create a standalone method to initialize
* the RedirectStateData for all the helpers.
*/
RedirectStateData *r = new RedirectStateData(http->uri);
if (conn != NULL)
r->client_addr = conn->log_addr;
else
r->client_addr.setNoAddr();
r->client_ident = NULL;
#if USE_AUTH
if (http->request->auth_user_request != NULL) {
r->client_ident = http->request->auth_user_request->username();
debugs(61, 5, HERE << "auth-user=" <<
(r->client_ident?r->client_ident:"NULL"));
}
#endif
- // HttpRequest initializes with null_string. So we must check both
defined() and size()
- if (!r->client_ident && http->request->extacl_user.defined() &&
http->request->extacl_user.size()) {
+ if (!r->client_ident && http->request->extacl_user.size() > 0) {
r->client_ident = http->request->extacl_user.termedBuf();
debugs(61, 5, HERE << "acl-user=" <<
(r->client_ident?r->client_ident:"NULL"));
}
if (!r->client_ident && conn != NULL && conn->clientConnection != NULL &&
conn->clientConnection->rfc931[0]) {
r->client_ident = conn->clientConnection->rfc931;
debugs(61, 5, HERE << "ident-user=" <<
(r->client_ident?r->client_ident:"NULL"));
}
#if USE_SSL
if (!r->client_ident && conn != NULL &&
Comm::IsConnOpen(conn->clientConnection)) {
r->client_ident =
sslGetUserEmail(fd_table[conn->clientConnection->fd].ssl);
debugs(61, 5, HERE << "ssl-user=" <<
(r->client_ident?r->client_ident:"NULL"));
}
#endif
if (!r->client_ident)
r->client_ident = dash_str;
=== modified file 'src/ssl/ErrorDetail.cc'
--- src/ssl/ErrorDetail.cc 2013-11-07 10:46:14 +0000
+++ src/ssl/ErrorDetail.cc 2013-11-22 14:53:40 +0000
@@ -387,41 +387,41 @@
/**
* The subject of the current certification in text form
*/
const char *Ssl::ErrorDetail::subject() const
{
if (!broken_cert)
return "[Not available]";
static char tmpBuffer[256]; // A temporary buffer
X509_NAME_oneline(X509_get_subject_name(broken_cert.get()), tmpBuffer,
sizeof(tmpBuffer));
return tmpBuffer;
}
// helper function to be used with Ssl::matchX509CommonNames
static int copy_cn(void *check_data, ASN1_STRING *cn_data)
{
String *str = (String *)check_data;
if (!str) // no data? abort
return 0;
- if (str->defined())
+ if (str->size() > 0)
str->append(", ");
str->append((const char *)cn_data->data, cn_data->length);
return 1;
}
/**
* The list with certificates cn and alternate names
*/
const char *Ssl::ErrorDetail::cn() const
{
if (!broken_cert)
return "[Not available]";
static String tmpStr; ///< A temporary string buffer
tmpStr.clean();
Ssl::matchX509CommonNames(broken_cert.get(), &tmpStr, copy_cn);
return tmpStr.termedBuf();
}
/**
@@ -484,41 +484,41 @@
snprintf(tmpBuffer, 64, "%d", (int)error_no);
err = tmpBuffer;
}
return err;
}
/**
* A short description of the error_no
*/
const char *Ssl::ErrorDetail::err_descr() const
{
if (error_no == SSL_ERROR_NONE)
return "[No Error]";
if (const char *err = detailEntry.descr.termedBuf())
return err;
return "[Not available]";
}
const char *Ssl::ErrorDetail::err_lib_error() const
{
- if (errReason.defined())
+ if (errReason.size() > 0)
return errReason.termedBuf();
else if (lib_error_no != SSL_ERROR_NONE)
return ERR_error_string(lib_error_no, NULL);
else
return "[No Error]";
}
/**
* Converts the code to a string value. Supported formating codes are:
*
* Error meta information:
* %err_name: The name of a high-level SSL error (e.g., X509_V_ERR_*)
* %ssl_error_descr: A short description of the SSL error
* %ssl_lib_error: human-readable low-level error string by
ERR_error_string(3SSL)
*
* Certificate information extracted from broken (not necessarily peer!) cert
* %ssl_cn: The comma-separated list of common and alternate names
* %ssl_subject: The certificate subject
* %ssl_ca_name: The certificate issuer name
* %ssl_notbefore: The certificate "not before" field
@@ -557,41 +557,41 @@
s = detailEntry.detail.termedBuf();
if (!s)
s = SslErrorDetailDefaultStr;
assert(s);
while ((p = strchr(s, '%'))) {
errDetailStr.append(s, p - s);
code_len = convert(++p, &t);
if (code_len)
errDetailStr.append(t);
else
errDetailStr.append("%");
s = p + code_len;
}
errDetailStr.append(s, strlen(s));
}
const String &Ssl::ErrorDetail::toString() const
{
- if (!errDetailStr.defined())
+ if (errDetailStr.size() == 0)
buildDetail();
return errDetailStr;
}
Ssl::ErrorDetail::ErrorDetail( Ssl::ssl_error_t err_no, X509 *cert, X509
*broken, const char *aReason): error_no (err_no), lib_error_no(SSL_ERROR_NONE),
errReason(aReason)
{
if (cert)
peer_cert.resetAndLock(cert);
if (broken)
broken_cert.resetAndLock(broken);
else
broken_cert.resetAndLock(cert);
detailEntry.error_no = SSL_ERROR_NONE;
}
Ssl::ErrorDetail::ErrorDetail(Ssl::ErrorDetail const &anErrDetail)
{
error_no = anErrDetail.error_no;
=== modified file 'src/ssl/ErrorDetailManager.cc'
--- src/ssl/ErrorDetailManager.cc 2013-11-07 10:46:14 +0000
+++ src/ssl/ErrorDetailManager.cc 2013-11-22 15:00:34 +0000
@@ -213,44 +213,43 @@
const String errorName = parser.getByName("name");
if (!errorName.size()) {
debugs(83, DBG_IMPORTANT, HERE <<
"WARNING! invalid or no error detail name on:" << s);
return false;
}
Ssl::ssl_error_t ssl_error =
Ssl::GetErrorCode(errorName.termedBuf());
if (ssl_error != SSL_ERROR_NONE) {
if (theDetails->getErrorDetail(ssl_error)) {
debugs(83, DBG_IMPORTANT, HERE <<
"WARNING! duplicate entry: " << errorName);
return false;
}
ErrorDetailEntry &entry = theDetails->theList[ssl_error];
entry.error_no = ssl_error;
entry.name = errorName;
String tmp = parser.getByName("detail");
- httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(),
&entry.detail);
+ int detailsParseOk =
httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.detail);
tmp = parser.getByName("descr");
- httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(),
&entry.descr);
- bool parseOK = entry.descr.defined() && entry.detail.defined();
+ int descrParseOk =
httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.descr);
- if (!parseOK) {
+ if (!detailsParseOk || !descrParseOk) {
debugs(83, DBG_IMPORTANT, HERE <<
"WARNING! missing important field for detail error:
" << errorName);
return false;
}
} else if (!Ssl::ErrorIsOptional(errorName.termedBuf())) {
debugs(83, DBG_IMPORTANT, HERE <<
"WARNING! invalid error detail name: " << errorName);
return false;
}
}// else {only spaces and black lines; just ignore}
buf.consume(size);
}
debugs(83, 9, HERE << " Remain size: " << buf.contentSize() << " Content:
" << buf.content());
return true;
}
=== modified file 'src/stat.cc'
--- src/stat.cc 2013-10-25 00:13:46 +0000
+++ src/stat.cc 2013-11-22 14:56:52 +0000
@@ -2033,41 +2033,41 @@
storeAppendPrintf(s, "\tnrequests: %d\n",
conn->nrequests);
}
storeAppendPrintf(s, "uri %s\n", http->uri);
storeAppendPrintf(s, "logType %s\n", LogTags_str[http->logType]);
storeAppendPrintf(s, "out.offset %ld, out.size %lu\n",
(long int) http->out.offset, (unsigned long int)
http->out.size);
storeAppendPrintf(s, "req_sz %ld\n", (long int) http->req_sz);
e = http->storeEntry();
storeAppendPrintf(s, "entry %p/%s\n", e, e ? e->getMD5Text() : "N/A");
storeAppendPrintf(s, "start %ld.%06d (%f seconds ago)\n",
(long int) http->start_time.tv_sec,
(int) http->start_time.tv_usec,
tvSubDsec(http->start_time, current_time));
#if USE_AUTH
if (http->request->auth_user_request != NULL)
p = http->request->auth_user_request->username();
else
#endif
- if (http->request->extacl_user.defined()) {
+ if (http->request->extacl_user.size() > 0) {
p = http->request->extacl_user.termedBuf();
}
if (!p && conn != NULL && conn->clientConnection->rfc931[0])
p = conn->clientConnection->rfc931;
#if USE_SSL
if (!p && conn != NULL && Comm::IsConnOpen(conn->clientConnection))
p = sslGetUserEmail(fd_table[conn->clientConnection->fd].ssl);
#endif
if (!p)
p = dash_str;
storeAppendPrintf(s, "username %s\n", p);
#if USE_DELAY_POOLS
storeAppendPrintf(s, "delay_pool %d\n",
DelayId::DelayClient(http).pool());
=== modified file 'src/store.cc'
--- src/store.cc 2013-10-25 00:13:46 +0000
+++ src/store.cc 2013-11-22 14:57:10 +0000
@@ -771,41 +771,41 @@
// TODO: storeGetPublic() calls below may create unlocked entries.
// We should add/use storeHas() API or lock/unlock those entries.
if (mem_obj->vary_headers && !storeGetPublic(mem_obj->url,
mem_obj->method)) {
/* Create "vary" base object */
String vary;
StoreEntry *pe = storeCreateEntry(mem_obj->url, mem_obj->log_url,
request->flags, request->method);
/* We are allowed to do this typecast */
HttpReply *rep = new HttpReply;
rep->setHeaders(Http::scOkay, "Internal marker object",
"x-squid-internal/vary", -1, -1, squid_curtime + 100000);
vary = mem_obj->getReply()->header.getList(HDR_VARY);
if (vary.size()) {
/* Again, we own this structure layout */
rep->header.putStr(HDR_VARY, vary.termedBuf());
vary.clean();
}
#if X_ACCELERATOR_VARY
vary = mem_obj->getReply()->header.getList(HDR_X_ACCELERATOR_VARY);
- if (vary.defined()) {
+ if (vary.size() > 0) {
/* Again, we own this structure layout */
rep->header.putStr(HDR_X_ACCELERATOR_VARY, vary.termedBuf());
vary.clean();
}
#endif
pe->replaceHttpReply(rep);
pe->timestampsSet();
pe->makePublic();
pe->complete();
pe->unlock();
}
newkey = storeKeyPublicByRequest(mem_obj->request);
} else
newkey = storeKeyPublic(mem_obj->url, mem_obj->method);
