search

nomacs / nomacs

nomacs is a free image viewer for windows, linux, and mac systems.
https://nomacs.org/
GNU General Public License v3.0
1.98k stars 152 forks source link

Nomacs silently modifies files on disk, destroying timestamp and metadata information. #799

Closed PhilSalkie closed 3 months ago

PhilSalkie commented 3 years ago

Describe the bug

If a user triggers a "Rotate" by pressing the "R" key, the file on disk is silently modified. This changes the modification date on the file and changes the file size, all without the knowledge or consent of the user. The change is immediate, silent, and irreversable, and the "Undo" function is ignored after an "R" key rotation.

To Reproduce

ls -ltr imagefile.png

Notice the time/date stamp and size.

nomacs imagefile.png

Click on image, press "R" key. Image rotates as expected. Close Nomacs.

ls -ltr imagefile.png

Notice that the time/date stamp is changed to just a few seconds ago, and that the file size may have changed as well.

Desktop (please complete the following information):

Additional context

There may be other operations which a user would consider "safe" which would destroy timestamp / metadata information on disk. I have only just found out about the "R" key rotation - that discovery caused an immediate

sudo apt-get remove nomacs

from all my systems, so I won't be looking for more hidden file mangling commands.

PhilSalkie commented 3 years ago

It has been pointed out elsewhere that it is possible in the advanced settings to disable "Save Exif Orientation" in order to keep nomacs from automatically writing to a user's files without warning.

While this does stop the files from being silently modified, it disables a useful function - lossless jpg rotation. The point is, the files should not be silently written to - especially since their existing modification time metadata is discarded, and for .png files, their file size can be massively expanded. The change to exif data should be held in memory until a save is triggered, just like any other change.

If an "auto-save after rotation" check box were added to allow easier mass modifications to the rotation of files, that would be a good answer to the problem - but it should not be turned on by default, and should display some information on-screen to show that files are being modified on disk, such as a non-modal popup saying "File Saved".

SteveHallArchitecture commented 3 years ago

I just noticed this myself. No program should ever change files on disk without prompting!

Nomacs needs a read-only or viewer mode where a user can have confidence that a file is not going to be edited. This one behavior makes the entire program feel very suspicious.

PhilSalkie commented 3 years ago

Glad I'm not alone! Fortunately, I didn't find this out by damaging a file in an embedded system image, or in a forensic analysis situation

On 10/15/21, Steve Hall @.***> wrote:

I just noticed this myself. No program should every change files on disk without prompting!

Nomacs needs a read-only or viewer mode where a user can have confidence that a file is not going to be edited. This one behavior makes the entire program feel very suspicious.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/nomacs/nomacs/issues/799#issuecomment-944580475

-- The thing I love about diving is the flowing feeling. I like a sport where the whole point is to move as little as humanly possible so your air supply will last longer. That's my kind of sport. Where the amount of effort spent is absolutely minimal.

DJCrashdummy commented 2 years ago

well... i was about to point out the same "workaround" as @PhilSalkie to disable "Save Exif Orientation" in the advanced settings.

if there is a function to save changes without asking, this should be a separate (opt-in) option. not to mention that it is pretty unfortunate that this auto-save is on by default (but this IMHO no-go gets sadly more and more common). :unamused:

c0xc commented 2 years ago

Wouldn't it be better to have this option, but keeping the file's timestamp? I'm mostly annoyed be the feature because when I sort my files by date, they're out of order. But if the rotated file would have the old modification date, it could actually be quite useful...?

PhilSalkie commented 2 years ago

It's a file viewer. No viewer should ever modify a file on disk without the user's knowledge.

Think of forensics, think of file packaging, software version control, file SHA256 sums - you've just changed the source material just by looking at it, now the file doesn't match the original - and you don't know that it's been changed. Maybe a file is supposed to be 90 degrees out because it's supposed to be shown on an LCD display that's mounted sideways (like a lot of video signage.) You're working on a system, you look at the image and rotate it to fit your screen, now the image doesn't display properly anymore because Nomacs has just silently re-saved it with different rotation information. Oh, and at next startup, the system flags an error because the directory checksum is wrong.

If there's such a mode, which certainly could be quite useful when sorting and saving files, the user should have to enable it, and there should be some visible indicator whenever the file's modified, like a non-modal pop-up saying "Overwriting Original File with New Rotation Information" that shows up for a second, then clears away so you can continue working.

As it is, the viewer software isn't a viewer, it's a stealth file editor. There's some tick-box to disable saving the EXIF rotation information, but that's just a work-around, because then when you want to save a rotated file, it moves the pixels instead of changing the metadata. The real problem is that it shouldn't just be writing things back to disk without letting anyone know - and there may be other things like that which it does, I haven't gone digging to find out.

On 1/20/22, c0xc @.***> wrote:

Wouldn't it be better to have this option, but keeping the file's timestamp? I'm mostly annoyed be the feature because when I sort my files by date, they're out of order. But if the rotated file would have the old modification date, it could actually be quite useful...?

-- Reply to this email directly or view it on GitHub: https://github.com/nomacs/nomacs/issues/799#issuecomment-1017815689 You are receiving this because you were mentioned.

Message ID: @.***>

-- The thing I love about diving is the flowing feeling. I like a sport where the whole point is to move as little as humanly possible so your air supply will last longer. That's my kind of sport. Where the amount of effort spent is absolutely minimal.

c0xc commented 2 years ago

