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

Case sensitive keys #1014

Open
sagikazarmark opened this issue Nov 3, 2020 · 28 comments
Open

Case sensitive keys #1014

sagikazarmark opened this issue Nov 3, 2020 · 28 comments
Labels
kind/enhancement New feature or request
Projects

Comments

@sagikazarmark
Copy link
Collaborator

This issue will track (probably) the most requested feature for Viper: case sensitivity.

Every other issue will be closed in favor of this one.

We know this is something that the community wants (although based on feedbacks, it might not be a majority), so it's being considered for Viper v2. It doesn't mean it will be implemented though.

Please see for more details https://github.com/spf13/viper#does-viper-support-case-sensitive-keys

@sagikazarmark sagikazarmark added the kind/enhancement New feature or request label Nov 3, 2020
@sagikazarmark sagikazarmark added this to To do in Viper 2 via automation Nov 3, 2020
@sagikazarmark sagikazarmark pinned this issue Nov 3, 2020
Repository owner deleted a comment from github-actions bot Nov 3, 2020
@aaronbuchwald
Copy link

What is the timeline for a decision on incorporating case sensitivity into v2 as well as the release of v2?

@li1234yun
Copy link

Maybe key case sensitive can be viper config param, maybe like WithCaseSensitive etc. When WithCaseSensitive == true, all viper operation is case sensitive, or not.

@li1234yun
Copy link

I want to mention a little bit about the benefits of case sensitivity. Because the structure public fields of golang start with uppercase, when the read configuration is consistent with the structure fields, we can directly use the configuration structure for deserialization, which saves a lot of time and avoids some unnecessary intermediate conversion.

@sagikazarmark
Copy link
Collaborator Author

Unmarshal is case insensitive, so it should work either way.

@li1234yun
Copy link

Unmarshal is case insensitive, so it should work either way.

Thanks!

@WeiChieh-Chen
Copy link

Maybe I'm wrong, but because of the automatic function here, I have to take on additional processing work. And usually, the configuration files are set by themselves, which means that these format designs are useful to users. Is this really right ?

@boatrainlsz
Copy link

it's been 2 years

@jeven2016
Copy link

God.... no fix .... it's 2022...

routes:
  # request path: /api/iam/**
  - serviceName: iam
    url: http://www.baidu.com/ # the url of backend service
    filters:
      - SetHeader: Host=www.test.com, X-Forwared-Host=www.test.com

the map key "SetHeader" will be "setheader" after called the method "viper.Unmarshal()"

@link-duan
Copy link

fix this please

@dreamlu
Copy link

dreamlu commented Dec 13, 2022

2cb5cbf
why delete it?

@jim3ma
Copy link

jim3ma commented Jan 17, 2023

There are many issues about case sensitive, why it is not end till now ?

@meln5674
Copy link

If this is not going to be fixed, then viper.BindPFlag() on a pflag.StringToString[Var] or calling viper.Unmarshal() on a struct with map fields should both panic, because those are both guaranteed not to do what the user intended.

@sngyai
Copy link

sngyai commented Feb 1, 2023

Any progress?

@chayuto
Copy link

chayuto commented Feb 23, 2023

i want this fix too!

when unmarshal map[string]string

this_is_cap_key:TEST VAL

@kayvonkhosrowpour
Copy link

This is super frustrating but I did find an undesirable workaround. If you create a custom "alias" type, viper seems to load the string case sensitive:

type ExampleKey string

And then in your config

type Config struct {
    MyMap map[ExampleKey]string `mapstructure:"myMap"`
}

And an example config

myMap:
  keyA: "1234"
  keya: "4567"

But then comes the challenge of how to reference the map keys by an ExampleKey, since the hash of the custom type is not the string itself...

@win5do
Copy link

win5do commented May 18, 2023

Implicit case conversions in config files cause a lot of trouble, we dropped the viper library for this reason.

@jazanne
Copy link

jazanne commented May 18, 2023

@win5do curious what you switched to? this issue is causing me headaches as well...

@win5do
Copy link

win5do commented May 18, 2023

@win5do curious what you switched to? this issue is causing me headaches as well...

@jazanne Just use go-yaml/yaml directly. It is not too difficult to support functions such as config file reading and watching.

@koh-osug
Copy link

Would be nice to have this implemented. I had to migrate to koanf which fits better my needs with this issue fixed.

@PaleNeutron
Copy link

3 years passed... unbelievable.

I have to make a lot of effort migrating to koanf, just for case.

jazanne added a commit to launchdarkly/ld-find-code-refs that referenced this issue Oct 18, 2023
Track issue in viper: spf13/viper#1014

Co-authored-by: Ember Stevens <79482775+ember-stevens@users.noreply.github.com>
@travisnewhouse
Copy link

@sagikazarmark I read the comments on #635. I understand the concern for changing the fundamental behavior of Viper to have case-sensitive keys. I am wondering if there is a possibility for incorporating this functionality as an opt-in Option only available to explicitly created Vipers (i.e., created with viper.NewWithOptions()). Thereby, users are make a conscious choice to accept a new behavior. The behavior of the "global Viper" would not change and remain case-insensitive always. Would you be willing to entertain PR that implements this idea?

