Opened 6 months ago

Closed 5 months ago

Last modified 5 months ago

#159 closed defect (fixed)

Long file names broken.

Reported by: Nathan Owned by: Thomas Bernard
Priority: major Milestone: 2.8
Component: GrafX2 Version: 2.6
Keywords: Cc:

Description

Loading images, palettes, etc. doesn't work on win32 if file name (without directory) is longer than 15 characters. This issue occurs on every win32 build I tried: 2.7, 2.8 nightlies, GDI, SDL.

When you click load the UI flashes red, and prints this to console:

Warning in file loadsave.c, line 617, function Load_image : Cannot open file for reading
Error number 0 occurred in file loadsave.c, line 618, function Load_image.

If you click load again on the same file it will crash.

When you look at WinAPI calls grafx2 is making you can see it's actually trying to access a truncated file path (see attachments).

Attachments (11)

grafx2_ui.PNG (25.9 KB) - added by Nathan 6 months ago.
grafx2_procmon.PNG (24.3 KB) - added by Nathan 6 months ago.
grafx2_status.png (11.1 KB) - added by Nathan 5 months ago.
grafx2_nameinvalid.png (15.2 KB) - added by Nathan 5 months ago.
gfx2-nonlatin1-NTFS-typed.png (12.7 KB) - added by Nathan 5 months ago.
gfx2-nonlatin1-NTFS-selected.png (11.1 KB) - added by Nathan 5 months ago.
gfx2-nonlatin1-NTFS-cmdarg.png (20.9 KB) - added by Nathan 5 months ago.
gfx2-nonlatin1-FAT32-typed.png (24.0 KB) - added by Nathan 5 months ago.
gfx2-nonlatin1-FAT32-selected.png (18.7 KB) - added by Nathan 5 months ago.
gfx2-nonlatin1-FAT32-cmdarg.png (87.3 KB) - added by Nathan 5 months ago.
gfx2-nonlatin1-FAT32-dnd.png (42.5 KB) - added by Nathan 5 months ago.

Download all attachments as: .zip

Change History (42)

Changed 6 months ago by Nathan

Attachment: grafx2_ui.PNG added

Changed 6 months ago by Nathan

Attachment: grafx2_procmon.PNG added

comment:1 Changed 6 months ago by Nathan

Summary: Long file names bronken.Long file names broken.

comment:2 Changed 6 months ago by Thomas Bernard

I'm pretty sure it used to work... I will debug it :(

comment:3 Changed 6 months ago by Thomas Bernard

Owner: changed from pulkomandy to Thomas Bernard
Status: newaccepted

comment:4 Changed 5 months ago by Thomas Bernard

I just tested with a freshly build GrafX2 GDI, from the current master a203da655ad7d19a6f2fb2fb0e91f1cb7b4a3b73 https://gitlab.com/GrafX2/grafX2/-/commit/a203da655ad7d19a6f2fb2fb0e91f1cb7b4a3b73
it works Well.

I'm using Windows 10 64bits

please tell us what is your operating system and how GrafX2 was built

comment:5 Changed 5 months ago by Thomas Bernard

Resolution: invalid
Status: acceptedclosed

I've tested on a Windows 10 version 1909 machine
18363.1139

comment:6 Changed 5 months ago by Nathan

Windows 8.1 64bit with latest updates.

I'm using the latest gitlab CI builds for win32. The file pickers still don't work. It does open long names when drag-and-dropping files but then the status bar becomes "very sad" (see attachment). Resaving a file opened like that produces a jibberish filename which most of the time the system refuses to create.

All my file names are straight ascii btw. no unicode chars.

Changed 5 months ago by Nathan

Attachment: grafx2_status.png added

Changed 5 months ago by Nathan

Attachment: grafx2_nameinvalid.png added

comment:7 Changed 5 months ago by PulkoMandy

What is your OS language? It may affect how these APIs work, I suspect.

comment:8 Changed 5 months ago by Nathan

My OS locale is PL, but my OS language is en-US, fallback language for non-unicode apps is also en-US. I've already tried changing OS locale to US, various compatibility settings etc.

comment:9 Changed 5 months ago by Thomas Bernard

GrafX2 uses regular fopen() to open files. Under windows, it converts to/from Unicode Long files names using GetShortPathName?() and GetLongPathName?().
We makes the assumption that short names are plain ASCII...

My MSVC build outputs :
built with _MSC_VER=1900 Windows ANSI Code Page=1252

What's your codepage ?

Also what's the result of trying to open a file from the command line ?

comment:10 Changed 5 months ago by Thomas Bernard

