amosjeffries [Fri, 18 May 2007 12:41:21 +0000 (12:41 +0000)]
Add string API layer for better string handling.
For various reasons listed below we adopt the std::string API (but not the implementation) as the basis for string operations.
This patch reverts the SquidString.h file to provide only the main API and hooks for any string implementation behind the API.
For Release 3.0 the old String (now SqString) will remain the default string core.
That code has been kept in this patch with some minor modifications and bug fixes as listed below.
For Release 3.1 it is expected that a better string core will be developed.
Reasons for these changes:
The initial String implementation was incomplete and non-standard causing some risky operations at points in the code and duplicated some operations.
std::string provides a better known API for string handling which is widely use amongst other string implementations beyond std::string itself and this enables std::string or a derivative to be used in squid at some future date.
String as used previously is a defined alternative to std::string in some systems
This patch:
- moves the old String class to SqString
- provides the well-known type of 'string' for general use
- provides implicit conversion from char* and char[] types
- migrates custom functions to well-known API:
buf() -> c_str()
clean() -> clear()
- removes redundant functions:
buf(char*) -> operator=(char*)
initBuf(char*) -> operator=(char*)
reset(char*) -> operator=(char*)
- adds well-known API methods for safer string use:
operator []
empty()
operator <<
strcmp(), strcasecmp(), etc
- May fix bug #1088 - segmentation fault after append(char*,int)
- Fixes several unreported char* handling bugs in String/SqString
hno [Fri, 18 May 2007 06:43:28 +0000 (06:43 +0000)]
Imported cf.data.pre -> HTML script from Squid-2.
Imported changes:
2007/05/13 12:18:00 adrian +114 -0 Flesh out the beginnings of a cf.data.pre -> web interface.
2007/05/13 12:34:31 adrian +23 -2 Flesh out a little more for testing. I need to treat NAME entries with
2007/05/13 13:49:56 adrian +18 -2 Handle name aliases right.
2007/05/13 14:02:40 hno +6 -1 Add --out and --verbose command line options
2007/05/13 14:04:40 adrian +33 -32 Prepare for the actual page generating code.
2007/05/13 14:33:12 adrian +72 -19 Begin writing pages.
2007/05/13 14:43:50 adrian +8 -7 More HTML generation fixes.
2007/05/13 14:46:32 adrian +3 -2 Fix escaping.
2007/05/13 14:51:55 adrian +20 -3 Add in some more "full" template..
2007/05/13 14:58:40 adrian +15 -6 * try to show the aliases in the output; they're not yet working
2007/05/13 17:01:45 hno +133 -54 Get the ToC running, with raw section separators.
2007/05/13 21:12:56 hno +8 -0 Dump out the config manual while making snapshots
2007/05/13 21:13:14 hno +5 -3 Pick up the templates relative to the script location
2007/05/13 22:16:56 hno +108 -34 Hacked up a single page document mode
2007/05/13 22:28:49 hno +2 -0 Build single-page config manual as well as part of the snapshot process
2007/05/13 22:34:18 hno +2 -1 Compress the single-page config manual
2007/05/13 22:38:26 hno +1 -1 Correct format of single page configuration manual
2007/05/14 14:34:21 amosjeffries +102 -67 Fancy-Up the squid.conf documentation output.
2007/05/14 14:51:26 adrian +11 -2 Fix the templates to <pre>'ify suggested config.
2007/05/14 14:55:48 adrian +11 -1 Use the new format CSS now.
2007/05/14 15:16:31 adrian +16 -29 * remove the CSS stuff for now; eww.
2007/05/15 11:45:59 amosjeffries +9 -9 Fixed HTML structure somehow broken: <td> =>
hno [Fri, 18 May 2007 01:55:52 +0000 (01:55 +0000)]
Author: Emilio Casbas <ecasbas@unav.es>
logformat %rp logging the URL-Path only (excluding hostname)
we need a "%ru" parameter like the httpd native log, that is showing;
/SI/images/servicios/normasdeuso/normas.swf instead of
http://X.X.X.60/SI/images/servicios/normasdeuso/normas.swf
I have done a small patch to have a new "rp" format code in order to
show only the urlpath in the access log.
rousskov [Fri, 11 May 2007 19:20:57 +0000 (19:20 +0000)]
Bug #1957 fix: Close a persistent ICAP connection if we have to open a
new connection because the transaction is not retriable.
This change avoids a growing number of open connections when many
transactions create persistent connections but only few are retriable
and can reuse them.
FwdState was already doing that. I moved FwdState logic to
PconnPool::pop so that any PconnPool user thinks about the problem and
benefits from the common solution. The change should have no material
affect on FwdState.
wessels [Wed, 9 May 2007 15:07:38 +0000 (15:07 +0000)]
Bug #1951: NTLM authentication does not work
Turns out there were a number of reasons why NTLM authentication
didn't work. Some of these were addressed by patches committed a
few revs earlier. With this current patch, Squid is now handling
a Web Polygraph workload with NTLM authentication.
The big change here is to locking of AuthUserRequest by the various
other classes that maintain a pointer to it. The old locking logic
was difficult for me to follow, and it seemed that there were not
enough unlocks, leading to helper processes getting stuck in RESERVED
state.
I copied the ideas from HttpMsg locking and created macros
AUTHUSERREQUESTLOCK and AUTHUSERREQUESTUNLOCK. I also tried to
make sure that locks and unlocks occur in places where pointers are
assigned. The most tricky part is with the ACLchecklist class,
which passes a pointer to a pointer. Previously AuthUserRequest
was doing a lot of locking on behalf of ACLchecklist, but now I
make ACLchecklist do all its own locking and AuthUserRequest really
shouldn't lock its own objects for any other classes.
Another important change was to helperStatefulReleaseServer(). The
old version apparently ignored helper servers in the S_HELPER_RESERVED
states. It was only concerned about the S_HELPER_DEFERRED state.
Now I think it does the right thing for both states.
This patch also contains numerous cosmetic "style" changes to the
source code and debugging that don't really affect its functionality.
I couldn't resist.
This is due to the checklist being marked as done twice, first in
ProxyAuthNeeded::checkForAsync during the match, and then in
ACLChecklist::matchAclList before returning the verdict.
wessels [Wed, 9 May 2007 14:26:57 +0000 (14:26 +0000)]
Fixed free() errors in AuthUser::~AuthUser()
AuthUser::~AuthUser() was freeing memory in the middle of an allocated
buffer. I think AuthUser needs a strdup'd copy of the username,
rather than keeping a pointer to the buffer read from the helper.
Also I found 'xfree((char *)username());' really odd. This username()
method does not return a reference so it shouldn't be used as LHS
in the macro.
wessels [Wed, 9 May 2007 14:07:23 +0000 (14:07 +0000)]
Port of 2.6/changesets/11280.patch
This is a port of
http://www.squid-cache.org/Versions/v2/2.6/changesets/11280.patch
to Squid-3. It seems that this particular patch was skipped
or missed when other squid-2 changes were ported to squid-3.
This patch removes the AUTHENTICATE_STATE_FINISHED state from
authentication code. It also moves some medium sized blocks of
code from the ::authenticate() method to the to
authenticateFooHandleReply() function.
The functions provided by ICAP/ICAPClientStream.{cc,h} were called from
client_side_reply.cc but the code was #ifdef-ed with
ICAP_CLIENT_RESPMOD_POSTCACHE, which was not defined anywhere. Somebody was
either trying to get ICAP RESPMOD post-cache work with client streams long
time ago (but did not get far) or just wanted to provide hooks for future use.
rousskov [Tue, 8 May 2007 22:46:37 +0000 (22:46 +0000)]
Bug #1819 fix: retry the ICAP transaction when its pconn fails.
The changes in these files (all ICAP initiators) were minor,
except the client side no longer bypasses failed queries for
essential ICAP services because ICAPLauncher tells its initiator
whether the failure is bypassable.
Search src/ICAP/ICAPXaction.cc log around v1.7 for complete
details of the bug #1819 fix.
rousskov [Tue, 8 May 2007 22:45:00 +0000 (22:45 +0000)]
Two unrelated changes: A failed ICAP RESPMOD transaction could leave
the HTTP transaction stuck and a bug #1819 fix.
Change 1: After detecting an ICAP error and calling fwd->fail(),
the HTTP Server now calls abortTransaction() (and, hence,
comm_close) instead of calling closeServer() (and, hence,
fwd->unregister). Server::closeServer() is a virtual method
that should not be called outside of Server::serverComplete().
It would be nice to enforce that somehow. Perhaps we should at
least rename closeServer?
Change 2: Bug #1819 fix: retry the ICAP transaction when its
pconn fails. The changes in this file were minor. Search
src/ICAP/ICAPXaction.cc log around v1.7 for complete details of
the bug #1819 fix.
rousskov [Tue, 8 May 2007 22:37:59 +0000 (22:37 +0000)]
Make sure the entire response body gets to ICAP, even when the
end of it does not fit into the BodyPipe buffer.
Re-enabled code that limits the amount of data the server may
read depending on the available ICAP BodyPipe buffer size.
While new BodyPipes limit the amount of data they accept, we
still need the external limit because there is no code to keep
pumping data into the pipe once the HTTP response ends and
serverComplete() is called. If any body data is left in the
HTTP server read buffer at the serverComplete() time, that data
is lost and the client gets a truncated response.
It is not clear whether it is better to stall the faster HTTP
server I/O to let the slower ICAP server consume the body from
the pipe OR to add code that will drain leftovers from the HTTP
read buffer into the pipe.
rousskov [Tue, 8 May 2007 22:32:11 +0000 (22:32 +0000)]
Bug #1819 fix in three parts: Retry failed ICAP pconns.
Bug #1819 fix, part 1: simplify ICAPInitiator-ICAPXaction link.
Part 1 changes should have no effect on Squid functionality.
To retry failing persistent ICAP connections we may need to
start more than one ICAP transaction for a given HTTP message.
This needs to be done somewhere between ICAPInitiator and
ICAPXaction so that neither is affected much. The design will
be similar to HTTP FwdState.
This change reduces ICAPInitiator dependency on ICAPXaction so
that it is easier to insert transaction retrying logic/code
between them. It should have no effect on Squid functionality.
HTTP client and server (both ICAPInitiators) used to keep a
Pointer to ICAP transaction and extracted adapted headers from
the finished transaction using that Pointer (which forced
ICAPModXact to use a self-Pointer). ICAP service
representative used custom callbacks to receive new options
from ICAPOptXact.
Now, all ICAP initiators use the same asynchronous
message-based API to communicate with ICAP transactions they
initiate. On the core side, the API is defined by
ICAPInitiator, as before. On the ICAP side, the API is defined
by the newly added ICAPInitiate class. The latter will also be
used as a base for ICAPLauncher, the future ICAP transaction
retrying class.
Bug #1819 fix, part 2: retry the ICAP transaction when its
pconn fails. Part 2 changes should enable Squid to handle race
conditions of ICAP pconns.
HTTP client, server, and ICAP server representative (all
ICAPInitiators) used to start ICAP transactions. If a
transaction fails due to a persistent connection race
condition, the initiator would either terminate the HTTP
transaction or bypass the ICAP failure, violating the protocol.
Now, instead of starting an ICAP transaction directly, the
above initiators start ICAP Launcher. The Launcher starts the
ICAP transaction and retries it (i.e., starts another
transaction with the same set of parameters) if needed. The
Launcher is both ICAP initiator (because it starts ICAP
transactions) and ICAP initiate (because it is started by other
ICAP initiators). The launcher proxies ICAP communication
(usually one or two messages each way) and makes retries
transparent to initiators.
All ICAP transactions corresponding to the same adaptation of
an HTTP message are sometimes referred to as a single ICAP
query.
An ICAP transaction can be retried until it read data from the
ICAP server or consumed virgin body. This means that a ICAP
transaction may go as far as writing the entire ICAP Preview
before aborting in a retriable state. When retrying, persistent
connections are not used. If the first retry fails, the query
fails. Thus, a query cannot contain more than two transactions.
Two new things were added to support ICAPLauncher:
First, AsyncJob, is a class that handles the basic logic of
maintaining an asynchronous job or task such as an ICAP
transaction or a launcher. All core AsyncJob functionality was
moved from ICAPXaction and ICAPModXact classes. We could not
use the two classes for the launcher because ICAPLauncher is
not a transaction. While currently specific to ICAP, this
object may eventually be moved to Squid core and serve as a
base for other asynchronous jobs or logical threads.
Second, a temporary hack was added to support cbdata operations
by two parents of a single CBDATA class (ICAPLauncher is both
ICAP Initiator and Initiate). The second parent (ICAPInitiator)
defines toCbdata() method that ICAPLauncher overwrites to
provide a usable cbdata pointer to code that only sees an
ICAPInitiator pointer. The ICAPInitiator pointer is unusable
for cbdata when it comes from the middle of the ICAPLauncher
object. Also, the initiator pointer is stored along with its
cbdata so that the latter can be accessed (and unlocked) even
if the initiator object is already deleted. These hacks will
disappear once cbdata becomes object-aware.
Bug #1819 fix, part 3: do not reuse a pconn when we may fail to
retry it. Part 3 changes ensure that the HTTP message body is
backed up when reusing a persistent connection (until we know
there are no race conditions). They also disable pconns when we
cannot backup enough.
Decide on preview before connecting to the ICAP server because
if we are doing preview, we can retry a failed pconn even if we
cannot backup the entire body. Renamed shouldPreview() to
decideOnPreview() and made the call less dependent on the
context.
Disable transaction retries if we are not using a persistent
connection. ICAP does not talk about retrying failures other
than pconn race conditions.
It might be OK to retry other failures, but I have not done the
required analysis and am not going to do that in the
foreseeable future. For example, other failures may need other
conditions for disabling retries (current code should disable
retries when the first response byte is read).
rousskov [Tue, 8 May 2007 22:16:42 +0000 (22:16 +0000)]
If a consumer leaves without consuming anything, do not tell
the producer that the consumer has aborted.
This change increases the risk that, due to bugs, no [other]
consumer will come and the producer will get stuck waiting for
the buffer space. However, this risk should be mitigated by
timeouts because no amount of code can ensure eventual consumer
presence if producer and consumer are asynchronous.
This change was needed to support ICAP transaction retries.
When a transaction aborts due to pconn race conditions, without
consuming body buffer, the second transaction will start from
scratch and become the body consumer.
wessels [Tue, 8 May 2007 00:38:40 +0000 (00:38 +0000)]
Author: Alex Rousskov <rousskov@measurement-factory.com
Bug #1950: cachemgr requests for auth helper stats can crash Squid
Partially-configured authentication helpers may result in Squid
crashes when requesting stats via cachemgr. The attached patch
moves the "is this helper valid?" check into the helper*Stats()
function so that new helpers do not introduce more bugs like this
one.
This patch also reports invalid/nonexistent helpers, which may or
may not be the right thing to do (but would be trivial to change
since the reporting code is in one place now).
wessels [Tue, 8 May 2007 00:12:28 +0000 (00:12 +0000)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Bug #1688: assertion when Squid filters HTTP headers
HttpHeader::delAt does not update the header mask. This patch adds a parameter
to delAt, which forces the caller to pay attention and call newly added
HttpHeader::refreshMask after calling HttpHeader::delAt.
The patch also optimizes Keep-Alive header handling and comments on
HDR_PROXY_CONNECTION handling.
wessels [Sat, 5 May 2007 06:14:17 +0000 (06:14 +0000)]
FTP will no longer send REST command if offset exceeds size.
Added a check to FtpState::restartable to detect when the desired
restart offset is greater than (or equal to) the size reported by
the server. If the server didn't report a size, then we won't
send/set the restart offset either.
wessels [Sat, 5 May 2007 05:46:42 +0000 (05:46 +0000)]
Fixed "spec->length >= 0" assertion for FTP request with range
An FTP request with range offset larger than the content length
would result in a negative spec->length. With this patch
we detect the condition and return a 200, instead of 206, reply.
The reply may be bogus, depending on how the FTP server behaves.
For example, the reply may have non-zero content-length header,
but no actual content (because we told the FTP server to restart
beyond the content size).
A future patch will try to make sure that we don't send a restart
command beyond the known object size.
Terminating null was written one byte past end of the buffer,
clobbering the dataWritten variable. Caused an assertion for whois
replies longer than BUFSIZ (1024) bytes.
At DebugLevel = 1, diskd prints "errors" to cache.log when operations
such as open, read, and unlink fail. These "errors" are not fatal.
In fact, some are to be expected due to the asynchronous nature of
diskd. The upper layers of Squid are designed to handle these
"errors" gracefully. They should not be in the log by default
because naive users may report them as bugs.
Bug #1940: assertion failed: store.cc:850
Avoid calling writeReplyBody when there is no reply body data to write.
FtpStateData calls processReplyBody from several places. When called from
icapAclCheckDone we may not be expecting a body for this response. If no body
is expected, the store entry may not be in a STORE_PENDING state and when we
try to append [zero-sized] body data, the store asserts.
If no body is expected, we probably should not be calling processReplyBody!
Unfortunately, it was not obvious for me what we should call so I left the
call there and made it bypass store writing if there is no body content.
If you understand the FtpStateData logic, please try to make sure
processReplyBody is not called for objects without a body (it is possible that
something else should be called instead though). Please note that ICAP sets
virginBodyDestination when body is expected.
Fix 'unknown http status code in reply' for status 408 replies
HttpStateData::cacheableReply complains about HTTP status codes
that aren't in the switch list. I've added 408 (HTTP_REQUEST_TIMEOUT)
and others that were missing.
Fix for compiling src/unlinkd.cc with kqueue and epoll.
When Squid is compiled with --enable-kqueue or --enable-epoll, we're
not supposed to use any fd_set structures. unlinkd.cc uses select()
to pause and wait for for feedback from the external unlinkd helper.
But when using kqueue or epoll, unlinkd.cc will have to use "usleep"
emulation rather than select.
The contents of lib/xusleep.c and include/xusleep.h have been copied
from Squid-2 sources.
'make check' fails with --disable-unlinkd because the Makefiles refer
to unlinkd.cc, which should not be compiled when it is disabled. Fixed
by changing references of unlinkd.cc to the $(UNLINKDSOURCE) macro.
Fixed "ftpReadTransferDone: Got code 426 after reading data" SEGV
Whenever ftpReadTransferDone got an unexpected status code, it would
also SEGV. It happened because the FtpStateData object was destroyed
in the middle of the dataRead method, just before the final call to
processReplyBody.
A workaround seems to be to call scheduleReadControlReply with
buffered_ok=0 so that the object isn't destroyed within the
same call sequence.
I was tempted to put a return after the dataComplete call in
ftpReadTransferDone so that we won't call processReplyBody
when len == 0, but I'm concerned that may break things when ICAP
is in use.
Fix "store_status == STORE_PENDING" assertion in FTP with large responses
An FTP object that exceeds reply_body_max_size leads to an assertion
failure. The StoreEntry is aborted by the call to appendSuccessHeader(),
but the abort callback is delayed due to using events. We need to
check for ENTRY_ABORTED (again) right after the appendSuccessHeader()
call -- yuck.
Author: Tsantilas Christos <chtsanti@users.sourceforge.net>
Fix compile-time issue related to removing getRaw() uses.
Tsantilas says Squid won't compile with delay pools enabled after
I removed getRaw() here. It seems to compile okay on FreeBSD (gcc
3.4.2), but it is certainly better to write "if (X == NULL)" rather
than "if (!X != NULL)".
Fix coredump bug when auth header contains invalid characters
Apologies for the previous empty commit message. Here is what
should have been there.
When a Proxy-Authorization header contains invalid characters (ie
NL or CR), BasicUser::extractUsername will emit an error message
and the username will not be set. However, extractPassword was
also being called and it does not check for funny characters in the
cleartext string. Thus passwd was being set. BasicUser::valid was
returning true because it only looked at the passwd.
This bugfix has three components:
1) BasicUser::valid now checks the username, as well as the password.
2) Moved the check for invalid characters to BasicUser::decodeCleartext,
which now returns boolean. We will not call extractUsername and
extractPassword if decodeCleartext fails.
3) extractUsername was freeing the cleartext string, but not setting
the pointer to NULL, thus causing a double-free in the BasicUser
destructor.
NOTE that the previous commit left the invalid character check in
extractUsername, but I realized it fits better in decodeCleartext
while writing the commit message, which led to the empty log.
adrian [Sat, 21 Apr 2007 19:34:42 +0000 (19:34 +0000)]
Convert most of the debug() to debugs()'s; just to see how difficult it is.
This is in preparation for converting chunks of the Http Header parsing and
management code to use C++ Strings (eventually..)
Somebody thought it would be nice to use a socketpair(), rather than
a UDP socket, for squid-pinger IPC. But socketpair() messages cannot
be larger than 4096 bytes. Pinger is sending ~8KB messages back to
Squid, so we must use UDP (or something that supports large messages).
Converted three store_swapout.cc functions to StoreEntry class methods
storeSwapOut() is now StoreEntry::swapOut()
storeSwapOutFileClose() is now StoreEntry::swapOutFileClose()
storeSwapOutAble() is now Storeentry::swapOutAble()