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

Add option to write to kdbx databases #5

Merged
merged 1 commit into from Aug 1, 2017

Conversation

wget
Copy link
Contributor

@wget wget commented Jul 28, 2017

Sometimes, we need to write to kdbx file. This library doesn't allow this ability without having to declare yet another file descriptor, just to be able to write.

This patch adds the ability to reuse the existing file descriptor opened in pykeepass's open context manager.

@Evidlo
Copy link
Member

Evidlo commented Jul 28, 2017

I think a better solution might be something like this:

def open(filename, mode='rw', **credentials):
    """
    A contextmanager to open the KeePass file with `filename`. Use a `password`
    and/or `keyfile` named argument for decryption.
    
    Files are identified using their signature and a reader suitable for 
    the file format is intialized and returned.
    
    Note: `keyfile` is currently not supported for v3 KeePass files.
    """
    kdb = None
    try:
        with io.open(filename, mode) as stream:
            signature = common.read_signature(stream)
            cls = get_kdb_reader(signature)
            kdb = cls(stream, **credentials)
            yield kdb
            kdb.close()
    except:
        if kdb:
            kdb.close()
        raise

@wget
Copy link
Contributor Author

wget commented Jul 28, 2017

@Evidlo Ok done as well.

@Evidlo
Copy link
Member

Evidlo commented Jul 30, 2017

The mode should actually be 'rb+' as per the io documentation.

Be sure to test your code before making a PR. I'm partially at fault too for approving it before testing.

Copy link
Member

@pschmitt pschmitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update that to rb+

@wget
Copy link
Contributor Author

wget commented Jul 30, 2017

@pschmitt Done.
@Evidlo Btw, I was using r+b in my previous PR and I tested it. But yes, I have to admit I haven't tested for rw nor rb+ (only r+b has been tested).

@pschmitt
Copy link
Member

pschmitt commented Aug 1, 2017

thanks!

@pschmitt pschmitt merged commit 77054be into libkeepass:master Aug 1, 2017
crass pushed a commit to crass/libkeepass that referenced this pull request Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants