Request for code review 6512101: Incorrect encoding in NetworkInterface.getDisplayName()

[Frank Ding]

Thanks Jonathan.


On 12/11/2012 10:44 AM, Jonathan Lu wrote:
> Hi Frank,
>
> Patch pushed @ http://hg.openjdk.java.net/jdk8/tl/jdk/rev/883feced1cdd
>
> Best regards!
> - Jonathan
>
> On 12/11/2012 09:49 AM, Frank Ding wrote:
>> Hi All,
>>   Thank you all.  I will have Jonathan help to commit.
>>
>> Best regards,
>> Frank
>>
>> On 12/10/2012 6:12 PM, Chris Hegarty wrote:
>>> On 10/12/2012 08:01, Dmitry Samersoff wrote:
>>>> Frank,
>>>>
>>>> Excellent! Thank you for doing it.
>>>
>>> Ditto, thanks Frank.
>>>
>>> I assume Sean or Neil will push this for you? Otherwise, just let me 
>>> know and I can do it.
>>>
>>> -Chris.
>>>
>>>>
>>>> -Dmitry
>>>>
>>>> On 2012-12-10 12:00, Frank Ding wrote:
>>>>> Hi Dmitry,
>>>>>    I updated wording accordingly @
>>>>> http://cr.openjdk.java.net/~dingxmin/6512101/webrev.03. It is now
>>>>> changed to "Cannot get multibyte char for interface display 
>>>>> name".  What
>>>>> about that?
>>>>>
>>>>> Best regards,
>>>>> Frank
>>>>>
>>>>> On 12/10/2012 3:43 PM, Dmitry Samersoff wrote:
>>>>>> Frank,
>>>>>>
>>>>>> Looks good for me.
>>>>>>
>>>>>> May be "Can't get WIDE string" should be changed to something more
>>>>>> verbose.
>>>>>>
>>>>>> -Dmitry
>>>>>>
>>>>>>
>>>>>> On 2012-12-10 11:40, Frank Ding wrote:
>>>>>>> Hi Dmitry and Chris,
>>>>>>>     Could you please review the second revision again?
>>>>>>>
>>>>>>> Thanks and Best regards,
>>>>>>> Frank
>>>>>>> On 11/29/2012 1:08 PM, Frank Ding wrote:
>>>>>>>> Hi Dmitry and Chris,
>>>>>>>>     Thanks for your comments.  With your comments incorporated, 
>>>>>>>> I've
>>>>>>>> prepared v2 @
>>>>>>>> http://cr.openjdk.java.net/~dingxmin/6512101/webrev.02/. Could you
>>>>>>>> please review it again?
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Frank
>>>>>>>>
>>>>>>>> On 11/14/2012 12:12 AM, Chris Hegarty wrote:
>>>>>>>>> On 11/11/2012 07:03 PM, Dmitry Samersoff wrote:
>>>>>>>>>> Frank,
>>>>>>>>>>
>>>>>>>>>> Changes look good for me.
>>>>>>>>> I admit that I am not an expert in this area, but given the
>>>>>>>>> information you provided, and I guess you verified this in your
>>>>>>>>> environment, the conversion would appear reasonable.
>>>>>>>>>
>>>>>>>>>> But it might be better to fall back to original behavior if
>>>>>>>>>> MultiByteToWideChar return error, rather than abort.
>>>>>>>>> I agree with Dmitry, fall back would be preferable. Can you 
>>>>>>>>> make the
>>>>>>>>> changes and post an updated webrev.
>>>>>>>>>
>>>>>>>>> -Chris.
>>>>>>>>>
>>>>>>>>>> -Dmitry
>>>>>>>>>>
>>>>>>>>>> On 2012-11-07 13:08, Frank Ding wrote:
>>>>>>>>>>> Hi guys,
>>>>>>>>>>>      Could you please take a look at patch below aimed to 
>>>>>>>>>>> resolve
>>>>>>>>>>> existing
>>>>>>>>>>> bug 
>>>>>>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6512101 ?
>>>>>>>>>>> http://cr.openjdk.java.net/~dingxmin/6512101/webrev.01/
>>>>>>>>>>>
>>>>>>>>>>>      I happen to have a Chinese Win 7 environment. 
>>>>>>>>>>> "buggy.png" is
>>>>>>>>>>> current
>>>>>>>>>>> output of test case described in bug system whereas "fixed.png"
>>>>>>>>>>> is the
>>>>>>>>>>> output after the my patch is applied.
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~dingxmin/6512101/buggy.png
>>>>>>>>>>> http://cr.openjdk.java.net/~dingxmin/6512101/fixed.png
>>>>>>>>>>>
>>>>>>>>>>>      The patch simply converts to wide chars encoded in 
>>>>>>>>>>> CP_OEMCP by
>>>>>>>>>>> calling
>>>>>>>>>>> MultiByteToWideChar.  We have confirmed a guy from Microsoft 
>>>>>>>>>>> who
>>>>>>>>>>> said that
>>>>>>>>>>>
>>>>>>>>>>> BEGIN QUOTE
>>>>>>>>>>> I'm not sure how common it is to call the Java code that 
>>>>>>>>>>> results in
>>>>>>>>>>> calling the GetIfTable API but I would guess it does not 
>>>>>>>>>>> happen that
>>>>>>>>>>> often. Additionally, if it's rare that the adapter contains the
>>>>>>>>>>> accented
>>>>>>>>>>> characters, it would definitely be quite easy to miss in 
>>>>>>>>>>> testing.
>>>>>>>>>>>
>>>>>>>>>>> I have not found any documentation about the encoding of the 
>>>>>>>>>>> bDescr
>>>>>>>>>>> string unfortunately. I did, however, debug through the API and
>>>>>>>>>>> located
>>>>>>>>>>> the place where it is generated. It is getting converted from a
>>>>>>>>>>> UTF-16
>>>>>>>>>>> string to a single-byte string using a conversion like this:
>>>>>>>>>>>
>>>>>>>>>>> WideCharToMultiByte(
>>>>>>>>>>> CP_OEMCP,
>>>>>>>>>>> WC_NO_BEST_FIT_CHARS,
>>>>>>>>>>> <source string>,
>>>>>>>>>>> -1,
>>>>>>>>>>> IfRow->bDescr,
>>>>>>>>>>> <size>,
>>>>>>>>>>> NULL,
>>>>>>>>>>> NULL);
>>>>>>>>>>>
>>>>>>>>>>> I have checked the source for Windows Vista, 2008, Windows 
>>>>>>>>>>> 7, and
>>>>>>>>>>> Windows 2008 R2. It is using CP_OEMCP in all of them. So 
>>>>>>>>>>> using the
>>>>>>>>>>> reverse conversion in your code using CP_OEMCP should be safe.
>>>>>>>>>>> Alternatively, you can use the GetIfTable2 function
>>>>>>>>>>> (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365945^(v=vs.85).aspx 
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ) which returns the same information in the original UTF-16
>>>>>>>>>>> encoding.
>>>>>>>>>>> END QUOTE
>>>>>>>>>>>
>>>>>>>>>>>      The link below may be helpful to the second param of
>>>>>>>>>>> WideCharToMultiByte.
>>>>>>>>>>> http://blogs.msdn.com/b/oldnewthing/archive/2012/05/04/10300670.aspx 
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> You comments are appreciated.
>>>>>>>>>>> Best regards,
>>>>>>>>>>> Frank
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>