Amos Jeffries [Sat, 9 Feb 2013 00:44:07 +0000 (17:44 -0700)]
Bug 3686: cache_dir max-size default fails
If some cache_dir are configured with max-size and some not the default
maximum_object_size limit fails.
This refactors the max-size management code such that each SwapDir always
has a value maxObjectSize(). This value is calculated from the SwapDir
local setting or global limit as appropriate.
The global maximum_object_size directive is migrated to simply be a default
for cache_dir max-size= option.
The global store_maxobjsize variable is altered to be the overall global
limit on how big an object may be cache by this proxy. It now takes into
account the max-size for all cache_dir and cache_mem limitation.
NP: The slow accumulation of these and earlier changes means Squid no
longer immediately caches unknown-length objects. The unit-tests are
therefore changed to test using explicit 0-length objects to ensure the
test is on a cached object not bypassing the apparently ested logic.
They are also provided with a large global store_maxobjsize limit in order
to do a weak test of the SwapDir types max-size in the presence of other
larger cache_dir or maximum_object_size settings.
Alex Rousskov [Fri, 8 Feb 2013 04:14:50 +0000 (21:14 -0700)]
Make squid -z for cache_dir rock work like UFS instead of like COSS.
Polish -z documentation and cache.log reporting.
When a startup script runs squid -z by mistake against a cache_dir that is
already initialized and full of cached entries, some admins prefer that
nothing happens. Rock store now skips reinitialization if both the cache_dir
directory and the db file in that directory exist. If one or both are missing,
the missing pieces are created.
UFS does something similar because it creates missing L1 and L2 directories
but does not erase any entries already present in the cache_dir path. COSS,
OTOH, re-initializes the existing db. Rock behavior will now be closer to UFS.
To clean a corrupted cache_dir, the admin must remove its top-level directory
before running squid -z.
Squid now logs "Creating missing swap directories" instead of "Creating Swap
Directories", and our documentation now reflects the "if missing" part of the
-z algorithm.
Also documented that recent Squid versions run -z in daemon mode (so that SMP
configuration macros continue to work).
Amos Jeffries [Tue, 5 Feb 2013 07:47:28 +0000 (20:47 +1300)]
Migrate HttpMsg to Lock refcounting
First stage of the conversion to RefCount<>. This replaces the custom
attempt at ref-count locking in HttpMsg with the locking mechanism
presented by class Lock.
Alex Rousskov [Mon, 4 Feb 2013 17:39:23 +0000 (10:39 -0700)]
Bug 3752: objects that cannot be cached in memory are not cached on disk
if cache_dir max-size is used.
This fix contains four related changes:
1) When fixing "trimMemory for unswappable objects" (trunk r11969), we
replaced swapoutPossible() with swappingOut()||mayStartSwapOut() but missed
the fact that swapoutPossible() had "possible now" semantics while
mayStartSwapOut() has "may start now or in the future" semantics. When all
cache_dirs had max-size set, mayStartSwapOut() returned false for objects of
unknown size and even for smaller-than-maximum but not-yet-received objects,
despite the fact that those objects could be swapped out later.
That false mayStartSwapOut() result allowed maybeTrimMemory() to trim those
objects memory and mark the objects for release, preventing their subsequent
disk caching.
2) To fix (1) above, mayStartSwapOut() had to return true for not-yet-received
objects of unknown size. However, returning true is correct only if no
subsequent check can return false. Thus, we had to move all lower/later checks
that could return false up, placing them before the maximum-of-all-max-sizes
check.
3) Once (2) was done, the end of mayStartSwapOut() had (a) a loop that could
return true while setting decision to MemObject::SwapOut::swPossible and (b)
an unconditional code that did ... the same thing. Thus, the loop could no
longer change the method outcome. The loop also had a lot of doubts and XXXs
attached to it. We removed it. If that loop is needed, it is needed and must
be resurrected elsewhere.
4) Since mayStartSwapOut() returns true if swapout is possible in the future
(but not necessarily now), we cannot rely on its return value to initiate
swapout code. We need to test whether swapout.decision is swPossible instead.
Amos Jeffries [Sun, 3 Feb 2013 08:49:00 +0000 (01:49 -0700)]
Fix lots of SNMP alignment, sizeof(), and memory uninitialized erorrs
They all stem from a few uses of "sizeof(name)" where name is a pointer
to an OID object rather than the object itself. This code is present as
far back as squid-2.6 and probably a lot further.
It appears to not have been causing obvious problems due to the
sizeof(oid*) [4 or 8 octets] on a pointer being larger than the
sizeof(oid) [1 or 2 octets] on the object stored into the allocated
memory.
Amos Jeffries [Fri, 1 Feb 2013 06:49:25 +0000 (23:49 -0700)]
Bug 3515: crash in FtpStateData::ftpTimeout
Since revision squid-3.2-11174 restructured teh TCO connection handling
in FTP the data connection has had separate 'opener' connection.
Meaning the conn->fd state is always NULL when the setup times out.
This cleans up the data connection opener state and allows the error
message generation to take place.
Alex Rousskov [Fri, 1 Feb 2013 00:28:16 +0000 (17:28 -0700)]
Fixed several ConnOpener problems
by relying on AsyncJob protections and comm_close(), while maintaining a
tighter grip on various I/O and sleep states.
Problems addressed:
* Connection descriptor was not closed when attempting to reconnect after
failures. We now properly close on failures, sleep with descriptor closed,
and then reopen.
* Timeout handler was not cleaned up properly in some cases, causing memory
leaks (for the handler Pointer) and possibly timeouts that were fired (for
then-active handler) after the connection was passed to the initiator.
* Comm close handler was not cleaned up properly.
* statCounter.syscalls.sock.closes counter was not updated on FD closure.
* Waiting pending accepts were not kicked on FD closure.
* Connection timeout was enforced for each connection attempt instead of
applying to all attempts taken together.
and possibly other problems. The full extent of all side-effects of mishandled
race conditions and state conflicts is probably unknown.
There are cases where the generated certificates do not mimic enough properties
and secure connection with the client fails. For example, Squid does not mimic
Key Usage extensions. Clients using GnuTLS (or similar libraries that validate
server certificate using those extensions) fail to secure the connection with
Squid.
This patch add mimicking for the following extensions, which are considered
as safe to mimic:
* X509v3 Key Usage
* X509v3 Extended Key Usage,
* X509v3 Basic Constraints CA.
We would be happy to add more "safe to mimic" extensions if users request (and
vouch for) them.
Currently, dst, dstdom, dstdom_regex (and other?) DNS-related ACLs do DNS
lookups if such a lookup is needed to convert an IP address into a domain name
or vice versa. This creates two kinds of problems:
- It is difficult to identify requests that use raw IP addresses in Request-URI
or Host headers. One would have to use something like url_regex and possibly
req_header to identify those before using dst ACLs to match the request
destination against a known IP subnet. IPv6 would only make this harder.
- It is difficult to use dst* ACLs in options that support fast ACLs only.
If an async lookup is required, the answer will be unpredictable (now)
or DUNNO (when the ACL bugs are fixed), possibly with warnings and other
complications.
This patch adds a -n option to dst, dstdom, dstdom_regex and other DNS-related
ACLs. The option disable lookups and address type conversions. If lookup or
conversion is required because the parameter type (IP or domain name) does not
match the message address type (domain name or IP), then the ACL with a -n
option would immediately declare a mismatch without any warnings or lookups.
The "--" option can be used to stop processing all options, in the case the
first acl value has '-' character as first character (for example the '-' is
a valid domain name)
For example:
# Matches requests with full URI host set to localhost
# but not requests with full URI host set to 127.0.0.1
acl toLocalRawName dstdom -n localhost
http_access allow toLocalRawName
# Use -- option to stop processing flags
acl AnACL dst_domain -n -- -cream-and-sugar.tumblr.com
# Matches requests with full URI host set to 127.0.0.1
# but not requests with full URI host set to localhost
acl toLocalRawIp dst -n 127.0.0.1/32
cache_peer_access peer1 allow toLocalRawIp
Please note that -n prohibits lookups in Squid's DNS caches as well.
Tianyin Xu [Fri, 25 Jan 2013 01:26:21 +0000 (18:26 -0700)]
Polish squid.conf parsing validation
* Updates ato*() functiosn to safer xato*() alternatives provided by
the Squid compat library. Along with error messages on invalid
configuration values detected by these.
* Add protection against integer overflow on most options
* Add parse deprecation messages on enable/disable for boolean and
and trilean options.
* Add 'wrong-value' error messages on most options.
Alex Rousskov [Thu, 24 Jan 2013 16:22:45 +0000 (09:22 -0700)]
Bug 3111: Mid-term fix for the forward.cc "err" assertion.
The assert is triggered when a close handler for the server connection
destroys FwdState before we have received anything from the origin server.
Instead of asserting, we now respond with a 502 (Bad Gateway) ERR_READ_ERROR.
That error seems the most appropriate single choice among available ones, but
it may be misleading (in access.log) when the close handler was called due to
client problems. Hopefully, another error will be logged in most of those
cases.
Propagate pinned connection persistency and closures to the client.
Squid was trying hard to forward a request after pinned connection failures
because some of those failures were benign pconn races. That meant re-pinning
failed connections. After a few iterations to correctly handle non-idempotent
requests, the code appeared to work, but the final design, with all the added
complexity and related dangers was deemed inferior to the approach we use now.
Squid now simply propagates connection closures (including pconn races) to the
client. It is now the client responsibility not to send non-idempotent requests
on idle persistent connections and to recover from pconn races.
Squid also propagates HTTP connection persistency indicators from client to
server and back, to make client job feasible. Squid will send Connection:close
and will close the client connection if the pinned server says so, even if
Squid could still maintain a persistent connection with the client.
These changes are not mean to affect regular (not pinned) transactions.
In access.log, one can detect requests that were not responded to (due to race
conditions on pinned connections) by searching for ERR_ZERO_SIZE_OBJECT
%err_code with TCP_MISS/000 status and zero response bytes.
Amos Jeffries [Tue, 22 Jan 2013 11:33:57 +0000 (00:33 +1300)]
Bug 3676: Fix Shadowed variables
This resolves a number more shadowed variables found by the more strict
compilers in the layer-02-maximus optional components.
There should be no logic changes in this patch.
Amos Jeffries [Wed, 16 Jan 2013 11:49:51 +0000 (00:49 +1300)]
squidpurge: Polish details of what filename is generated from
* change strlen(url) to strlen(ptrt) since the string under ptr is being
added to filename, not the whole URL.
* also add a few extra magic bytes to make it clear that there is space
for delimiters and termination bytes. These were previously hidden as
extra bytes in the URL prefix length.
Should resolve Coverity Scan false positive issue 740414
Amos Jeffries [Wed, 16 Jan 2013 11:22:25 +0000 (00:22 +1300)]
squidpurge: display friendly errors on missing command line options
Currently the tool will crash with a segmentation fault if any one of
several command switches which are expected to have a mandatory argument
are in fact followed by nothing.
Detect these cases and display a message about what is missing.
Amos Jeffries [Wed, 16 Jan 2013 07:30:47 +0000 (20:30 +1300)]
Fix ConnOpener IPv6 awareness
When updating IPv6 support for split-stack one USE_IPV6 wrapper was
omitted conversion to the EnabledIpv6 stack auto-detect mechanism.
This resulted in IPv6 addresses being mis-converted on split-stack
systems or with IPv6 disabled in the kernel and --enable-ipv6 built.
The visible symptom is "assert(FALSE)" in Ip::Address::GetAddrInfo().
Amos Jeffries [Tue, 15 Jan 2013 01:01:30 +0000 (18:01 -0700)]
Initialize mem_node fully
Experience in squid-2.7 shows that with MemPools use of malloc instead
of calloc mem_node requires full initialization in order to get accurate
memory accounting.
Amos Jeffries [Mon, 14 Jan 2013 05:01:04 +0000 (22:01 -0700)]
MemPools: remove zero on allocate (calloc) from some pool types
As we are closing defects identified by Coverity and improving
constructors everywhere we are creating a minor anti-pattern in
MemPool'ed objects with calloc() in the pool initializing the memory
then constructors re-initializing it in a better way for that object.
MemPools contains a doZeroOnPush flag to optimize performance by removing
use of memset() as chunks are added back into the pool. However, on
closer inspection it is clear that the following pop() process to re-use
those chunks is never performing memset() anyway. As such I believe that
there is no special need to use calloc() on these particular object types
in the first place.
Update MemPools to use malloc() instead of calloc() on all types with
doZeroOnPush set. This should increase performance a little, and allows
us to remove the anti-pattern by setting doZeroOnPush for more objects
as we can verify they are correctly initialized by their constructors.
Alex Rousskov [Sat, 12 Jan 2013 18:34:10 +0000 (11:34 -0700)]
Fix "address.GetPort() != 0" assertion for helpers on FreeBSD (at least).
The order (or set of?) #include files used by src/ssl/helper.cc (and
probably by other helper source files) has changed recently, exposing a
defines.h dependency on sys/socket.h where that system header is
required to define AF_UNIX. With AF_UNIX incorrectly undefined,
IPC_STREAM was set to IPC_TCP_SOCKET instead of IPC_UNIX_STREAM, and
helpers that do not have a notion of a listening port, were trying to
create communication sockets using TCP streams, triggering a "must have
a port" assertion in comm_connect_addr() called from ipcCreate().
TODO: Moving IPC_* defines into an IPC-specific header file may be a
better solution then exposing all defines.h users to sys/socket.h.
Amos Jeffries [Fri, 11 Jan 2013 05:05:52 +0000 (22:05 -0700)]
WCCP: Fix memory leak in mask assignment, improve debuggsing.
* Release temporary weight array allocated on each HERE_I_AM packet sent
by Squid. For mask assignment these were not released properly and may
build up to a significant size of memory over time.
* Add debug traces to send() events to report failures sending packets
* Also, on HERE_I_AM event send() failure, reduce the timeout to 2sec
for the retry in a crude attempt to prevent router state flapping.
* Silence compiler warnings on use of connect() to disconnect a socket.
Inconsistent OS behaviour makes the result useless in this case.
Amos Jeffries [Tue, 8 Jan 2013 23:51:02 +0000 (16:51 -0700)]
Bug 3731: use 'int' on all systems settign TOS value.
FreeBSD is confirmed erroring out on 8-bit variable size. Other BSD are
documented in a way that implies they do as well, although not at this
stage confirmed to be failing.
Linux seems to be the only confirmed system workign with 8-bit size sent
to setsockopt(). So we revert this to 'int' (32-bit or 64-bit) as was
working in Squid 3.1.
projects / thirdparty / squid.git / log