Maybe a file is supposed to be 90 degrees out because it's supposed to be shown on an LCD display that's mounted sideways (like a lot of video signage.) You're working on a system, you look at the image and rotate it to fit your screen, now the image doesn't display properly anymore because Nomacs has just silently re-saved it with different rotation information. Oh, and at next startup, the system flags an error because the directory checksum is wrong.

I do agree! It should be opt-in.

If there's such a mode, which certainly could be quite useful when sorting and saving files, the user should have to enable it, and there should be some visible indicator whenever the file's modified, like a non-modal pop-up saying "Overwriting Original File with New Rotation Information" that shows up for a second, then clears away so you can continue working.

I'm not a fan of things that show up for one second and disappear, but that's a matter of taste. It could just be a hint in the status bar, indicating that the image has been changed and will be saved automatically (or, if that's not enabled, the user could be asked "do you want to save your changed?").

The real problem is that it shouldn't just be writing things back to disk without letting anyone know

Yes, indeed...

PhilSalkie commented 2 years ago

(Anyway, this thing isn't being actively developed, so it's unlikely to ever be resolved.)

On 1/20/22, c0xc @.***> wrote:

Maybe a file is supposed to be 90 degrees out because it's supposed to be shown on an LCD display that's mounted sideways (like a lot of video signage.) You're working on a system, you look at the image and rotate it to fit your screen, now the image doesn't display properly anymore because Nomacs has just silently re-saved it with different rotation information. Oh, and at next startup, the system flags an error because the directory checksum is wrong.

I do agree! It should be opt-in.

If there's such a mode, which certainly could be quite useful when sorting and saving files, the user should have to enable it, and there should be some visible indicator whenever the file's modified, like a non-modal pop-up saying "Overwriting Original File with New Rotation Information" that shows up for a second, then clears away so you can continue working.

I'm not a fan of things that show up for one second and disappear, but that's a matter of taste. It could just be a hint in the status bar, indicating that the image has been changed and will be saved automatically (or, if that's not enabled, the user could be asked "do you want to save your changed?").

The real problem is that it shouldn't just be writing things back to disk without letting anyone know Yes, indeed...

-- Reply to this email directly or view it on GitHub: https://github.com/nomacs/nomacs/issues/799#issuecomment-1017846598 You are receiving this because you were mentioned.

Message ID: @.***>

-- The thing I love about diving is the flowing feeling. I like a sport where the whole point is to move as little as humanly possible so your air supply will last longer. That's my kind of sport. Where the amount of effort spent is absolutely minimal.

c0xc commented 2 years ago

I took the time to analyze this behavior and write a fix for it. It's a work in progress, I still need to make the notes field actually do something when the user clicks save. Otherwise those notes would be lost because I've disabled the implicit auto-save call(s).

If someone wants to read my notes, I've saved them here for now: https://gist.github.com/c0xc/9c0e6eb8bc23e1998823e9a94de63afe

I think, when I'm done, I'll remove it and copy the relevant pieces of my text here. Or elsewhere, not sure. And some part of that goes to bug report #804, which matches my definition of "minor" changes quite well. I'm going to implement that as well (opt-in).

I'm not really sure who'll want to review my changes and check my pull request when I'm done, but let's see how far I get.

PhilSalkie commented 2 years ago

Thank you! I had a look at your notes - certainly a more complex issue than it seems on the surface.

On 2/6/22, c0xc @.***> wrote:

I took the time to analyze this behavior and write a fix for it. It's a work in progress, I still need to make the notes field actually do something when the user clicks save. Otherwise those notes would be lost because I've disabled the implicit auto-save call(s).

If someone wants to read my notes, I've saved them here for now: https://gist.github.com/c0xc/9c0e6eb8bc23e1998823e9a94de63afe

I think, when I'm done, I'll remove it and copy the relevant pieces of my text here. Or elsewhere, not sure. And some part of that goes to bug report #804, which matches my definition of "minor" changes quite well. I'm going to implement that as well (opt-in).

I'm not really sure who'll want to review my changes and check my pull request when I'm done, but let's see how far I get.

-- Reply to this email directly or view it on GitHub: https://github.com/nomacs/nomacs/issues/799#issuecomment-1030942978 You are receiving this because you were mentioned.

Message ID: @.***>

-- The thing I love about diving is the flowing feeling. I like a sport where the whole point is to move as little as humanly possible so your air supply will last longer. That's my kind of sport. Where the amount of effort spent is absolutely minimal.

c0xc commented 2 years ago

I've made a couple of minor but relevant changes to the comment panel which had some sort of odd behavior (rotating or changing the image would reset the comment, in other cases it could even be cleared instead of being reset to the original text). Clicking on "Save Note" now explicitly marks the image as edited, just like after rotating it.

I've created a pull request but I'm still testing a bit and I think I'll add some option to retain the file date in some cases. https://github.com/nomacs/nomacs/pull/837

Now, I'm waiting for feedback from other developers (I have modified quite a few places in the code).

c0xc commented 1 year ago

@PhilSalkie My fix was merged some time ago. To try it, download the latest dev version. Here's a download link for Windows, the link will expire. And if it works well, please close this issue. I hope it works ok for you because I'm now busy with other projects... There are already duplicates like issue #936.

nickisashkir commented 1 year ago

Thank you for this. I'm on the flip side and was actually looking for something that can silently rewrite the file when rotated. So now I know I can download an older source code :D

leejuyuu commented 3 months ago

Closing as fixed by https://github.com/nomacs/nomacs/pull/837.