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

entry.parentgroup doesn't work for history entries #122

Closed
BoomerB opened this issue Dec 11, 2018 · 5 comments · Fixed by #128
Closed

entry.parentgroup doesn't work for history entries #122

BoomerB opened this issue Dec 11, 2018 · 5 comments · Fixed by #128

Comments

@BoomerB
Copy link
Contributor

BoomerB commented Dec 11, 2018

Hi,

I accidentally found the following issue:

dub@gamora:~/src/tests$ python2
Python 2.7.15rc1 (default, Nov 12 2018, 14:31:15) 
[GCC 7.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from pykeepass import PyKeePass
>>> 
>>> kp = PyKeePass('tests.kdbx', 'password')
>>> 
>>> item = kp.find_entries_by_title('root_entry', first=True)
>>> print(item)
Entry: "root_entry (foobar_user)"
>>> 
>>> hist = item.history
>>> 
>>> hist_item = hist[0]
>>> print(hist_item)
Entry: "[History of: root_entry] (foobar_user)"
>>> 
>>> hist_item.parentgroup
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/pykeepass/entry.py", line 170, in parentgroup
    return pykeepass.group.Group(element=ancestor, version=self._version)
  File "/usr/local/lib/python2.7/dist-packages/pykeepass/group.py", line 39, in __init__
    'element, but a {}'.format(element.tag)
AssertionError: The provided element is not a Group element, but a Entry
>>> 

same happens with python3

dub@gamora:~/src/tests$ python3
Python 3.6.7 (default, Oct 22 2018, 11:32:17) 
[GCC 8.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from pykeepass import PyKeePass
>>> 
>>> kp = PyKeePass('tests.kdbx', 'password')
>>> 
>>> item = kp.find_entries_by_title('root_entry', first=True)
>>> print(item)
b'Entry: "root_entry (foobar_user)"'
>>> 
>>> hist = item.history
>>> 
>>> hist_item = hist[0]
>>> print(hist_item)
b'Entry: "[History of: root_entry] (foobar_user)"'
>>> 
>>> hist_item.parentgroup
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.6/dist-packages/pykeepass/entry.py", line 170, in parentgroup
    return pykeepass.group.Group(element=ancestor, version=self._version)
  File "/usr/local/lib/python3.6/dist-packages/pykeepass/group.py", line 39, in __init__
    'element, but a {}'.format(element.tag)
AssertionError: The provided element is not a Group element, but a Entry
>>> 
@BoomerB
Copy link
Contributor Author

BoomerB commented Dec 11, 2018

BTW: the representation of the items under python3 looks a bit strange, the leading "b" should not be part of the representation but indicate a type, but I'm no specialist

@BoomerB
Copy link
Contributor Author

BoomerB commented Dec 11, 2018

I have a possible solution and some tests at https://github.com/BoomerB/pykeepass
but when I try a pull request it also contains the changes for my previous pull request.

@whwright
Copy link
Contributor

BTW: the representation of the items under python3 looks a bit strange, the leading "b" should not be part of the representation but indicate a type, but I'm no specialist

I agree with this. The issue is the __str__ function actually returns bytes instead of a str.

https://github.com/pschmitt/pykeepass/blob/0b6240866bdc62d04d41ba7c86cf010ad827ce3f/pykeepass/entry.py#L270

May deserve it's own ticket

@BoomerB
Copy link
Contributor Author

BoomerB commented Dec 16, 2018

@whwright the question is: what type should be returned? is it ok to return an unicode string - which is the default for python3 anyway - or does it have to be a plain string?
the easiest would be

return 'Entry: "{} ({})"'.format(item.path, item.username)

if it has to be a plain str, then something like

s = 'Entry: "{} ({})"'.format(item.path, item.username)
if not isinstance(s, str):
    s = s.encode('utf-8')
return s

would do the trick
BTW: we probably should add some tests with non-ascii groups and entries

@Evidlo
Copy link
Member

Evidlo commented Dec 17, 2018

BTW: we probably should add some tests with non-ascii groups and entries

There are entries with cyrillic characters in one of the test databases.

Evidlo added a commit to Evidlo/pykeepass that referenced this issue Jan 10, 2019
@Evidlo Evidlo mentioned this issue Jan 10, 2019
Evidlo added a commit to Evidlo/pykeepass that referenced this issue Jan 12, 2019
Evidlo added a commit to Evidlo/pykeepass that referenced this issue Jan 12, 2019
pschmitt pushed a commit that referenced this issue Jan 16, 2019
* add test for delete_custom_property

* string fixes and python2 FIXMEs

* move `first`, `history` into _xpath, use kp._xpath everywhere

* clarify return types

* move .parentgroup to baseelement.py

* use UUID for __eq__, fix #122

* fix for when no attachments present

* perform all tests on both kdbx3 and kdbx4
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 a pull request may close this issue.

3 participants