What's it weird is that it works for directory listing. (grafx2_ui.PNG screenshot)
Grafx2 is using FindFirstFileW / FindNextFileW API to list directory content.
it uses cFileName as Unicode filename and cAlternateFileName as the "short" ASCII one. (DOS 8.3)

comment:11 Changed 5 months ago by Thomas Bernard

the issue may be explained here :
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-160

The fopen function opens the file that is specified by filename. By default, a narrow filename string is interpreted using the ANSI codepage (CP_ACP). In Windows Desktop applications this can be changed to the OEM codepage (CP_OEMCP) by using the SetFileApisToOEM function. You can use the AreFileApisANSI function to determine whether filename is interpreted using the ANSI or the system default OEM codepage.

comment:12 Changed 5 months ago by Thomas Bernard

@Nathan could you try this test version ?
From the jobs artifiacts :
https://gitlab.com/miniupnp/grafX2/-/jobs/892600268/artifacts/browse

I've added a call to SetFileApisToANSI()...
https://gitlab.com/miniupnp/grafX2/-/commit/e13a5e32bdc3371272615e40639b627ef046d676

comment:13 Changed 5 months ago by Nathan

Opening some long name files from command line works fine with the file correctly identified in the status bar. Opening some longer file paths shows "file not found" except now instead of being truncated it seems to look for the file with some random memory appended at the end of the path.

My code page was 437 for some reason. After changing cp to 1252:

Opening some long name files works in the command line, opening longer file paths crashed before the window even showed up, now it mostly works but still crashes intermittently at exit.

Still, no changes in GUI.

wip3039: Same, mostly works in command line, sometimes crashes at exit. Doesn't work in GUI.

Also am I supposed to see "built with _MSC..." in the verbose output? Because it's no there. That whole string isn't even in the binary.

comment:14 Changed 5 months ago by Nathan

And what's even weirder, if I rename one of the fonts in share/grafx2/fonts to a super long name, it has no problem with that. But it won't load a script with a long name.

comment:15 Changed 5 months ago by Thomas Bernard

"built with _MSC..." is only if you build with MS Visual Studio. gitlab CI builds are made using MinGW32 compiler.

I'm afraid I'm not able to understand what's going on on your machine.
Could you build and debug yourself ?
You can use MS Visual Studio Community :
https://visualstudio.microsoft.com/pl/free-developer-offers/

comment:16 Changed 5 months ago by Thomas Bernard

@Nathan: I'm making a new version with more logs
https://gitlab.com/miniupnp/grafX2/-/jobs/892646382/artifacts/browse

comment:17 Changed 5 months ago by Thomas Bernard

please run with the -v command line flag and tell us if they are errors related to GetShortPathName?() etc.

comment:18 Changed 5 months ago by Nathan

There are no new errors in the verbose output. I'm not going to install Visual Studio but if you have a MSVC build with a pdb, I might be able to take a closer look.

comment:19 Changed 5 months ago by Thomas Bernard

I'm very puzzled, if there is no error with the GetLongPathName? / GetShortPathName?
I don't understand what's going on.

About the Drag&Drop, I found an issue. Could you test this new version ?
https://gitlab.com/miniupnp/grafX2/-/jobs/893198919/artifacts/browse

I'm really curious to see if it shows the long filename.

comment:20 Changed 5 months ago by Nathan

Drag&drop now works great ;) Shows the long name and saves correctly.

comment:21 Changed 5 months ago by Thomas Bernard

I'm even more puzzled by the issues you're having with loading from the file selector :(

comment:22 Changed 5 months ago by Nathan

OK, I've done some more testing:

Manually typing the long filename in the "filename" text box loads the file correctly.

Clicking on a long file name in the list box produces a red flash and an error described in the original report. Clicking LOAD after that crashes the program.

Clicking on a long file name in the list box, then after the flash and the error clicking on the filename text box and then clicking LOAD successfully loads the file.

comment:23 Changed 5 months ago by Thomas Bernard

What filesystem is your T: ?

comment:24 Changed 5 months ago by Thomas Bernard

I think I have found the bug and fixed it
https://gitlab.com/miniupnp/grafX2/-/jobs/898541241/artifacts/browse

but it must be something specific with the filesystem on your T: Drive

comment:25 Changed 5 months ago by Thomas Bernard

Resolution: invalid
Status: closedreopened

comment:26 Changed 5 months ago by Nathan

Resolution: fixed
Status: reopenedclosed

Yep, it reads all directories correctly now. Scripts and everything.
No errors or warnings.

I was using files on different partitions, all NTFS.

So, out of curiosity I just put a few files on a FAT32 USB stick to see if it would make any difference and it does! The old builds work fine with FAT32.

Thank you for looking into this ;)

comment:27 Changed 5 months ago by Thomas Bernard

