On Mon, 16 Jan 2023 14:19:52 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

> This is somewhat a continuation of #11104 where the issue was discussed and 
> where I found [the root 
> cause](https://github.com/openjdk/jdk/pull/11104#issuecomment-1382435784).
> 
> **Root Cause**
> 
> The icon extraction code compares the returned handle to zero: 
> https://github.com/openjdk/jdk/blob/8cb25d3de494c6d9357a1c199e1a9dbff9be9948/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L1157-L1158
> 
> On 64-bit systems, a valid handle can be negative. For example:
> 
> 
> getIcon : Desktop 16(16)
> GetIconLocation(uFlags=0x22, flags=0x4, index=-110) SUCCESS - 
> szBuf=C:\Windows\system32\imageres.dll
> Extract - hres: 0, hIcon=0000000019B200F9, hIconSmall=FFFFFFFFF98C00EB, 
> size=16(0x100020)
> SUCCEEDED
> !!! hIcon(0xfffffffff98c00eb) <= 0 : Desktop 16(16)
> 
> 
> Here in `size=16(0x100020)`, 16 is the requested icon size, the value in 
> parenthesis is value of `iconSize` passed to the 
> [`Extract`](https://learn.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_core-iextracticonw-extract)
>  method that contains the size for small icon in high word and that for large 
> icon in low word. `Desktop 16(16)` means the icon of size 16×16 is requested, 
> and we're extracting 16×16 icon.
> 
> The icon was extracted _successfully_ but its handle was interpreted as an 
> error.
> 
> The same problem exists in the fallback code:
> https://github.com/openjdk/jdk/blob/8cb25d3de494c6d9357a1c199e1a9dbff9be9948/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L1161
> 
> Then when the fallback code is executed, another problem occurs:
> 
> 
> GetIconLocation(uFlags=0x40, flags=0x1a3, index=-675088752) SUCCESS - szBuf=
> Extract - hres: 80004005, hIcon=0000000000000000, 
> hIconSmall=000001A3D7C2F83A, size=16(100020)
> NOT SUCCEEDED
> hIcon = 0x1a3d7c2f83a : Desktop 16(16)
> 
> 
> The error code 0x80004005 is `E_FAIL`. Yet the return value is not verified 
> in the code:
> https://github.com/openjdk/jdk/blob/8cb25d3de494c6d9357a1c199e1a9dbff9be9948/src/java.desktop/windows/native/libawt/windows/ShellFolder2.cpp#L997-L1003
> 
> Since the icon handle is *seemingly* valid, it continues to extracting the 
> icon bits:
> 
> 
> native getIconBits - fn_GetIconInfo returned false
> Error code: 0x57a - Invalid cursor handle.
> makeIcon: iconBits = null
> 
> 
> It's not a surprise. After all, the `Extract` method returned an error, which 
> means neither icon handle is valid.
> 
> In the end, the `MultiResolutionIcon` contains `null`:
> 
>     16 -> null
>     32 -> BufferedImage @ 59e735d
>     24 -> BufferedImage @ 60b2faf0
> 
> 
> This problem has existed fo...

Thank you, Karl, for bringing the problem in this PR. It already exist in JBS 
as [JDK-8320692](https://bugs.openjdk.org/browse/JDK-8320692): _Cannot invoke 
java.awt.Image.getWidth(java.awt.image.ImageObserver)_.

I added my observations there. The problem is likely applicable to other file 
types which have per-instance icons, however, these are uncommon with the 
exception of exe files. The suggested fix won't work, there should be an icon 
but we have to handle the fallback gracefully.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/12010#issuecomment-1874358079

Reply via email to