[squid-users] External acl on delay_access directive

[Alex Rousskov]

On 1/21/19 5:22 AM, Luca Savarino wrote:

> Attached is a patch which seems to fix the issue.

Glad you have a fix that works for you, but this mailing list is not the
right place for patch reviews. If you want to submit your changes to the
Squid project, I suggest creating a GitHub pull request. The procedure
is outlined at https://wiki.squid-cache.org/MergeProcedure


> Does it seem correct to you ?

The patch is buggy: Setting AccessLogEntry::reply field like that may
lead to memory leaks and/or crashes. Ideally, the reply field should be
set when the reply becomes known. I do not know whether that is already
done (elsewhere in the code). If it is done, than the reply setting line
in the patch can be removed. You can answer that question for your
particular use case by checking (e.g., in a debugger or by adding a
debugs() message) whether http->al->reply is nil before the assignment.


HTH,

Alex.


> On 1/17/19 5:39 PM, Alex Rousskov wrote:
>> On 1/17/19 9:13 AM, Luca Savarino wrote:
>>
>>> WARNING: ip_list ACL is used in context without an ALE
>>> state. Assuming mismatch.
>>> delay_access 1 allow ip_list
>> Looks like a Squid bug to me -- Squid should supply ALE (a blob
>> containing various transaction details) to the delay_access code but
>> evidently does not.
>>
>> If you are a developer or can hire a developer to fix this bug, a good
>> starting point could be the missing ACLFilledChecklist::al
>> initialization in DelayId::DelayClient().
>>
>>
>> HTH,
>>
>> Alex.
>> _______________________________________________
>> squid-users mailing list
>> squid-users at lists.squid-cache.org
>> http://lists.squid-cache.org/listinfo/squid-users