[squid-dev] [PATCH] VIA creation code duplication

[Alex Rousskov]

On 02/09/2017 10:19 AM, Amos Jeffries wrote:
> On 3/02/2017 4:02 a.m., Eduard Bagdasaryan wrote:
>> This patch fixes VIA appending code duplication, moving common
>> code into a separate method.

> Since Via is a list header we should be able to just append a new Via
> header to the header list with putStr. No need to use getList, String,
> delById to inject on the end of existing Via string.
> 
> Please also take the opportunity to fix a long standing protocol
> violation bug in Via header produced by Squid:
>  The version part of the header is only supposed to elide the protocol
> label if the that label would be "HTTP/" (ie. for messages received via
> HTTP and HTTPS).
> 
> eg. for ICY replies:
>   Via: ICY/1.1 squid
> 
> eg. for FTP gatewayed replies (or relayed requests):
>   Via: FTP/2 squid
> 
> 
> After those calls that require String are gone please assemble the value
> in an SBuf instead of static char* buffer.
> 
> Also, for messages where Squid itself generated the message (ie
> Downloader or ESI) there should be no Via header added at all.


Also, please fix the 10 oldest open bugs on Squid Bugzilla while you are
at it.</sarcasm>

IMHO, you may declare any and all of the changes requested by Amos to be
outside the scope of the code deduplication patch you have prepared.
Your patch is valuable even with its current narrow scope. If the patch
has no problems, it should be acceptable "as is". Amos, if you disagree
with this assessment, please say so.

It is your call which additional changes/improvements to implement (and
become responsible for), but my personal recommendation is to treat any
risky changes and any desirable changes visible outside addVia() as
separate issues/projects/TODOs because they probably require
careful/focused consideration and dedicated commits.


Thank you,

Alex.