db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andreas Korneliussen <andreas.kornelius...@sun.com>
Subject Re: DRDA folks: Please review DERBY-846 patch
Date Tue, 02 May 2006 21:09:20 GMT
David W. Van Couvering wrote:
> Hi, Andreas.  Thanks for reviewing this.
> 
> I was trying to give the other-side-of-pond folks a day to review it by 
> posting the patch and request for review last night.
> 

Well, thanks for contributing this important piece of work.

I think I missed the first request for review.  I guess it only got my 
full attention when you posted the deadline.

> How much time do you need?

I cannot review it more tonight - I am fine with your responses to my 
comments.

I will look at it tomorrow, and let you know if I have found any issues, 
regardless if you decide to check-in or not.

Andreas

> 
> Let me respond to your comments below.
> 
> Andreas Korneliussen wrote:
> 
>> David W. Van Couvering wrote:
>>
>>> Hi, all.  This patch is blocking further i18n work for me.  I also 
>>> don't want to have to do any merges and re-run derbyall.
>>>
>>> If you could get any comments in by noon, that would be appreciated, 
>>> otherwise I will go ahead and check in and we can fix anything 
>>> post-checkin.
>>>
>> This was quite a short deadline - if you want comments from the other 
>> side of the world, it would be wise to extend the deadline.
>>
>> I have only looked briefly at the diff file. I think that the changes 
>> in NetConnection40 and NetResultSet40 are not necessary (it appears 
>> the changes only fix code indentation), so to avoid getting other 
>> developers into merge conflicts, you could probably revert those changes.
>>
>> Another issue, is this error message:
>> ERROR 08004: Connection authorization failure occurred.  Reason: 
>> userid invalid.
>>
>> I think, for security policy reasons, the message should not reveal 
>> that userid is invalid.
> 
> 
> Hm, I agree.  That said, "invalid userid" and "invalid password" are 
> both standard DRDA values for the SECCHKCD parameter.  I guess I can say 
> "invalid userid or password" or "invalid credentials" for both of them, 
> that's a pretty common approach.
> 
>>
>> Also, shouldn't it read "authentication failed", instead of 
>> "authorization  failure occured" ?
> 
> 
> Yes, I agree, I can change that.
> 
>>
>> (note: the patch did not really introduce this last issue, only fixed 
>> the message to have messageid as well)
>>
>>
>> -- Andreas
>>
>>> Thanks,
>>>
>>> David
>>>
>>> David W. Van Couvering wrote:
>>>
>>>> This patch is the first patch internationalizing strings in the 
>>>> network part of the network client.  I thought it would be good to 
>>>> get the eyes of some of the folks working in this area on this, to 
>>>> make sure it makes sense to you.
>>>>
>>>> http://issues.apache.org/jira/browse/DERBY-846?page=all
>>>>
>>>> Thanks,
>>>>
>>>> David
>>
>>


Mime
View raw message