gmane.comp.web.squid.devel

Build failed in Jenkins: 3.HEAD-amd64-FreeBSD-9.0-clang #278

2013-06-19T06:28:03

See <http://build.squid-cache.org/job/3.HEAD-amd64-FreeBSD-9.0-clang/278/changes>

Changes:

[Amos Jeffries] Cleanup: update several CBDATA classes

Removes several cbdataAlloc/cbdataFree by updating classes to use
CBDATA_CLASS2 macro..

------------------------------------------
[...truncated 17833 lines...]
mv -f .deps/Pages.Tpo .deps/Pages.Plo
/bin/sh ../../libtool  --tag=CXX   --mode=compile clang++ -DHAVE_CONFIG_H -DDEFAULT_STATEDIR=\"/usr<http://build.squid-cache.org/job/3.HEAD-amd64-FreeBSD-9.0-clang/ws/btlayer-02-maximus/squid-3.HEAD-BZR/_inst/var/run/squid\">  -I../../.. -I../../../include -I../../../lib  -I../../../src -I../../include  -I/usr/local/include -I/usr/include  -I/usr/include -I../../../libltdl  -I/usr/include  -I/usr/include -Werror -Wno-error=parentheses-equality -Qunused-arguments  -D_REENTRANT -g -O2 -MT PageStack.lo -MD -MP -MF .deps/PageStack.Tpo -c -o PageStack.lo `test -f 'mem/PageStack.cc' || echo '../../../src/ipc/'`mem/PageStack.cc
libtool: compile:  clang++ -DHAVE_CONFIG_H -D

Build failed in Jenkins: 3.HEAD-amd64-FreeBSD-9.0-clang #277

2013-06-19T03:37:22

See <http://build.squid-cache.org/job/3.HEAD-amd64-FreeBSD-9.0-clang/277/changes>

Changes:

[Amos Jeffries] Add Master Transaction class

... to store and propigate the shared state used end-to-end through Squid
for logging or server-side component input. This excludes Job and Call
pointers, but does include any 'factual' data regarding the traansaction.

[Alex Rousskov] Make sure %<tt includes all [failed] connection attempts.

The old code was using zero n_tries to detect the first connection attempt,
but n_tries is not incremented when we are opening a new connection rather
than reusing an old one. Perhaps n_tries should be updated differently as
well, but this change simply makes %<tt (hier.total_response_time) management
independent from that [complex] counter.

------------------------------------------
[...truncated 17840 lines...]
mv -f .deps/Pages.Tpo .deps/Pages.Plo
/bin/sh ../../libtool  --tag=CXX   --mode=compile clang++ -DHAVE_CONFIG_H -DDEFAULT_STATEDIR=\"/usr<http://build.squid-cache.org/job/

Re: [PATCH] Master transaction state object

Amos Jeffries — 2013-06-19T02:04:59

Done and applied as trunk rev.12915.

Amos

Re: [PATCH] Master transaction state object

Alex Rousskov — 2013-06-18T17:05:49


Hi Amos,

    I am glad we reached an agreement on limiting exposure to master
transaction. My only remaining concerns is related to the new class
scope/description:


I would remove the reference to "client transaction" from the beginning
of the class description because internal requests may not have an
associated client transaction.



The current description does not explain what protocols are eligible for
creating master transactions. I think we should document your intent to
include ICP and SNMP requests here. It is an important design decision.

Finally, the "all the state generated" part is a lie. Certainly, we do
not and are not going to store ALL the state here. I believe you have
indicated previously that you want to store historical information worth
logging (rather than providing access to current master transaction jobs
and their state). I think that intention was the right one (at least as
a starting point).


