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

All environment variables with valid URLs get recognized as aliases #5045

Closed
jwadolowski opened this issue Sep 26, 2024 · 3 comments
Closed

Comments

@jwadolowski
Copy link

Expected behavior

Only MC_HOST_ prefixed env variables should be considered valid aliases.

Actual behavior

There's quite a few env variables with URL value configured on my machine. They're meant to be consumed by other apps, not by mc.

Here's an example - let's say I have the following variables configured:

export FOO="http://localhost:8080"
export MC_HOST_FOO="https://localhost:9090"
export BAR_URL="http://localhost:9999"
export PREFIX_BAZ_SUFFIX="https://example.com/a/path/to/a.json"

As long as variable value matches to this regex mc considers given variable as an alias.

$ docker run -e FOO="http://localhost:8080" -e MC_HOST_FOO="https://localhost:9090" -e BAR_URL="http://localhost:9999" -e PREFIX_BAZ_SUFFIX="https://example.com/a/path/to/a.json" minio/mc:RELEASE.2024-09-16T17-43-14Z alias ls
BAR_URL
  URL       : http://localhost:9999
  AccessKey :
  SecretKey :
  API       :
  Path      :
  Src       : env
FOO
  URL       : http://localhost:8080
  AccessKey :
  SecretKey :
  API       :
  Path      :
  Src       : env
FOO
  URL       : https://localhost:9090
  AccessKey :
  SecretKey :
  API       :
  Path      :
  Src       : env
gcs
  URL       : https://storage.googleapis.com
  AccessKey : YOUR-ACCESS-KEY-HERE
  SecretKey : YOUR-SECRET-KEY-HERE
  API       : S3v2
  Path      : dns
  Src       : /root/.mc/config.json
local
  URL       : http://localhost:9000
  AccessKey :
  SecretKey :
  API       :
  Path      : auto
  Src       : /root/.mc/config.json
play
  URL       : https://play.min.io
  AccessKey : Q3AM3UQ867SPQQA43P2F
  SecretKey : zuf+tfteSlswRu7BJ86wekitnifILbZam1KYY3TG
  API       : S3v4
  Path      : auto
  Src       : /root/.mc/config.json
s3
  URL       : https://s3.amazonaws.com
  AccessKey : YOUR-ACCESS-KEY-HERE
  SecretKey : YOUR-SECRET-KEY-HERE
  API       : S3v4
  Path      : dns
  Src       : /root/.mc/config.json

FOO, MC_HOST_FOO and BAR_URL match to the aforementioned regex (PREFIX_BAZ_SUFFIX does not) and all 3 of them showed up in the output. What's interesting is that FOO got listed twice as both FOO and MC_HOST_FOO got misinterpreted by mc.

The problem seems to be here - strings.TrimPrefix returns original string if defined prefix does not match, therefore len will always be != 0 which leads to incorrect behaviour.

strings.HasPrefix should do the trick in this case.

Steps to reproduce the behavior

Just run the following command:
docker run -e FOO="http://localhost:8080" -e MC_HOST_FOO="https://localhost:9090" -e BAR_URL="http://localhost:9999" -e PREFIX_BAZ_SUFFIX="https://example.com/a/path/to/a.json" minio/mc:RELEASE.2024-09-16T17-43-14Z alias ls

mc --version

# Inside Docker
$ docker run minio/mc:RELEASE.2024-09-16T17-43-14Z --version
mc version RELEASE.2024-09-16T17-43-14Z (commit-id=11ebe952ea30e426e564f66e78d178465ae7c432)
Runtime: go1.22.6 linux/arm64
Copyright (c) 2015-2024 MinIO, Inc.
License GNU AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>

# Directly on my macOS
$ mc --version
mc version RELEASE.2024-09-16T17-43-14Z (commit-id=11ebe952ea30e426e564f66e78d178465ae7c432)
Runtime: go1.23.1 darwin/arm64
Copyright (c) 2015-0000 MinIO, Inc.
License GNU AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>

System information

Docker image & macOS Sonoma 14.6.1

@harshavardhana
Copy link
Member

harshavardhana commented Sep 26, 2024

Perhaps a bug introduced in this PR #5028

@dormanze ^^

@dormanze
Copy link
Contributor

Perhaps a bug introduced in this PR #5028

@dormanze ^^

Let me fix it. Please assign the issue to me.

@harshavardhana
Copy link
Member

fixed by #5048

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants