[squid-dev] Build failed in Jenkins: trunk-full-matrix ยป clang,d-debian-unstable #183

2016-06-15 Thread noc
See 


--
[...truncated 6854 lines...]
make[5]: Leaving directory 
'
Making dvi in NIS
make[5]: Entering directory 
'
make[5]: Nothing to be done for 'dvi'.
make[5]: Leaving directory 
'
Making dvi in POP3
make[5]: Entering directory 
'
make[5]: Nothing to be done for 'dvi'.
make[5]: Leaving directory 
'
Making dvi in RADIUS
make[5]: Entering directory 
'
make[5]: Nothing to be done for 'dvi'.
make[5]: Leaving directory 
'
Making dvi in SMB
make[5]: Entering directory 
'
make[5]: Nothing to be done for 'dvi'.
make[5]: Leaving directory 
'
Making dvi in fake
make[5]: Entering directory 
'
make[5]: Nothing to be done for 'dvi'.
make[5]: Leaving directory 
'
Making dvi in getpwnam
make[5]: Entering directory 
'
make[5]: Nothing to be done for 'dvi'.
make[5]: Leaving directory 
'
make[5]: Entering directory 
'
make[5]: Nothing to be done for 'dvi-am'.
make[5]: Leaving directory 
'
make[4]: Leaving directory 
'
Making dvi in digest
make[4]: Entering directory 
'
Making dvi in file
make[5]: Entering directory 
'
make[5]: Nothing to be done for 'dvi'.
make[5]: Leaving directory 
'
make[5]: Entering directory 
'
make[5]: Nothing to be done for 'dvi-am'.
make[5]: Leaving directory 
'
make[4]: Leaving directory 
'
Making dvi in negotiate
make[4]: Entering directory 

Re: [squid-dev] [PATCH] Do not hide important/critical messages

2016-06-15 Thread Eliezer Croitoru
Thanks!

Eliezer


Eliezer Croitoru
Linux System Administrator
Mobile: +972-5-28704261
Email: elie...@ngtech.co.il


-Original Message-
From: squid-dev [mailto:squid-dev-boun...@lists.squid-cache.org] On Behalf Of 
Amos Jeffries
Sent: Thursday, June 16, 2016 1:24 AM
To: squid-dev@lists.squid-cache.org
Subject: Re: [squid-dev] [PATCH] Do not hide important/critical messages

On 12/04/2016 2:59 a.m., Alex Rousskov wrote:
> On 04/09/2016 10:42 PM, Amos Jeffries wrote:
>> On 29/03/2016 12:44 p.m., Alex Rousskov wrote:
>>> unpatched Squid console only says:
>>>
>>>   2016/03/27 14:19:48.297| SECURITY ALERT: By user agent:
>>>   2016/03/27 14:19:48.297| SECURITY ALERT: on URL: dut70.test:443
>>>
>>> A patched Squid produces the expected three console lines:
>>>
>>>   2016/03/27 15:25:42| SECURITY ALERT: Host header forgery detected...
>>>   2016/03/27 15:25:42| SECURITY ALERT: By user agent:
>>>   2016/03/27 15:25:42| SECURITY ALERT: on URL: dut70.test:443
> 
>>> If this v3.5 patch is accepted in principle, I hope somebody 
>>> volunteers a trunk port (which should not be difficult).
> 
> 
>> This appears to be just a polished implementation of the intended
>> debugs() original design. So in principle its already acceptible.
> 
>> To get into v3.5 it does need to go in through v4 first though.
> 
> 

Now that the v4 port is done and working. Iv've applied this to 3.5 as 
rev.14058.

Amos


___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


Re: [squid-dev] [PATCH] Do not hide important/critical messages

2016-06-15 Thread Amos Jeffries
On 12/04/2016 2:59 a.m., Alex Rousskov wrote:
> On 04/09/2016 10:42 PM, Amos Jeffries wrote:
>> On 29/03/2016 12:44 p.m., Alex Rousskov wrote:
>>> unpatched Squid console only says:
>>>
>>>   2016/03/27 14:19:48.297| SECURITY ALERT: By user agent:
>>>   2016/03/27 14:19:48.297| SECURITY ALERT: on URL: dut70.test:443
>>>
>>> A patched Squid produces the expected three console lines:
>>>
>>>   2016/03/27 15:25:42| SECURITY ALERT: Host header forgery detected...
>>>   2016/03/27 15:25:42| SECURITY ALERT: By user agent:
>>>   2016/03/27 15:25:42| SECURITY ALERT: on URL: dut70.test:443
> 
>>> If this v3.5 patch is accepted in principle, I hope somebody volunteers
>>> a trunk port (which should not be difficult).
> 
> 
>> This appears to be just a polished implementation of the intended
>> debugs() original design. So in principle its already acceptible.
> 
>> To get into v3.5 it does need to go in through v4 first though.
> 
> 

Now that the v4 port is done and working. Iv've applied this to 3.5 as
rev.14058.

Amos


___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


Re: [squid-dev] [PATCH] Do not make bogus recvmsg calls

2016-06-15 Thread Alex Rousskov
On 06/09/2016 08:09 PM, Amos Jeffries wrote:
> On 10/06/2016 12:20 p.m., Alex Rousskov wrote:
>> The attached simple patch prevents bogus recvmsg(2) calls when
>> closing file descriptors. Please see the patch preamble for details.

