[squid-dev] [PATCH] splicing resumed sessions

[Tsantilas Christos]

Patch applied as r14012.
I am attaching the t13 patch for squid-3.5 too.


On 04/11/2015 06:18 AM, Amos Jeffries wrote:
> On 11/04/2015 1:49 a.m., Tsantilas Christos wrote:
>> I am attaching patch for trunk and squid-3.5
>>
>
> Thank you. Looks pretty good now.
>
>> On 04/09/2015 04:13 PM, Amos Jeffries wrote:
>>>
>>> * Ssl::Bio::sslFeatures::parseV3Hello()
>>>    - similar issues with s/Client Hello/ClientHello/ and "SSL Extension"
>>> as above.
>>
>> I did it only for the comments added or modified by this patch to avoid
>> increasing the size of this patch.
>> If required we can do it as separate patch.
>
> Theres still one s/SSL Extension/TLS Extension/ near the end of this method.
>
>>> * Ssl::Bio::sslFeatures::print()
>>>    - seems to be lacking display of ALPN received
>>>    - missing the format details for sslVersion used elsewhere:
>>>        std::hex << std::setw(8) << std::setfill('0')
>>
>>
>> I did not fix this. The ALPN is in an encoded form and requires some
>> development to correctly print it. We do not have to gain something
>> implementing this.
>> Also the Ssl::Bio::sslFeatures::print currently is not used. It is here
>> for using it for debugging if required.
>>
>
> That means the print() will be incomplete, and needs a TODO added about
> the above.
>
>>
>>>
>>> * parseMsgHead() documentation about return result >0 is wrong.
>>>     - it does not return <0 when the contents of the buffer are a TLS
>>> (non-SSL) message.
>>
>> This is what it does!
>>
>
> The .h comment says it will return a negative number if the Hello
> message "is not SSL". TLS != SSL.
>
> For the case of head[0] == 0x16 the only SSL indicator (SSLv3) is
> *exactly* {0x16, 0x03, 0x00}
>
> For TLS versions that final 0x00 byte changes. The method correctly
> accepts those and returns the Hello size (N>0) - in contradiction to
> what is documented.
> The comment should either say "SSLv3 or TLS", or not mention the
> protocol name at all.

OK I made this fixes too.

>
>
> +1. I dont think this needs another review, once those comments are
> added/updated it can merge.
>
>
> Amos
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: splicing-resumed-sessions-Squid-3.5-t13.patch
Type: text/x-patch
Size: 57977 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150411/10a22fa4/attachment-0001.bin>