To put it all together, consider something like this (with proper
formatting, of c

Build failed in Jenkins: 3.HEAD-amd64-FreeBSD-9.0-clang #276

2013-06-18T16:30:03

See <http://build.squid-cache.org/job/3.HEAD-amd64-FreeBSD-9.0-clang/276/changes>

Changes:

[Christos Tsantilas] Sending root certificate for validation

This patch modify squid cert validation subsystem to sent to cert validator
helper the complete certificates chain, not only the certificates sent by
web server. This is may not be possible in all cases, for example  in cases
where the root certificate is not stored localy.

Also this patch includes a small optimization, it checks for domain mismatch
error only when the checked (current) certificate is the server certificate.

This is a Measurement Factory project

[Christos Tsantilas] Deprecate log_icap and log_access configuration directives

The log_icap and log_access are not really needed to control requests logging.
Someone can use acls  with access_log and icap_log configuration directives
for this purpose.
Also currently the requests denied for logging using the log_access access list
will not be accounted for in performance counters.

This patch:

Re: [PATCΗ] Quoted values in squid.conf

Alex Rousskov — 2013-06-18T16:20:10


parameters() or options() seems better to me because the code actually
loads a squid.conf directive parameters. It cannot load directives
themselves this way (we have an "import" keyword for that).

I think I prefer parameters() because the current squid.conf text uses
"option" instead of "directive" in many places. "Parameters" would not
add to that confusion.


HTH,

Alex.

Jenkins build is back to normal : 3.HEAD-amd64-FreeBSD-7.2 #1891

2013-06-18T12:29:35

See <http://build.squid-cache.org/job/3.HEAD-amd64-FreeBSD-7.2/1891/changes>

Re: [PATCH] Sending root certificate for validation

Tsantilas Christos — 2013-06-18T11:28:59


On 06/18/2013 12:53 PM, Amos Jeffries wrote:

The patch applied to trunk....


For a stupid reason, this is looks very difficult (header dependencies
or other)...
I will try it, but I have my doubts that it worth it...

Re: [RFC] Annotation quoting

Tsantilas Christos — 2013-06-18T10:35:56

Please see my comment bellow ("Currently the logging code when needs...")


Yep. The quoting just added to log the anotation. But currently looks
that it is done with a bad way..


Assume we have thw following anotations:
  X-Name: test
  X-Name: a,sec-test

The  %'{X-Name}note  will print:
  test, "a,sec-test"

(Note the space after first comma)

The %"{X-Name}note  will print
  test, \"a,sec-test\"

The %[{X-Name}note will print:
 test, "a,sec-test"

The %#{X-Name}note will print
test,%20%22a,sec-test%22


The delimiter can be used for ecamples in cases there is coma in values.
For example
  X-Name: test
  X-Name: a,sec-test

We may want to use %{X-Name:;}note to print:
   test;a,sec-test

Re: [PATCH] Sending root certificate for validation

Amos Jeffries — 2013-06-18T09:53:10

Yes that will do.

Amos

Re: [RFC] Annotation quoting

Amos Jeffries — 2013-06-18T09:50:26

Where are those %22 characters coming from?
  They are not part of HTTP/ICAP header syntax, and quoting shoudl be 
stripped by the helper on receipt. So this is something special being 
added somewhere along the line.


Test the logformat quoting syntax is working and inform the customers 
how to use it to customize quoting.

     %'{X-Name:;}note %"{X-Name:;}note %[{X-Name:;}note %#{X-Name:;}note

Which of the four above tokens displays the log field the way they want?



Delimiter is not up for questino is it? the quoting syntax is the 
problem, and the logformat definition contains flexible quoting codecs.

Amos

Re: [PATCΗ] Quoted values in squid.conf

Tsantilas Christos — 2013-06-18T09:42:24

This discussion is also frozen...

Alex and Kinki suggest the function style, and looks that is a good
choice and there are very good reasons for doing this.

Are we OK to implement it like this?

< at >Alex
  What are you suggesting file("location") or parameters("location")?



On 06/04/2013 05:55 PM, Alex Rousskov wrote:

[RFC] Annotation quoting

Tsantilas Christos — 2013-06-18T08:21:24

Some customers complaining that their annotations (set by url_rewriter)
look wrong when logged to access.log. Here are a few examples of logged
annotations:
    %22-%22
    %22Default_Google%22
    %22pg13,k12%22


Currently the logging code when needs to log an annotation:
 1) first check for each note value if quotation needed (it uses the
ConfigParser::QuoteString, so it adds quotes if any non alphanumeric
char exist in string)
 2) then apply logformat quoting . This is means by default just do a
url encoding before print.


If we just remove the default logging quote, the values will be printed
inside quotes (""), but if the user wants to change quoting style, will
have the same problems.
If we remove quotes from values we may have problems, because
annotations may include comas (',') or spaces which will confuse logging.


Looks that we have the following choices to fix this:

