Bug 440986 - Okular is able to overwrite read-only files
Summary: Okular is able to overwrite read-only files
Status: RESOLVED FIXED
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: 20.12.3
Platform: Debian testing Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-14 22:04 UTC by Adriano Vilela
Modified: 2021-09-16 20:02 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adriano Vilela 2021-08-14 22:04:36 UTC
SUMMARY

I came across a very weird behavior while annotating a pdf file in Okular. Long story short: I opened a read-only pdf file (permissions: 400), inserted some comments and hit the save button. At this point, I thought I had been working on a write-enabled copy of the file. After a while, I realized that I was actually working on the read-only version of the file, that somehow got saved to disk when I hit the save icon. Okular was not only able to save the file to disk, but the file permissions were changed to 644.

To be honest, I was able to reproduce the problem with Xournal. This makes me think that the problem may not be with Okular or Xournal, but with some common library used by both of these packages (maybe libpoppler?).

I reported this on a Debian mailing list (I'm using Debian Testing), and somebody suggested that this probably happens because Okular is saving the modifications to a temporary file and then deleting the original file and writing the temporary file to a new file with the same name as the original file. I understand that. However, I think that this behavior is unexpected and very problematic.

STEPS TO REPRODUCE

1. Open a read-only file in Okular
2. Insert some comments on the file
3. Hit the save button

OBSERVED RESULT

The file gets saved to disk, even though it is marked as read-only.

EXPECTED RESULT

Okular should show an error message saying it can't write to the file.


SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: Debian Testing
(available in About System)
KDE Plasma Version: 5.20.5
KDE Frameworks Version: 5.78.0
Qt Version: 5.15.2

ADDITIONAL INFORMATION
Comment 1 Paul Worrall 2021-08-15 08:41:35 UTC
I can reproduce this surprising bug on Neon User edition:

Okular 21.08.0

Operating System: KDE neon 5.22
KDE Plasma Version: 5.22.4
KDE Frameworks Version: 5.84.0
Qt Version: 5.15.3
Kernel Version: 5.11.0-25-generic (64-bit)
Graphics Platform: X11
Processors: 2 × AMD A6-6400K APU with Radeon(tm) HD Graphics
Memory: 7.7 GiB of RAM
Graphics Processor: AMD CEDAR
Comment 2 David Hurka 2021-08-15 19:07:01 UTC
(In reply to Adriano Vilela from comment #0)
> [...] Okular is saving the
> modifications to a temporary file and then deleting the original file and
> writing the temporary file to a new file with the same name as the original
> file.

This is AFAIK the only way of saving files that is used by existing applications. Modifying files in-place maybe makes sense for databases, which Okular isn’t.

Anyway, apparently other applications do not overwrite files with mode 0400. (Tested with nano and Kate.)

And the problem is not Poppler, because the same behavior occurs with other file types than PDF too.
Comment 3 argonel 2021-08-16 05:38:04 UTC
(In reply to Adriano Vilela from comment #0)

> I reported this on a Debian mailing list (I'm using Debian Testing), and
> somebody suggested that this probably happens because Okular is saving the
> modifications to a temporary file and then deleting the original file and
> writing the temporary file to a new file with the same name as the original
> file. I understand that. However, I think that this behavior is unexpected
> and very problematic.

That description is almost correct. What actually happens is that the file is saved as a new file which is then renamed to have the old file's name. This method of saving a file is to preserve the original data if something goes wrong during save, taking advantage of the POSIX rules regarding renaming files.

A discussion about why the lack of a "write" permission doesn't make a file "read-only" belongs elsewhere, but the "write" permission doesn't apply to the file's name (or any of its other metadata).

A proper solution for truly making files "read-only" isn't in Okular's scope, but to be friendly Okular could check to see if the file is "writable" and prompt if it isn't.


(In reply to David Hurka from comment #2)

> This is AFAIK the only way of saving files that is used by existing
> applications. Modifying files in-place maybe makes sense for databases,
> which Okular isn’t.

The lack of a "write" permission actually _prevents_ "in-place" modification, so technically the behavior here is correct.
Comment 4 Paul Worrall 2021-08-16 10:45:42 UTC
I'm struggling to find any other applications that allow editing files which do not have write permission. I've tried Kate, LibreOffice Write, Kolour Paint, Gwenview, Inkscape, Karbon, Calligra Words, ... none of those allow it.
Comment 5 Nate Graham 2021-08-16 18:21:04 UTC
(In reply to argonel from comment #3)
> A discussion about why the lack of a "write" permission doesn't make a file
> "read-only" belongs elsewhere, but the "write" permission doesn't apply to
> the file's name (or any of its other metadata).
> 
> A proper solution for truly making files "read-only" isn't in Okular's
> scope, but to be friendly Okular could check to see if the file is
> "writable" and prompt if it isn't.
Yes, that would seem to make sense. Or just prevent it entirely.
Comment 6 gszymaszek 2021-08-23 14:22:41 UTC
FWIW, here’s how TeXstudio explains the issue to the user:

> This uses QSaveFile to prevent losing existing data if the writing
> operation fails. As a drawback, the current user becomes the owner of
> the file and extended file attributes are lost. Also, there appear to
> be problems of this method with dropbox folders.

https://sources.debian.org/src/texstudio/3.0.4+ds-1/src/configdialog.ui/#L2191
Comment 7 Bug Janitor Service 2021-09-09 14:54:36 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/okular/-/merge_requests/480
Comment 8 Albert Astals Cid 2021-09-16 20:02:39 UTC
Git commit d9d09ef7388e9582d3882d3b2fb4969700baf1a2 by Albert Astals Cid.
Committed on 16/09/2021 at 18:50.
Pushed by aacid into branch 'release/21.08'.

Don't allow saving over read-only files

M  +9    -0    part/part.cpp

https://invent.kde.org/graphics/okular/commit/d9d09ef7388e9582d3882d3b2fb4969700baf1a2