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

[Patch attached] Geeqie fails to copy files with file name length close to linux maximum #1544

Open
installgentoo opened this issue Nov 23, 2024 · 9 comments
Labels

Comments

@installgentoo
Copy link

Setup (please complete the following information):

  • Distribution: gentoo
  • Geeqie version [geeqie --version]: Geeqie 2.4 GTK3

Describe the bug
Ditto.

To reproduce
Name a file with 255 characters, try to copy it. Get a nondescript error.

Additional context
This happens because of

	std::unique_ptr<gchar, decltype(&g_free)> randname{g_strconcat(tl.get(), ".tmp_XXXXXX", NULL), g_free};

at src/ui-fileops.cc:648

I assume this is meant to safeguard against partial operations that get interrupted, ironic that the safety measure itself caused a bug. See attached patch with a fix, refactor it into geeqie code standard and merge in.
filenamelen.patch

@qarkai
Copy link
Contributor

qarkai commented Nov 24, 2024

Could you create a PR for this patch?

@qarkai
Copy link
Contributor

qarkai commented Nov 24, 2024

ironic that the safety measure itself caused a bug

Could you give more details? I don't see how unique_ptr wrapper gives nondescript error.

@installgentoo
Copy link
Author

@qarkai

appending ".tmp_XXXXXX" to filename extends the filename length past 255 characters.

@qarkai
Copy link
Contributor

qarkai commented Nov 24, 2024

@qarkai

appending ".tmp_XXXXXX" to filename extends the filename length past 255 characters.

So IIUC any g_strconcat and g_build_filename is dangerous. Also it has nothing to do with memory safety.

@qarkai
Copy link
Contributor

qarkai commented Nov 24, 2024

@caclark should we apply wrapper from patch to all relevant g_strconcat and g_build_filename code?

@installgentoo
Copy link
Author

@qarkai is there any other code that appends arbitrary strings to filenames?

If so i would think my check+truncate approach should be a function and used everywhere.

@qarkai
Copy link
Contributor

qarkai commented Nov 24, 2024

@qarkai is there any other code that appends arbitrary strings to filenames?

If so i would think my check+truncate approach should be a function and used everywhere.

Proposed patch looks like overengineering. Should be enough randname = g_build_filename(tl_dirname, ".tmp_XXXXXX", NULL);.
Again, could you create PR? Note that unique_ptr was replaced with g_auto* macros.

@qarkai
Copy link
Contributor

qarkai commented Nov 24, 2024

@installgentoo Is it related to #944?

@installgentoo
Copy link
Author

@qarkai must be. Please write PR or whatever and close that other bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants