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

unescaped slashes for entry names/paths #205

Closed
doganoo opened this issue Oct 9, 2020 · 16 comments
Closed

unescaped slashes for entry names/paths #205

doganoo opened this issue Oct 9, 2020 · 16 comments

Comments

@doganoo
Copy link

doganoo commented Oct 9, 2020

Hey there,

thank you for creating this awesome library!

I have noticed that entry names are not escaped. This leads to wrong path names, as you can see here:

Bildschirmfoto 2020-10-09 um 14 37 06

Bildschirmfoto 2020-10-09 um 14 38 03

Test/Entry is a password in Test/datenbank within the path ../Datenbanken/

Is it possible to escape those names? Or any known workarounds for this?

Thanks in advance!

EDIT

database = PyKeePass("", "") 

for group in database.groups:
    print(group.name)
    print("----------------")
    for entry in group.entries:
        print(entry.path)
@pschmitt
Copy link
Member

pschmitt commented Oct 9, 2020

How do you create these entries?

@doganoo
Copy link
Author

doganoo commented Oct 9, 2020

How do you create these entries?

using KeepassXC "new group"

@KeyWeeUsr
Copy link
Contributor

I'm thinking it's not an issue of PyKeePass but rather of KeepassXC and it's way of inserting entries.

>>> from pykeepass import PyKeePass as P
>>> kp = P("<somewhere>", password="pass")
>>> kp.entries[17].group.group.group.group.name
'Root'
>>> kp.entries[17].group.group.group.name
'foobar_group'
>>> kp.entries[17].group.group.name
'subgroup/name'
>>> kp.entries[17].group.name
'hello/hi'
>>> kp.entries[17].title
'My/super/entry'
>>> kp.entries[17].group.path
'foobar_group/subgroup/name/hello/hi/'
>>> kp.entries[17].group.group.path
'foobar_group/subgroup/name/'
>>> kp.entries[17].group.group.group.path
'foobar_group/'
>>> kp.entries[17].group.group.group.group.path
'/'
>>> kp.add_entry(kp.entries[17].group, title="path/test", username="user/name", password="pass/word")
Entry: "foobar_group/subgroup/name/hello/hi/path/test (user/name)"
>>> kp.entries[18]
Entry: "foobar_group/subgroup/name/hello/hi/path/test (user/name)"
>>> kp.entries[18].username
'user/name'
>>> kp.entries[18].password
'pass/word'
>>> kp.entries[18].title
'path/test'
>>> kp.entries[18].path
'foobar_group/subgroup/name/hello/hi/path/test'
>>> kp.entries[18].group.path
'foobar_group/subgroup/name/hello/hi/'
>>> kp.find_groups(name="hello/hi", first=True)
Group: "foobar_group/subgroup/name/hello/hi/"

@Evidlo
Copy link
Member

Evidlo commented Oct 10, 2020

The KeePass spec allows / inside of entry titles and group names, and since we use it as a delimiter, things can be a bit confusing.

@KeyWeeUsr
Copy link
Contributor

But then it's as good delimiter as any other due to keepass allowing almost everything in there. We can do spaces between the paths or choose a new delimiter (e.g. > )and the same issue will be opened later.

We might also do some bracketing for visibility e.g.:

[Root]/[Group/with/slash]/[Title]

@Evidlo
Copy link
Member

Evidlo commented Oct 10, 2020 via email

@KeyWeeUsr
Copy link
Contributor

@Evidlo Not necessarily, we can just add a new attribute and keep the original path in place. This way it won't be a breaking change and people who'd want to use the list would be able to.

@doganoo
Copy link
Author

doganoo commented Oct 10, 2020

@Evidlo whats about represeting as a graph?

@KeyWeeUsr KeyWeeUsr mentioned this issue Oct 12, 2020
@KeyWeeUsr
Copy link
Contributor

@doganoo Do you mean as a CLI tool like in #207 ?

@doganoo
Copy link
Author

doganoo commented Oct 12, 2020

@doganoo Do you mean as a CLI tool like in #207 ?

@KeyWeeUsr Yes exactly :-)

When do you plan to merge this?

@Evidlo
Copy link
Member

Evidlo commented Nov 17, 2020

I think I'll make this a breaking change and release it with the next major version.

@doganoo
Copy link
Author

doganoo commented Nov 17, 2020

cool, do you have a timeline for that?

@Evidlo
Copy link
Member

Evidlo commented Nov 18, 2020

Check out https://github.com/libkeepass/pykeepass/tree/pathlist and let me know what you think.

I may do a major release in the next couple of weeks.

@doganoo
Copy link
Author

doganoo commented Jan 8, 2021

@Evidlo any updates here? :)

@Evidlo
Copy link
Member

Evidlo commented Jan 16, 2021

pathlist is now merged into master. Just waiting on #204 before making a release.

@spaetz
Copy link

spaetz commented Feb 7, 2021

As a downstream user I have to say that I would have preferred a fixed up path2 attribute instead of a breaking change in order to avoid version dependent-behavior changes. Especially, given that the version variable itself is not guaranteed to be there (as it was just introduced a version earlier).
That having said, we can certainly cope with the change...

@Evidlo Evidlo closed this as completed Feb 7, 2021
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.

5 participants