On 11/19/2013 04:01 PM, Amos Jeffries wrote: > On 2013-11-20 08:11, Alex Rousskov wrote: >> On 11/19/2013 02:54 AM, Tsantilas Christos wrote: >> >>> My understanding is that we need: >>> 1) Allow configuring the request format using one of the following: >>> a) Use a request_format configuration parameter plus the >>> %credentials formating code >>> b) Use the following request format: >>> [prefix] credentials [key_extras] >> >> Yes. And since the official code implements (b) already (without >> key_extras), it may be best to add key_extras processing to that code >> rather than to add [hidden] %credentials functionality. To avoid redoing >> this code for the third time, I recommend getting a clear answer from >> Amos first: >> >> Amos, do you want Christos to yank all internal support for %credentials >> macro, going back to the hard-coded generation of the credentials part >> of helper requests? Or are you OK with Squid supporting %credentials >> macro, and the patch using that internal support to generate the helper >> request? In both cases, the admin will not use %credentials in >> key_extras configuration (because credentials info will be automatically >> prepended by Squid in both cases). > > I'm in two minds about the %credentiasl macro itself. It makes sens as a > separate thing for logging those details, or using in other helpers > formats (rewriter, exetrnal ACL, etc).
Agreed, it is only potentially useful. Note that, in most cases, it would be useful keep KK/etc. a part of the %credentials value so that the recipient of NTLM/Negotiate credentials can tell what the logged/received value means. > But it is definitely not necessary for what this patch needs to do, IMO. Agreed, it is not essential, especially given your restrictions on the overall functionality. Hard-coded credentials code is sufficient. > If you want to omit it from this patch and do it as a second one that > would be okay. I do not like the idea of increasing the amount of work further by splitting this feature into two. I think your choices are: 1) Allow %credentials to be used internally in the key_extras patch. This is less work for Christos (because the code is already written) and adds a potentially useful feature to Squid. 2) Object to %credentials in the key_extras patch. This makes the final patch simpler/smaller. Please pick one. > The need for it would be quite rare, such as admin passing the helper > values used directly as SQL query parameters. Agreed. Thank you, Alex.
