On 09/07/2011 05:03 AM, Amos Jeffries wrote:
On Tue, 06 Sep 2011 19:04:29 +0300, Tsantilas Christos wrote:
I am sending a new patch which does not touch the current ">la"
formating code but instead adjust the "la" formatting code to follow
rules as discussed by Amos.
Please look in the cf.data.per documentation and the
LFT_CLIENT_LOCAL_USED_IP/LFT_CLIENT_LOCAL_USED_PORT enum names I am
using...
* LFT_LOCAL_LISTENING_ would be a bit more descriptive for the enum. The
documentation could also use the word "listening" to be clearer on where
it comes from.
ie:
+ la Local listening IP address the client connection uses.
+ lp Local listening port number the client connection uses.
I have my doubts that the listening IP address is what we want to log here.
Normally an administrator needs to now the IP address the client
connected to. It needs the IP address and not listening address.
In the case of intercepted connections this information does not exist
so why not get this info from listening IP address?
Moreover it is practical to have ONE formating code for the above (eg
easier to create a script for log analysis)
* The IP address display needs a re-structure. al->tcpClient is possibly
a backup source on non-intercepted requests if cache.port->s.IsAnyAddr()
[only this case]. Otherwise always use cache.port->s or dash.
+ case LFT_LOCAL_LISTENING_IP:
+ if (al->cache.port == NULL) {
+ break; // dash
+ } else if (al->cache.port->s.IsAnyAddr()) {
+ if (al->tcpClient != NULL &&
+ !(al->request != NULL && (al->request->flags.spoof_client_ip ||
al->request->flags.intercepted))) {
+ out = al->tcpClient->local.NtoA(tmp, sizeof(tmp));
+ } // else dash.
+ } else {
+ out = al->cache.port->s.NtoA(tmp, sizeof(tmp));
+ }
+ break;
My original code was not good, I agree...
I have in my mind something like this:
case LFT_LOCAL_LISTENING_IP:
+ if ((al->request->flags.spoof_client_ip ||
al->request->flags.intercepted) && al->cache.port) {
+ if (!al->cache.port->s.IsAnyAddr())
+ out = al->cache.port->s.NtoA(tmp, sizeof(tmp));
+ } else if (al->tcpClient != NULL)
+ out = al->tcpClient->local.NtoA(tmp, sizeof(tmp));
+ break;
If it is an intercepted connection use the listening address (if it is
available), else use the IP address used by the client to connect to squid.
Is it OK?
* Drop all use of al->tcpClient and traffic types out of the port
display code. To match the documentation cache.port->s should always be
set when cache.port != NULL. If its missing from a particular path that
needs to be known why (internally generated requests would be 0, others
should not ever be).
+ case LFT_LOCAL_LISTENING_PORT:
+ if (al->cache.port) {
+ outint = al->cache.port->s.GetPort();
+ doint = 1;
+ }
+ break;
You have right here...
* Drop the deprecation warning added for *_OLD_31 entirely now. That
will prevent the altered codes having any effect.
Amos
On 09/01/2011 12:08 AM, Amos Jeffries wrote:
%>la to always display tcpClient->local with a config documentation note
about it being external IPs in intercepted traffic.
%la to display cache.caddr with a config documentation note that it is
the squid receiving *_port details as known by Squid (caddr also used by
icp_port and htcp_port on their messages).
Amos
