minio / mc

Unix like utilities for object store
https://min.io/download
GNU Affero General Public License v3.0
2.86k stars 548 forks source link

Length check for alias #4812

Closed shtripat closed 9 months ago

shtripat commented 10 months ago

Community Contribution License

All community contributions in this pull request are licensed to the project maintainers under the terms of the [Apache 2 license] (https://www.apache.org/licenses/LICENSE-2.0). By creating this pull request I represent that I have the right to license the contributions to the project maintainers under the Apache 2 license.

Description

Alias name should be at least 2 characters long.

Motivation and Context

Fixes: https://github.com/minio/mc/issues/4512

How to test this PR?

mc alias set a http://localhost:9000 minioadmin minioadmin

Types of changes

Checklist:

klauspost commented 10 months ago

Remind me why an alias can't be 1 character?

shtripat commented 10 months ago

Remind me why an alias can't be 1 character?

cmd/config.go#isValidAlias() checks against regex ^[a-zA-Z][a-zA-Z0-9-_]+$

klauspost commented 10 months ago

@shtripat I am not doubting it is there. I am asking why is it there.

Seems to me naming your clusters a and b would be nice.

shtripat commented 10 months ago

@shtripat I am not doubting it is there. I am asking why is it there.

Oh ok.

Seems to me naming your clusters a and b would be nice.

If we agree, we can change the regex. I think this could have been done just to make sure we dont end up using nos like 1 or 2 as alias names. @harshavardhana thoughts?

vadmeste commented 10 months ago

It looks like we need to ask the guy who added it:

0bd1edfd8 config.go     (Anand Babu (AB) Periasamy 2015-12-19 22:11:06 -0800 150) // isValidAlias - Check if alias valid.
0bd1edfd8 config.go     (Anand Babu (AB) Periasamy 2015-12-19 22:11:06 -0800 151) func isValidAlias(alias string) bool {
0bd1edfd8 config.go     (Anand Babu (AB) Periasamy 2015-12-19 22:11:06 -0800 152)       return regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9-]+$").MatchString(alias)
0bd1edfd8 config.go     (Anand Babu (AB) Periasamy 2015-12-19 22:11:06 -0800 153) }
klauspost commented 10 months ago

Just seems like a regex having an unintended side effect.

minio-trusted commented 10 months ago

Just seems like a regex having an unintended side effect.

We can fix the regex and allow it.

feorlen commented 9 months ago

@shtripat to clarify, you can have a single char alias name, but only if that char is [A-Za-z] ?

shtripat commented 9 months ago

@shtripat to clarify, you can have a single char alias name, but only if that char is [A-Za-z] ?

Yes, single char can be a-z Or A-Z and multi char char could be like hello123, myminio2 etc.