Re: [PATCH] url_rewrite_extras for redirector helpers

On 25/02/2014 5:53 a.m., Tsantilas Christos wrote:
> On 02/24/2014 01:40 AM, Amos Jeffries wrote:
>> On 2014-02-24 10:12, Tsantilas Christos wrote:
>>> Hi all,
>>>
>>> This patch add the url_rewrite_extras for redirector helpers.
>>> The url_rewrite_extras is a "quoted string" with logformat %macro
>>> support. It is appended to request line for redirector helpers.
>>>
>>> Example usage:
>>> url_rewrite_extras "Note1=%{Note1}note Note2=%{Note2}note"
>>>
>>> The url_rewrtite_extras it is similar to the "key_extras" authenticator
>>> helpers options. Originally developed to allow notes exchange between
>>> authenicator helpers and redirector helpers.
>>>
>>> In this patch we add a new type for cf.data the TokenOrQuotedString to
>>> allow new configurations use quoted strings for the new option without
>>> explicitly turning configuration_includes_quoted_values on.
>>>
>>> This is a Measurement Factory project.
>>
>>
>> in src/cf.data.pre:
>> * why are you calling this new directive an alias of a non-existent
>> "format" directive?
>> "
>> +NAME: url_rewrite_extras format
>> "
>>

You still have the above issue. Looking at the way you added it to
storeid_extras as well it looks like you are completely misunderstanding
the syntax of the NAME: entry.

   NAME: a b c

... means that directive name 'a' is the current official squid.conf
name but this directive has previously been known by the names 'b' and
'c'. The code is *also* able to auto-convert between syntax of a, b, and c.
 NP: if the auto-convert is not possible (such as splitting an existing
direcive in two like your patch is implying about directive "format")
then the "TYPE: obsolete" mechanism is used instead of aliases to give
instructinos on replacing the old directive.

There has never been a "format" directive and it certainly is not
deprecated by two separate new directives. So please remove the extra
"format" words in cf.data.pre.

Also I see some more:

in src/redirect.cc
* new ::Format::Format("redirecor_extras");
  - s/redirecor_extras/url_rewrite_extras/

* new ::Format::Format("storeId_extras");
 - s/storeId_extras/store_id_extras/

Those can be done on commit. +1 conditional on the above change.

Amos
Received on Thu Mar 13 2014 - 04:06:14 MDT