travisnewhouse added a commit to travisnewhouse/viper that referenced this issue Oct 20, 2023
When reading configuration from sources with case-sensitive keys,
such as YAML, TOML, and JSON, a user may wish to preserve the case
of keys that appear in maps.  For example, consider when the value
of a setting is a map with string keys that are case-sensitive.
Ideally, if the value is not going to be indexed by a Viper lookup
key, then the map value should be treated as an opaque value by
Viper, and its keys should not be modified.  See spf13#1014

Viper's default behaviour is that keys are case-sensitive, and this
behavior is implemented by converting all keys to lower-case.  For
users that wish to preserve the case of keys, this commit introduces
an Option `CaseSensitiveKeys()` that can be used to configure Viper
to use case-sensitive keys.  When CaseSensitiveKeys is enabled, all
keys retain the original case, and lookups become case-sensitive
(except for lookups of values bound to environment variables).

The behavior of Viper could become hard to understand if a user
could change the CaseSensitiveKeys setting after values have been
stored.  For this reason, the setting may only be set when creating
a Viper instance, and it cannot be set on the "global" Viper.
@maknapik
Copy link

My workaround for 1.12.0 (I checked it on the latest 1.17.0 and it did not work there) is to embed a map in a list:

Config     []map[string]string

Then, it reads keys case sensitive, e.g.:

config:
  - oneTwo: threeFour
  - fiveSix: sevenEight

@royalzzz
Copy link

royalzzz commented Jan 9, 2024

This year is 2024 😎

@sagikazarmark
Copy link
Collaborator Author

@royalzzz indeed it is, good observation. :)

@travisnewhouse I may consider accepting such a PR, but the concern (in addition to the ones voiced in #635) is the added complexity on top of the already overly complex implementation and the potential increase in support requests on the issue tracker (ie. why does AutomaticEnv not work (with case sensitive keys).

@travisnewhouse
Copy link

travisnewhouse commented Jan 9, 2024

@sagikazarmark I understand your concerns. That's why I proposed a design in #1673 that requires explicit "opt-in" by the user without affecting the default "global Viper" that may be used by many. Please review PR #1673 and let me know your thoughts. Thank you!

@c9s
Copy link

c9s commented Feb 13, 2024

oh, please add this feature, this is so critical

@EngHabu
Copy link

EngHabu commented Feb 16, 2024

I'm sure this has been talked about enough (possibly including this proposal) but we, at flyte.org, have been using viper since 2018/2019 and from the get go we realized how messy it'll be to not have types and structure so we built these two libraries:

  1. https://pkg.go.dev/github.com/flyteorg/flyte/flytestdlib/config to allow us to define configs in code structures that automatically get tracked, parsed, that can be sections and nested and that automatically subscribe to be updated when configs are updated.. comes with cobra commands to find and validate config files as well and automatically generates expected ENV VAR names and asks viper to merge it all together
  2. https://pkg.go.dev/github.com/flyteorg/flyte/flytestdlib@v1.10.7/cli/pflags to generate command line pflags from these properties.

With these two we have weathered a lot of miss configurations and allowed us to catch mistakes very early on..

Besides pitching these libraries to viper to adopt/align directions, having these strongly typed classes allowed us to correctly parse out configs using viper and maintain case sensitivity (we create a hook to check if we are trying to unmarshal into a map and instead write configs as slices and do the conversion there)

@upkit
Copy link

upkit commented Mar 11, 2024

I hope this message finds you well. I am writing to provide feedback regarding the default case sensitivity behavior in Viper. I have encountered difficulty in understanding why Viper defaults to not distinguishing between uppercase and lowercase keys. In my opinion, it would be more intuitive for Viper to prioritize case sensitivity as the default behavior, while case insensitivity could be offered as a configurable option.

When configuring Viper, I often find myself intentionally using uppercase keys for a specific purpose. In such cases, I believe there is a deliberate reason behind my decision to use uppercase letters. If the default behavior of Viper were to differentiate between cases, it would align with the expectation that uppercase keys hold significance and should not be treated the same as their lowercase counterparts.

Moreover, considering the physical aspect, typing uppercase letters requires an additional effort of pressing the shift key. If the default behavior of Viper were to ignore case sensitivity, it would essentially render the extra effort unnecessary in many cases. This could lead to potential confusion and frustration when trying to differentiate between keys based on their case.

I kindly request that you consider revisiting the default behavior of Viper and potentially making case sensitivity the default option, with the ability to configure case insensitivity as needed. This adjustment would enhance the usability and clarity of the Viper configuration framework, aligning it more closely with user expectations.

Thank you for your attention to this matter. I appreciate your dedication to continuously improving Viper and providing a seamless experience for its users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
Viper 2
  
To do
Development

No branches or pull requests