Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Blank pages when opening .cbz archives with very long filenames #241

Closed
1 task done
himouto opened this issue Jun 21, 2024 · 13 comments
Closed
1 task done
Assignees
Milestone

Comments

@himouto
Copy link

himouto commented Jun 21, 2024

Preflight Checklist

  • I have searched the issue tracker for a bug report that matches the one I want to file, without success.

OpenComic Version

1.2.0

Operating System

Windows 11 23H2 build 22631.3767

Steps to reproduce

Open or rename a comics folders with a filename of over 170 characters then convert it to .cbz

Expected Behavior

Comics archive should open normally

Actual Behavior

Bug is similar to #193 When I toggle devtools it shows net::ERR_FILE_NOT_FOUND for each pages.

This error only happens with .cbz archives but doesn't occur with folders when extracted with the exact same long filenames which is weird.

I tested multiple files and found out this error only starts with filenames with over 170 characters (I used an online character counter while testing)

Error message

net::ERR_FILE_NOT_FOUND

Additional Information

No response

Sample file

No response

@himouto himouto added the bug label Jun 21, 2024
@himouto
Copy link
Author

himouto commented Jun 21, 2024

Just want to clarify the Preflight Checklist and mention of bug #193

The issues were just similar but not the exact same match

@ollm
Copy link
Owner

ollm commented Jun 24, 2024

It seems to be an electron/chromium bug, I have opened an issue in the electron repository electron/electron#42634

@ollm ollm self-assigned this Jun 24, 2024
@ollm
Copy link
Owner

ollm commented Jun 30, 2024

In the end it turns out that it is a problem with long paths in Windows (Equal or greater than 260 characters), the reason it only happens with compressed files is because the final path can be a little longer when extracted to the Tmp folder.

Example, in this case the Tmp path has 40 character more:

C:\Users\Llopart\Documents\Manga\LongFolderName
C:\Users\Llopart\AppData\Local\Temp\opencomic\24f634c70f0dce47df2ded2510722c6c7aba675c\LongFolderName

I have decided that in the event that the path is equal or greater than 260 characters, the short path of the file will be generated (Windows 8.3 short filename), NTFS supports these short paths (If it has not been manually disabled), so it should solve this issue in most cases 884bccd

Example of the 8.3 short filename, in this case has 19 characters less:

C:\Users\Llopart\AppData\Local\Temp\OPENCO~1\24F634~1\LongFolderName

Build with fix: https://mega.nz/file/SGh1FCgC#rjRqBd1aKsLeuzKKGZ3OsJ5mYJ8PWqtdBVp6u8VtM5c

@himouto
Copy link
Author

himouto commented Jul 1, 2024

In the end it turns out that it is a problem with long paths in Windows (Equal or greater than 260 characters), the reason it only happens with compressed files is because the final path can be a little longer when extracted to the Tmp folder.

Example, in this case the Tmp path has 40 character more:

C:\Users\Llopart\Documents\Manga\LongFolderName C:\Users\Llopart\AppData\Local\Temp\opencomic\24f634c70f0dce47df2ded2510722c6c7aba675c\LongFolderName

I have decided that in the event that the path is equal or greater than 260 characters, the short path of the file will be generated (Windows 8.3 short filename), NTFS supports these short paths (If it has not been manually disabled), so it should solve this issue in most cases 884bccd

Example of the 8.3 short filename, in this case has 19 characters less:

C:\Users\Llopart\AppData\Local\Temp\OPENCO~1\24F634~1\LongFolderName

Build with fix: https://mega.nz/file/SGh1FCgC#rjRqBd1aKsLeuzKKGZ3OsJ5mYJ8PWqtdBVp6u8VtM5c

Thanks so much for this

@ollm ollm added this to the v1.3.0 milestone Aug 11, 2024
@dajotim937
Copy link

dajotim937 commented Aug 19, 2024

I have one of your build after your fix for this issue.
image
For these files: https://nyaa.si/view/1828761
It's second time I caught this error.

And if I manually extract files into folder and try to open just images, then I get blank pages.

@ollm
Copy link
Owner

ollm commented Aug 22, 2024

It seems to be the same error as #251

I have made these changes to fix it 549b54c

Build: https://mega.nz/file/iX4XWbxS#OdWlWLY3-P0FV_wPAe8ypxwgEVrMiPUMPAq27dJJWg0

@dajotim937
Copy link

Indeed, problem with cbz.
But if I extract files and trying to open folder, not cbz, I'm getting bunch of these (same error like in first comment):
2024-08-22_13-02-24

If it's important, I've already enabled 'LongPathsEnabled' in registry.

@ollm
Copy link
Owner

ollm commented Aug 23, 2024

It seems that the Windows short path solution does not work in this case, probably because the HDD where the images are located does not have short path enabled.

I've made some new changes to fix this 7b1bbdf, when a path of less than 260 characters cannot be generated, if sharp interpolation method is enabled, the \\?\ prefix is ​​added to the path (This is enough for sharp), in case interpolation is disabled or you're viewing the image in its original size, I've had to make it copy the image to tmp folder.

Build: https://mega.nz/file/rbZEgbRI#rGbM2her1XlWFlQf9ZbAuJZSt8NOiOo359TQOMr8w-A

@dajotim937
Copy link

Yeah it's work now.

I've had to make it copy the image to tmp folder.

Depends on what's under the hood, eventually, it probably would be better to find better way to handle that. I mean sometimes such folder with long names is vol (like in my link above) and copy 300-600 mb to tmp for each folder it's kinda costly, I guess.

But it's just thinking out loud, because it's up to you of course.

@ollm
Copy link
Owner

ollm commented Aug 24, 2024

Depends on what's under the hood

Copied files are handled the same way as files extracted from CBZ, CBR, etc. As long as the time/size set in the app settings are not exceeded for the tmp folder, the files are kept (Most likely it will only be copied once, unless the maximum size of the tmp is small or 0GB), once some of the 2 are exceeded, the files that have not been used for the longest time will be deleted.

I mean sometimes such folder with long names is vol (like in my link above) and copy 300-600 mb to tmp for each folder it's kinda costly, I guess.

Copying is not done with all images at once, when reading only the first 5-10 adjacent images are rendered, and as the reading progresses the rest, also if you have interpolation activated, it is likely that the copying will not be done during the entire reading.

eventually, it probably would be better to find better way to handle that

A possible better option would be to create a Blob object for these cases, which is what is used for sharp interpolated images, I will do some tests to see if this is better option or not.

@dajotim937
Copy link

A possible better option would be to create a Blob object for these cases, which is what is used for sharp interpolated images, I will do some tests to see if this is better option or not.

I'm not familiar with electron\js\ts, so is there a reason why it can copy files with long path to tmp but can't display them without short path when path.length>260?
If blob object exists only in RAM without copying to tmp that would be a good alternative, I guess. Yeah, depends on performance of course.

@ollm
Copy link
Owner

ollm commented Aug 25, 2024

I'm not familiar with electron\js\ts, so is there a reason why it can copy files with long path to tmp but can't display them without short path when path.length>260?

This is because electron combines node.js and chromium, the main process, which is where I am doing the copy/reading of the file is handled by node.js, but the rendering part is handled by chromium, node.js handles long paths but chromium does not.

If blob object exists only in RAM without copying to tmp that would be a good alternative, I guess. Yeah, depends on performance of course.

I've been looking and blob files should be kept in memory up to a certain size (2GB - 4GB Chrome's Blob Storage System Design), if it is exceeded then they are also saved as temporary files.

@ollm
Copy link
Owner

ollm commented Oct 4, 2024

Closed as fixed and available in v1.3.0

@ollm ollm closed this as completed Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants