[Freeipa-devel] [PATCH] 746-747 append domain into network.negotiate-auth.trusted-uris

[Endi Sukma Dewata]

On 9/10/2014 9:59 AM, Petr Vobornik wrote:
> On 4.9.2014 21:26, Endi Sukma Dewata wrote:
>> On 8/29/2014 11:00 AM, Petr Vobornik wrote:
>>> [PATCH] 746 webui: append network.negotiate-auth.trusted-uris
>>>
>>> https://fedorahosted.org/freeipa/ticket/4478
>>
>> Some comments/questions:
>>
>> 1. If I'm reading this correctly, if the preference is currently empty,
>> the method will just return without setting the new value.
>
> Fixed. Yes, even if there was 'continue', it would be wrong.
>
>>
>> 2. If the new value already exists, the method will just return.
>> Shouldn't it "continue" with the rest of the loop instead of "return"?
>> This applies to #1 as well.
>
> Yes, fixed.
>
>>
>> 3. Using indexOf() to find a URI in a string can produce false matches.
>> For example, aa.com will match baa.com. Ideally the existing value
>> should be parsed into a collection of URI's, then the new URI should be
>> matched using a proper URI matching algorithm.
>
> Fixed with function which matches splitted and striped values using
> strict equality.

ACK. A possible improvement is to normalize the current value to remove 
excessive white spaces.

-- 
Endi S. Dewata