On Thu, Sep 05, 2013 at 05:17:24PM +0200, Pavel Březina wrote:
On 09/05/2013 03:09 PM, Simo Sorce wrote:
>On Thu, 2013-09-05 at 10:49 +0200, Pavel Březina wrote:
>>On 09/04/2013 09:12 PM, Simo Sorce wrote:
>>>On Wed, 2013-09-04 at 17:06 +0200, Sumit Bose wrote:
>>>>On Tue, Sep 03, 2013 at 02:07:29PM -0400, Simo Sorce wrote:
>>>>>On Tue, 2013-09-03 at 15:25 +0200, Pavel Březina wrote:
>>>>>>On 09/03/2013 01:27 PM, Sumit Bose wrote:
>>>>>>>Hi,
>>>>>>>
>>>>>>>while looking at expand_ccname_template() becasue of
shadowing rewind()
>>>>>>>I realized that there a some issues with some of the new
krb5.conf
>>>>>>>templates. This patch fixes them and adds some tests to avoid
similar
>>>>>>>issues in the future.
>>>>>>>
>>>>>>>There is one change in behaviour. If the name in the %{}
braces does not
>>>>>>>match any of the known krb5.conf templates for UNIX the new
code returns
>>>>>>>an error while the old just returned something, which in most
case will
>>>>>>>not be the original input. Please tell me if you prefer the
original
>>>>>>>input in this case so that I can change the patch
accordingly.
>>>>>>
>>>>>>I think we should expand the parameters we can, i.e. uid and euid
and
>>>>>>leave everything else intact (i.e. don't even check ${LIBDIR}
etc.). Ad
>>>>>>absurdum: for what we know, users can ran custom build that
supports
>>>>>>other variables...
>>>>>
>>>>>True, I was looking at this code yesterday again as I am hadling
2071
>>>>>and I was thinking the same, we should just ignore and leave
unchanged
>>>>>%{xyz} patterns we do not understand, so that if libkrb5 adds
something
>>>>>the admin can use we do not have to explicitly support it.
>>>>>
>>>>>>But if you don't agree, I'll ack this patch.
>>>>>
>>>>>No, let's nack, and if you all do not mind I can come up with an
>>>>>alternative patch that does what I describe above.
>>>>
>>>>Whatever you prefer, but I can do the needed changes as well. If you do
>>>>them, please do not forget to add tests for the cases which are
>>>>currently not covered.
>>>
>>>I opened bug #2076 to track this issue and attached find a different
>>>approach with testes against current master.
>>>
>>>Simo.
>>
>>Nack.
>>
>>krb5_utils-tests.c: In function ‘test_krb5_style_expansion’:
>>krb5_utils-tests.c:679:9: error: unused variable ‘ret’
>>
>>It looks good otherwise.
>>Can we also include Sumit's tests?
>
>Sumit's tests consider any 'unknown' expansion variable as invalid, so
>they would fail.
>
>Attached new version w/o ret and rebased on current master.
>
>Simo.
OK then. Ack.
Pushed to master and sssd-1-10