> +1. Looks okay for now.

Committed to trunk (r14714).


> Its relation to RST implies UDP sockets are also doing useless reads.
> IIRC there is a difference between client and server TCP sockets RST
> behaviour as well. So this probably needs a full re-analysis to get right.

Agreed.


Thank you,

Alex.

___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


Re: [squid-dev] [PATCH] Better support for unknown URL schemes

2016-06-15 Thread Amos Jeffries
On 16/03/2016 5:52 a.m., Alex Rousskov wrote:
> On 03/15/2016 09:36 AM, Amos Jeffries wrote:
>> Squid already contains AnyP::PROTO_UNKNOWN support for unknown protocols
>> but currently does not preserve the actual string value received for them.
>>
>> This adds a textual representation ('image') to the UriScheme object to
>> fill that gap and ensure that all URL representatinos (ie cache keys,
>> logs and outgoing messages) are generated with the scheme string as it
>> was received.
> 
> Preserving textual representation is the right thing to do, but the
> implementation itself needs a lot more work IMO.
> 
> 
>>  /// convert the URL scheme to that given
>>  void setScheme(const AnyP::ProtocolType ) {scheme_=p; touch();}
>> +void setSchemeImage(const char *str) {scheme_.setImage(str); touch();}
> 
> 
> The parameter type should be changed instead. Yes, that will require
> more changes, but those changes are essential to properly support
> foreign protocol names. Without those changes, you are creating an API
> bug in addition to adding a useful feature.
> 
> We (mostly you) have already done similar work for foreign HTTP request
> methods. This is similar -- UriScheme should be used nearly everywhere
> (instead of ProtocolType that does not carry enough information). It
> could be argued that the UriScheme class itself should be renamed to
> something like Protocol, but I do not want to argue about that.
> 

There are two distinct layers involved. I'm sure you know this, but I'll
reiterate for the sake of clarity.

- ProtocolType is the transfer or transport protocol type.
- UriScheme is the URL/URI/URN scheme label.

They have been conflated in the past and are still in the difficult
process of de-tangling. The URI scheme implies a protocol, but is not
one in and of itself. For example HTTP ProtocolType can be used to
transfer ftp:// scheme URLs, etc.

Untangling them is happening, but not part of the scope of this patch
except to the point that the URL image() is being made to no longer
generate directly from the ProtocolType value in most cases.

> 
>> +/// Sets the string representation of this scheme.
>> +/// Only needed if the scheme type is PROTO_UNKNOWN.
>> +void setImage(const char *str) {assert(theScheme_ == 
>> AnyP::PROTO_UNKNOWN); schemeImage_ = str;}
> 
> This method should be a constructor.

Done. Note that this has side effects on HttpRequest construction and
requires move semantics to be added to UriScheme. Not a problem exactly,
but scope creep.

> 
>>  char const *
>>  AnyP::UriScheme::c_str() const
>>  {
> 
> This method should be replaced with AnyP::UriScheme::image() returning SBuf.
> 

Done.

> 
>> +if (!schemeImage_.isEmpty())
>> +return schemeImage_.c_str();
>> +
> 
> Why are we not lowering the case of a foreign protocol image if we are
> lowering the case of standard protocol images below? Either both should
> be lowered or there should be a source code comment explaining the
> discrepancy.

Transfer Protocol type and Scheme labels are both case-sensitive (and
differently cased).

So when using an externally supplied scheme label we must take what was
given.

But, since our registered ProtocolType and its ProtocolType_str is
actually the message transfer protocol label (upper case) we need to
lower the case of ProtocolType_str when using it as implicit URI scheme
for one of the registered/known scheme names.


> 
> If both should be lowered, the lowering should be done in the
> constructor to avoid lowering the same string multiple times.
> 

Trunk is what lowers the string multiple times (on each display of the
URL, including debugs). These patches make that happen only once and
stores the lowered result in the scheme image_ to re-use it for any
followup calls after the initial down-case.
 Initial patch did it on first display. After the ctor changes it now
happens one on construction regardless of whether display ever happens.


> 
>>  if (theScheme_ > AnyP::PROTO_NONE && theScheme_ < AnyP::PROTO_MAX) {
>> -const char *in = AnyP::ProtocolType_str[theScheme_];
>> -for (; p < (BUFSIZ-1) && in[p] != '\0'; ++p)
>> -out[p] = xtolower(in[p]);
>> +schemeImage_ = AnyP::ProtocolType_str[theScheme_];
>> +schemeImage_.toLower();
>>  }
> 
> This slow code may not be needed at all if the rest of the changes are
> implemented. If this code stays, please add an "XXX: Slow." or a similar
> comment after toLower(). There are many ways to fix/optimize this, but
> the best way probably depends on the rest of the changes so I am not
> going to suggest any specific TODO here.

AFAICT the best optimization we can do is just take the image string
from the urlParse logic. Which is what this patch is doing now after the
ctor changes.

The slow logic path is now only done for URI which are generated
internally by Squid from just a ProtocolType enum.

> 
>> @@ -157,6 +158,9 @@
>>  if (strncasecmp(b, "whois",