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

IPNS's key names case sensitivity #5947

Closed
AuHau opened this issue Jan 27, 2019 · 10 comments
Closed

IPNS's key names case sensitivity #5947

AuHau opened this issue Jan 27, 2019 · 10 comments
Labels
kind/enhancement A net-new feature or improvement to an existing feature

Comments

@AuHau
Copy link
Member

AuHau commented Jan 27, 2019

Version information:

go-ipfs version: 0.4.18-
Repo version: 7
System version: amd64/darwin
Golang version: go1.11.1

Type:

bug / doc

Description:

Is IPNS key name's matching case insensitive? (By accident) I tried to create two keys with different case sensitivity and got an error that the key name already exists.

I am aware of the base-32 migration, but would not expect this to be a case for the key names. If it is not a bug, but a feature, then I would suggest document it somewhere (unless I have missed it).

@Stebalien
Copy link
Member

You mean the key name from ipfs key gen <name>? That's probably due to your filesystem. Unlike everything else (which we store in the "repo"), we store keys directly in the filesystem.

Although, really, we should consider encoding the names before storing them.

@AuHau
Copy link
Member Author

AuHau commented Jan 31, 2019

Yes, I am talking about ipfs key gen <name>.

Hmm I see, I am running macOS so thought that it is case sensitive, but after some more digging I found out that my FS is not case sensitive, so make sense.

I would be happy to provide PR for docs or encoding of the names, let me know which way to take.

@Stebalien
Copy link
Member

Encoding (base32) would be great, IMO. However, that'll require a migration: https://github.com/ipfs/fs-repo-migrations/. Luckily, it should be a pretty simple migration relative to most of the migrations in that repo.

@Kubuxu
Copy link
Member

Kubuxu commented Feb 2, 2019

I question that we want to encode it. It is just adding complexity.
I would propose that we enforce all newly created keys being lower case. This way moving between OSes is not a problem and user confusion is also reduced.

@Stebalien what do you think?

@Kubuxu Kubuxu added the kind/enhancement A net-new feature or improvement to an existing feature label Feb 2, 2019
@AuHau
Copy link
Member Author

AuHau commented Feb 2, 2019

Agree that it would be a simpler solution, but what about the existing keys? If you are talking about moving between OSes, then for old keys that could be still a problem. If there would be a migration that would make all existent keys lowercase, that could break things. Also, this approach will result in a change of behavior, that I think is more prone to user's confusion, then encoding itself.

Encoding does not change any behavior, only unites it across all platforms.

That said I don't have any hard preferences, maybe bit leaning to the encoding approach.

I have a clarification question though, we are talking about encoding the name of the keys right? Do you have knowledge of how many people interacts directly with the key's files and not your API?

@Stebalien
Copy link
Member

Stebalien commented Feb 13, 2019

I have a clarification question though, we are talking about encoding the name of the keys right?

Yes.

Do you have knowledge of how many people interacts directly with the key's files and not your API?

If anyone is messing with the keys in the repo directly, they'll have to change their code to match the new behavior.


@Kubuxu my worry is that different platforms have different constraints. We could just whitelist [a-z][0-9] but users should be able to use any character set they want.

Note: We should not treat these keys as an stable API. We should also support storing them in, e.g., the operating system's keyring. Given that, I really don't see an advantage to not encoding the names.

@Stebalien
Copy link
Member

Stebalien commented Feb 13, 2019

@AuHau If you'd like to work on this, please go ahead with the encoding. We should store keys as key_base32(name), or something like that. The key_ avoids collisions with special filenames on windows.

@AuHau
Copy link
Member Author

AuHau commented Feb 13, 2019

Alright! Sounds good!
I should have something ready latest in a week.

@Stebalien
Copy link
Member

Awesome!

AuHau added a commit to AuHau/go-ipfs that referenced this issue Feb 22, 2019
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
AuHau added a commit to AuHau/go-ipfs that referenced this issue Mar 14, 2019
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
AuHau added a commit to AuHau/go-ipfs that referenced this issue Mar 18, 2019
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
Stebalien pushed a commit that referenced this issue Mar 14, 2020
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
hsanjuan pushed a commit that referenced this issue Mar 16, 2020
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
Stebalien pushed a commit that referenced this issue Mar 24, 2020
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
Stebalien added a commit that referenced this issue Mar 24, 2020
Introducing EncodedFSKeystore with base32 encoding (#5947)
ralendor pushed a commit to ralendor/go-ipfs that referenced this issue Jun 6, 2020
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
ralendor pushed a commit to ralendor/go-ipfs that referenced this issue Jun 6, 2020
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
ralendor pushed a commit to ralendor/go-ipfs that referenced this issue Jun 8, 2020
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
ralendor pushed a commit to ralendor/go-ipfs that referenced this issue Jun 8, 2020
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
ralendor pushed a commit to ralendor/go-ipfs that referenced this issue Jun 8, 2020
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
@aschmahmann
Copy link
Contributor

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants