bastibe / annotate.el

Annotate.el
Other
388 stars 20 forks source link

Added early checking encrypted (GPG) file format #96

Closed cage2 closed 3 years ago

cage2 commented 3 years ago

Hi @bastibe

Inspired by the issue reported in #94 i added the magic number check for GPG encrypted files, from the Changelog:

    In  many parts  of  the code  we  use 'annotate-guess-file-format',  a
    function that  try to guess the  format of the file  that produced the
    contents of a  buffer. Sometimes (for example buffers  that shows info
    nodes)  this check  is very  expensive because  involves reading  (and
    possibly decompress or even decrypt) the file contents and check for a
    regular expression.

    If we could  know in advance that the file  is symmetrically encrypted
    with GPG we could skip other,  more expensive checks for guessing file
    format (like info files).

I know that this is probably sort of a corner case (people who annotates encrypted files) but i found no reason to implement it if this can makes life a bit easier for someone :-) (and the code is trivial by the way ;-)).

Bye! C.

EverardNisse commented 3 years ago

While my use case is only asymmetric encryption (should've probably mentioned this), I tried to cherry-pick the commit and symmetrically encrypt 'annotations.gpg'. Saving still seems to resolve in decryption of the file even though it's already open as a buffer, much like before. It's possible my test was flawed as it wasn't isolated from my usual easypg setup, so trust your machine first.

Good work nonetheless.

cage2 commented 3 years ago

On Sun, Jan 17, 2021 at 02:42:32PM -0800, EverardNisse wrote:

Hi!

While my use case is only asymmetric encryption (should've probably mentioned this), I tried to cherry-pick the commit and symmetrically encrypt 'annotations.gpg'.

Thank you for reviewing this PR.

Saving still seems to resolve in decryption of the file even though it's already open as a buffer, much like before. It's possible my test was flawed as it wasn't isolated from my usual easypg setup, so trust your machine first.

I think your test is fine as although this PR was inspired by your issue report is actually meant for the opposite of your case: annotating encrypted files, not keeping annotations in an an encrypted database.

Sorry for this misunderstanding, but thanks anyway for testing!

Good work nonetheless.

Thank you!

The issue you reported could be (sort of) fixed using some caching, ads you suggested, i have some code that i am going to put online soon but i found that using caching could render the database corrupted on same use case, so i need to discuss this changes before proposing to be merged.

Bye! C.

bastibe commented 3 years ago

Looks good to me. Good idea, too!

cage2 commented 3 years ago

Hi @bastibe !

Looks good to me. Good idea, too!

Thank you, :) i am actually a big fan of magic number for file formats! :) Maybe we could extend this types of test for other file formats in the future, we'll see!

Bye! C.