are you sure it is NTFS ? Is there a specific configuration ?
Because it if fixes it, it confirms FinFirstFile? / FindNextFile? doesn't return the "short name" on your machine / this specific file system.

Could you test with a long filename that is non latin1 ? (contains characters such as Ελληνική_γλώσσα Россия ひらがな カタカナ)

comment:29 Changed 5 months ago by Nathan

It seems they are disabled.

C:\WINDOWS\system32>fsutil 8dot3name query F:
The volume state is: 1 (8dot3 name creation is disabled).
The registry state is: 2 (Per volume setting - the default).

Based on the above two settings, 8dot3 name creation is disabled on F:

C:\WINDOWS\system32>fsutil 8dot3name query T:
The volume state is: 1 (8dot3 name creation is disabled).
The registry state is: 2 (Per volume setting - the default).

Based on the above two settings, 8dot3 name creation is disabled on T:

comment:30 Changed 5 months ago by Nathan

wip3044 on NTFS:

manually typing it in (gfx2-nonlatin1-NTFS-typed.png):

Change_directory("T:\")
Change_directory("gfx2test")
Non latin1 character in translation : \u0105
Non latin1 character in translation : \u0119
KEYDOWN wParam=004e lParam=00310001
KEYDOWN wParam=004f lParam=00180001
KEYDOWN wParam=004e lParam=00310001
KEYDOWN wParam=004c lParam=00260001
KEYDOWN wParam=0041 lParam=001e0001
KEYDOWN wParam=0054 lParam=00140001
KEYDOWN wParam=0049 lParam=00170001
KEYDOWN wParam=004e lParam=00310001
KEYDOWN wParam=0031 lParam=00020001
KEYDOWN wParam=00bd lParam=000c0001
KEYDOWN wParam=0011 lParam=001d0001
KEYDOWN wParam=0012 lParam=21380001
KEYDOWN wParam=0041 lParam=201e0001
KEYDOWN wParam=0045 lParam=20120001
KEYDOWN wParam=00be lParam=00340001
KEYDOWN wParam=0047 lParam=00220001
KEYDOWN wParam=0049 lParam=00170001
KEYDOWN wParam=0046 lParam=00210001
KEYDOWN wParam=000d lParam=001c0001
GetShortPathNameW(007CDCC4, NULL, 0) failed !
Generated a temporary ansi name : nonlatin1-¹ê.gif
Change_directory("nonlatin1-¹ê.gif")
cannot chdir to "nonlatin1-¹ê.gif" !
Current directory is "T:\gfx2test"
Error number 0 occurred in file filesel.c, line 2312, function Button_Load_or_Save.

selecting from the list box (gfx2-nonlatin1-NTFS-selected.png):
(the API call seems to use the correct name, but apparently isn't.)

Change_directory("gfx2test")
Non latin1 character in translation : \u0105
Non latin1 character in translation : \u0119
Cannot open file for reading
Error number 0 occurred in file loadsave.c, line 619, function Load_image.

opening from console (gfx2-nonlatin1-NTFS-cmdarg.png):

shows file not found messagebox, usage messagebox and video modes.
doesn't print anything because the console window doesn't appear 
looking at the api calls it's super weird, it uses both correct and malformed filenames

opening from D&D (same calls as gfx2-nonlatin1-NTFS-cmdarg.png):

WM_DROPFILES 1 files
Cannot open file for reading
Error number 0 occurred in file loadsave.c, line 619, function Load_image.

wip3044 on FAT32:

manually typing it in (gfx2-nonlatin1-FAT32-typed.png):

KEYDOWN(... same keys as above...)
GetShortPathNameW(00F7D6E4, NULL, 0) failed !
Generated a temporary ansi name : nonlatin1-¹ê.gif
Change_directory("nonlatin1-¹ê.gif")
cannot chdir to "nonlatin1-¹ê.gif" !
Current directory is "E:\gfx2test"
Error number 0 occurred in file filesel.c, line 2312, function Button_Load_or_Save.

selecting from list box (gfx2-nonlatin1-FAT32-selected.png):

works. no errors.

opening from console (gfx2-nonlatin1-FAT32-cmdarg.png):

works. no errors.
short names in the API log

opening from D&D (gfx2-nonlatin1-FAT32-dnd.png):

works. no errors.

Changed 5 months ago by Nathan

Changed 5 months ago by Nathan

Changed 5 months ago by Nathan

Changed 5 months ago by Nathan

Changed 5 months ago by Nathan

Changed 5 months ago by Nathan

Changed 5 months ago by Nathan

comment:31 Changed 5 months ago by Thomas Bernard

So now we know that GrafX2 need the 8.3 filename support to be enabled on NTFS volumes...

Note: See TracTickets for help on using tickets.