Ebook editor silently overwrites read-only ebook files

[theducks]

Originally Posted by falsifier

How long does it usually take the moderators (there's 10 of them listed?), to review the posts before they appear here? The first I sent appeared in minutes but I sent a reply over 12 hours ago and it's still not here… Not a great experience for a new member.

Sorry to not have a life or go to sleep. Ya know, I might be off READING

The Mod list is from day 1. Not all are active (or even alive now). Our coverage is very good as we have folk in many different Timezones. We take the day-to-day load off the Greenies (Do use the Report Post button if the Guidelines have been violated as they are the only ones that can solve that)

Take a few (and read the various threads) and you will see who is still active frequently.

Your post has been approved 06:45, now that I have Caffeinated

[falsifier]

Quoth, theducks, thanks for explanations, I'll try to be more patient about it. And, silly me, I didn't even realize it's Christmas Eve, probably because I'm not a christian -- Merry Christmas to all who are then!

[lomkiri]

Originally Posted by falsifier

Here's a patch for the problem Lomkiri originally reported.

Or maybe something like this ?

== boss.py 
(line 1331)
        def do_save(c, path, tdir):
-           save_container(c, path)
+           save_container(c, path, save_copy=True)
            shutil.rmtree(tdir, ignore_errors=True)

== save.py
(line 28)
-def save_container(container, path):
+def save_container(container, path, save_copy=False):
    if container.is_dir:

(line 35)
        fno = temp.fileno()
-        st = None
+        st = mode = None
        try:
            st = os.stat(path)
        except OSError as err:
            if err.errno != errno.ENOENT:
                raise
            # path may not exist if we are saving a copy, in which case we use
            # the metadata from the original book
            try:
                st = os.stat(container.path_to_ebook)
            except OSError as err:
                if err.errno != errno.ENOENT:
                    raise

                # Somebody deleted the original file
        if st is not None:
            try:
-               os.fchmod(fno, st.st_mode | stat.S_IWUSR)
+               if save_copy:
+                   # if the edited ebook is read-only, temporarily change its right to be able to save a copy 
+                   os.fchmod(fno, (mode := st.st_mode) | stat.S_IWUSR)
+               else:
+                   os.fchmod(fno, st.st_mode)
            except OSError as err:
                if err.errno != errno.EPERM:
                    raise
                raise OSError('Failed to change permissions of {} to {} ({}), with error: {}. Most likely the {} directory has a restrictive umask'.format(
                    temp.name, oct(st.st_mode), format_permissions(st.st_mode), errno.errorcode[err.errno], os.path.dirname(temp.name)))
            try:
                os.fchown(fno, st.st_uid, st.st_gid)
            except OSError as err:
                if err.errno not in (errno.EPERM, errno.EACCES):
                    # ignore chown failure as user could be editing file belonging
                    # to a different user, in which case we really can't do anything
                    # about it short of making the file update non-atomic
                    raise

-   temp.close()
-   temp = temp.name
    try:
+       if save_copy and mode:
+           os.fchmod(fno, mode)
        container.commit(temp)
        atomic_rename(temp, path)
    finally:
+       temp.close()
+       temp = temp.name
        if os.path.exists(temp):
            os.remove(temp)

[kovidgoyal]

Originally Posted by falsifier

Why are you even thinking I'm editing read-only files? Of course I don't edit them. But I do open and keep them open so I can quickly consult them while indeed editing other ones.

That's what a viewer is for. If you open it in an editor, calibre assumes, rightly, that you want to EDIT the file.

I make them write-protected on purpose, to make sure they stay unchanged, because I open them in an editor after all, and s#%t happens. You make a mistake and edit wrong file, and then too quickly click those “Save” and “Close” buttons, giving yourself no chance to revert the changes. Or your cat steps on the ‘Delete’ key on your keyboard while you went to the loo and you don't notice when you're back. You do know Murphy's law, don't you? That read-only file attribute is supposed to protect you from such things, so it's just not right to silently ignore/remove that protection and overwrite files without owner's knowledge and approval…

Yes, sure, and cosmic rays can hit your harddrive and edit your read only file as well. Maybe you should engrave them on stone tablets to make sure they arent accidentally edited. But then the ghost of Ozymandias could show up and edit that as well. Hmm a conundrum.

Here's a patch for the problem Lomkiri originally reported. With this applied, if the original file is write-protected, ‘Save’ will fail with the ‘Could not save’ error message, but ‘Save a copy’ will succeed. I think it works OK but I only tested it with epubs, and I'm not a proper Python programmer, so someone better verify it.

Your patch is insufficient. Read what I said. This change was deliberate to prevent people from losing editing work if they happen to accidentally open a read only file, you know if their cat steps on the keyboard and makes the file read only.

So you need to *warn* the user and allow the user to override, not fail to save, if you want your patch accepted.

[Bradles]

Originally Posted by kovidgoyal

That's what a viewer is for. If you open it in an editor, calibre assumes, rightly, that you want to EDIT the file.

The OP may be opening the read only files in the editor to see the CODE. "How was this done?" "I'll copy that into this other book."

Your original change to overwrite read-only files introduced a very unexpected behaviour, and was not even what the requestor asked for.

[kovidgoyal]

Originally Posted by Bradles

The OP may be opening the read only files in the editor to see the CODE. "How was this done?" "I'll copy that into this other book."

Which can also be seen in the viewer.

Your original change to overwrite read-only files introduced a very unexpected behaviour, and was not even what the requestor asked for.

My original change had nothing to do with this OP.

[theducks]

Use the Bug icon in the Viewer to see the code.

[kovidgoyal]

And here you go now you get a warning before saving ass well as on opening: https://github.com/kovidgoyal/calibr...91735307251240

[falsifier]

Originally Posted by theducks

Use the Bug icon in the Viewer to see the code.

I did try it but it's not even remotely as convenient as accessing all the internals of a bookfile through the Editor. Can you even see the contents of an .opf or .ncx file in the Viewer at all? I certainly can't find them there. And frankly, I don't care, since in the Editor I'm always just a double-click away of accessing the raw contents of any of all the files inside the ebook I need. So I'm just gonna keep opening my read-only files in the Editor (especially now that that botched change introduced in version 5.37 has been fixed), even though calibre assumes, wrongly, that I want to edit them.

Originally Posted by kovidgoyal

And here you go now you get a warning before saving ass well as on opening: https://github.com/kovidgoyal/calibr...91735307251240

Thank you. Good to see that typing in these few new lines wasn't “too much effort for this” for you after all.

[kovidgoyal]

Originally Posted by falsifier

Thank you. Good to see that typing in these few new lines wasn't “too much effort for this” for you after all.

The change I made was not what you suggested which was indeed too much effort for an entirely made up use case, involving cats and Murphy's "law". Seriously, removing read permissions on files before editing them is the most absurd thing I have ever heard.

Popping up a warning before saving, rather than dealing with an error after saving, specially for you, is within my "jump through hoops to accommodate idiotic use cases, while still trying to get useful work done" budget.

[Bradles]

Originally Posted by kovidgoyal

The change I made was not what you suggested which was indeed too much effort for an entirely made up use case, involving cats and Murphy's "law". Seriously, removing read permissions on files before editing them is the most absurd thing I have ever heard.

Popping up a warning before saving, rather than dealing with an error after saving, specially for you, is within my "jump through hoops to accommodate idiotic use cases, while still trying to get useful work done" budget.

Wow! Get up on the wrong side of the bed this morning? Your online persona can be quite brusque, to say the least. I don't think the OP deserved such an insulting response for a perfectly valid concern.

My comment about the requestor was not referring to the OP of this thread, but the requestor of the change to the "save" function:

Originally Posted by lomkiri

I am the author of this request, but in fact, my concern was with "save-as", not "save". I agree with Falsifier that it shouldn't be possible to overwrite a RO file, not without warning, at least, but my problem was that it wasn't possible to "save as" to a new file either, which would be the expected behavior. So I've lost my work on that epub. The change you've made at that time solved this issue, but a side-effect was that RO files were not protected anymore (I've noticed it at that time, but said nothing, I didn't want to bother you).

So not only have you done something dumb, it was for no good reason.

[kovidgoyal]

[QUOTE=Bradles;4284528]Wow! Get up on the wrong side of the bed this morning? Your online persona can be quite brusque, to say the least. I don't think the OP deserved such an insulting response for a perfectly valid concern.
[quote]

Then I suggest you and the OP stop interacting with me. You are not doing me a favor by asking me to do work for you, you do realize that dont you?

So not only have you done something dumb, it was for no good reason.

YOU think it was something dumb, I disagree.

[JSWolf]

I have never saved an eBook in the editor that I didn't want saved. When I close the editor and I have made some sort of change, I click the button that reads Quit without saving.

[DNSB]

One of the few times I find myself agreeing with Jon.

Not to mention that I would never depend on a software flag to keep a file from being modified or overwritten. That's what creating a backup before editing is for.

[JSWolf]

Originally Posted by DNSB

One of the few times I find myself agreeing with Jon.

Not to mention that I would never depend on a software flag to keep a file from being modified or overwritten. That's what creating a backup before editing is for.

I use Modify ePub before editing in most cases. It creates a backup and I have occasionally used the backup to double check something or fix something I screwed up.