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()
The cache_dir read-only flag is a bit confusing. It only stops objects
from getting added to the store, it does not stop old objects from being
deleted. no-store is a more appropriate name for this option.
Add source code comments for retry-related functions in forward.cc
We have at least three functions (methods) in forward.cc that come into
play when reforwarding/retrying requests. My comments attempt to
explain these methods and when they may be called.
Bug #1935 fix: Do not retry a request after sending [some of] its body.
Replaced forgotten request_flags::body_sent with HttpRequest::bodyNibbled()
that does not need to be manually updated. The BodyPipe changes probably
deleted the code that set request_flags::body_sent.
Packaging of cppunit within the squid sources has caused more trouble
than was solved. Ease of install independently of squid combined with
the apparently widespread use of cppunit as a standard tool we feel
there is no need for it to be included.
Maintainers and source testers will need to have cppunit installed on
their systems or not use "make check".
This patch adds Linux TPROXY support to Squid 3 forward
porting 2.6 code. Based on Steven Swilton work.
The changes are verified to not break non-TPROXY use, while the
TPROXY code is still untested.
If there will be issues in the optional TPROXY code we deal with
them as bugs later on.
store_dirs_rebuilding is initialized to _1_ as a hack so that
storeDirWriteCleanLogs() doesn't try to do anything unless _all_
cache_dirs have been read. For example, without this hack, Squid
will try to write clean log files if -kparse fails (becasue it
calls fatal()).
The --enable-truncate and USE_TRUNCATE code has been removed.
As discussed in Bug 1371, the truncate feature is dangerous on
async disk I/O storage schemes and more troublesome than any
possible performance benefits.
Fixed SEGV during reconfigure due to NULL pointer bug in eventDelete()
Since the cancel method may now delete multiple events (when
arg is NULL) it no longer returns after a deletion and
we have a potential NULL pointer problem. If we just
deleted the last event in the list then *E is now equal
to NULL. We need to break here or else we'll get a NULL
pointer dereference in the last clause of the for loop.
A fix for the !flags.write_draining assertion. Due to the way diskd
works, drainWriteQueue() could get called twice in the stack. If so,
just return when the write_draining flag is set.
Comment out some debugging messages that are printed before cache_log
is initialized because we see them on stderr for commands like
squid -k reconfigure.
Fixed disk file leak. UFS-based disk files were not always closed
because the storeClose() may be called when an I/O request is pending.
With this fix, UFSStoreState::writeCompleted now calls tryClosing()
if the try_closing flag is set.
The DiskdFile::close function calls the DiskdIOStrategy::send function and
consequently the DiskdIOStrategy::SEND function, with shm_offset value of -1.
If the msgsnd in SEND fails, the SharedMemory::put function is called with
offset=-1, triggering an abort.
Patch by Christos Tsantilas. Reviewed by Guido Serassio and Duane Wessels.
This change should fix bug #1837: Segfault on configuration error
When quitting on a fatal error, such as a configuration error, Squid may need
to write clean state/log files. Squid uses comm_ routines to do so. Thus, we
must initialize comm_ before such fatal errors are discovered.
Perhaps a better fix would be to avoid writing clean state/log files until
the old ones become dirty?
This change should fix bug #1898: "assertion failed: tunnel.cc:372"
The assertion fails in SslStateData::Connection::dataSent method which is
called by SslStateData::writeClientDone and SslStateData::writeServerDone
methods.
Both write*Done methods did not check for the comm_err_t flag.
When the SSL side closes the connection after a successful but partial write,
write*Done is called with the len argument equal to the number of written
bytes, which is positive but less than the size of the given data for write,
and the dataSent() assertion "amount == (size_t)len" fails.
Server transactions keep refcounted pointer to FwdState that launched them.
Server transactions use the pointer to call FwdState upon completion or
abnormal termination. If nobody else points to the FwdState object after the
transaction is done, the object gets destroyed, even if there are other
servers to be tried.
To avoid FwdState destruction when selecting the next server, FwdState must
keep a refcounted pointer to self. That pointer should be created before the
server-selection loop and cleared after the loop. Currently, we create the
pointer when FwdState is created (which is earlier than needed) and clear it
in various places (which may be late and is probably buggy).
cache.log was being created as root, then would later fail to open
with 'Permission denied'. We were calling leave_suid() after parsing
the config, but before calling configDoConfigure().
We must call configDoConfigure() before leave_suid() because
configDoConfigure() is where we turn username strings into
uid values.
Since I changed the semantics of store_dirs_rebuilding yesterday (to match
how it works in Squid-2 again), I forgot that storeRebuildComplete()
probably wouldn't do any work as long as store_dirs_rebuilding was greater
than zero. Now it needs to do its work when store_dirs_rebuilding == 1.
- Bug #1356: Close open DNS sockets when doing idnsShutdown. Leaving them
open may result in an idnsVCClosed callback being called when the name
servers array have been freed by idnsFreeNameservers.
(based on Christos Tsantilas and Guido Serassio work)
store_dirs_rebuilding is initialized to _1_ as a hack so that
storeDirWriteCleanLogs() doesn't try to do anything unless _all_
cache_dirs have been read. For example, without this hack, Squid
will try to write clean log files if -kparse fails (becasue it
calls fatal()).
The diskd helper passes errno in responses. With this patch, errno
is now set in the main squid process after processing a diskd
response.
Initialize unused fields of diomsg in messages from diskd to squid.
Previously, the diskd helper did not initialize the unused fields
of diomsg. This was annoying when looking at debugging output
and trying to match requests with responses. This patch initializes
the unused fields.