1. Do nothing. Claim that we do not support values with commas even
though they can be passed to Squid (and Squid will correctl

Re: [PATCH] Sending root certificate for validation

Tsantilas Christos — 2013-06-18T07:29:43

This patch forgotten.

I am suggesting:
 - apply the patch as is
 - With a second patch move the SSL indexes from global.h to support.h

Is it OK?


On 06/10/2013 07:31 PM, Tsantilas Christos wrote:

Re: [PATCH] Master transaction state object

Amos Jeffries — 2013-06-17T16:52:44

Oh rats. mk3 attached.

Hopefully you will have time to review this today. I have incorporated 
your latest items (elided) in this as the intended final version.



The fact that the CfgPortPointer is a CbcPointer is unfortunate factor 
of its use elsewhere in the code. If we can make it a RefCount that 
would be good, but a separate patch. All I'm aiming at here is to ensure 
that the available Pointer is used consistently by the new code ready 
for that fix.



Quite. I'm having second thoughts about my initial reasons - 
particularly that the Jobs or state objects which are receiving those 
close and I/O calls likely have a local referance to the Xaction anyway. 
This mk3 removes it from those CommCall until such time as it becomes 
necessary to add there.


I was thinking more along the lines of error counters. But that is more 
global in scope anyway, or can be added at the other end of the Call 
(handler) where the Xaction is probably available anyway.





I don't mean it to be universally accepted.

Re: [PATCH] Master transaction state object

Alex Rousskov — 2013-06-17T15:33:40

Thank you for addressing my concerns related to pipelining changes.

Please note that the mk2 patch is missing the new MasterXaction class
being added. The notes below are for the changes actually present in the
patch.



Please remove a whitespace change.



Please use doxygen /// comments to describe class data members.



Do we have to kill worker when the port becomes invalid while the accept
call is being propagated? If yes, please do that in httpAccept() as
well. If not, please handle the case of an
invalidated-while-we-were-waiting port more gracefully.

BTW, is is probably a bad idea to use cbdata-protected PortCfg. Ideally,
when reconfiguration changes the port, it should not be invalidated,
causing all sorts of assertions for pending transactions using that
port. We should use reference counting instead (with some additional API
to mark no-longer-configured ports), but that change is outside your
project scope, of course.



Storing a master transaction pointer may be appropriate for
AcceptCbParam

Re: [PATCH] https_port interception options

Alex Rousskov — 2013-06-17T14:29:25


Yes, but that does not contradict what I said: The capability to proxy
without bumping is there, but there is no good configuration support for
it (because a combination of ssl-bump and "ssl_bump none" for port
traffic is currently required). We should not change configuration
support in a way that will prevent us from providing a good way to
nicely enable that existing capability in the future. Making ssl_bump
implicit is such a change.



Implicitly enabling ssl_bump is the opposite of what we should do in
this case.



We need to change code so that when an https_port does not have an
ssl-bump option, the traffic received on that port acts as if "ssl-bump
none" was configured for it (i.e., the traffic is forwarded without
bumping). After those changes, SslBump will continue to be disabled for
that port, but the forwarding-only configuration will be allowed.


HTH,

Alex.

Re: [PATCH] Deprecate log_icap and log_access configuration directives

Tsantilas Christos — 2013-06-17T14:03:05

I will apply the latest patch if there is not any objection



On 06/16/2013 07:27 AM, Amos Jeffries wrote:

Re: [PATCH] https_port interception options

Amos Jeffries — 2013-06-17T06:10:12

Okay, yes it would be.



No. Please look at the if-statements which this patch is altering again.

         if (hijacked && !s->flags.tunnelSslBumping) {
             ...
             self_destruct();
         }

There is no way to configure working interception for 3.3+ on https_port 
without "ssl-bump" flag being present as well.
Bit of a problem.


The intention of this patch was to make your "Furthermore," argument 
case actually work.

You have shown why it wont work (ssl_bump access list checks also halt 
the proxy, and more reliance on the ssl-bump hack). But we still need 
something to do that.

Amos

Re: [PATCH] Master transaction state object

Amos Jeffries — 2013-06-17T05:50:08

If there are no further objections I would like to apply a slightly 
tweaked version of this patch and begin work on the next stages.

By "tweak" I mean I have now shuffled CommCalls xaction to 
CommAcceptCbParams as requested earlier.

Amos

Re: [PATCH] Deprecate log_icap and log_access configuration directives

Amos Jeffries — 2013-06-16T04:27:07

+1.

Amos