[App Configuration] Not log value of key #40132
Conversation
ivywei0125
requested review from
mrm9084,
chenrujun,
Netyyyy,
saragluna and
moarychan
as code owners
ivywei0125
changed the title
|
API change check API changes are not detected in this pull request. |
| JsonNode json = MAPPER.readTree(setting.getValue()); | ||
| parseSetting(setting.getKey(), json, settings); | ||
| } catch (JsonProcessingException e) { | ||
| throw new IOException( |
There was a problem hiding this comment.
InvalidConfigurationPropertyValueException
There was a problem hiding this comment.
I checked the constructor of InvalidConfigurationPropertyValueException. It requires name, value and reason.
What shall we provide for value? We didn't want to log the value
There was a problem hiding this comment.
If you read the description a null value for the value field is allowed.
There was a problem hiding this comment.
But is it a little wired that if we use a null value?
The error message would be Property test_key with value 'null' is invalid: Expected type: JSON String, Number, Array, Object or token 'null', 'true' or 'false'
Besides, I also tried to pass an empty string as value. It's also a little wired.
Property test_key with value '' is invalid: Expected type: JSON String, Number, Array, Object or token 'null', 'true' or 'false'
There was a problem hiding this comment.
Maybe we do what we do with our logs and put <Redacted>
There was a problem hiding this comment.
Updated. Thanks!
ivywei0125
requested review from
kushagraThapar,
FabianMeiswinkel,
trande4884,
TheovanKraay,
xinlian12,
aayush3011,
simorenoh,
jeet1995 and
Pilchie
as code owners
ivywei0125
force-pushed
the
yuwe/update_provider_log
branch
from
9273585 to
b513221
Compare
Description
When content type is json, we'll check if the value is a valid json. If invalid, throw
JsonProcessingException.Before fix, for a key-value:
test_key: test_value, the error message in the log is as the following, contains the value "test_value"."com.fasterxml.jackson.core.JsonParseException: Unrecognized token 'test_value_3': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')
at [Source: (String)"test_value_3"; line: 1, column: 13]"
We shouldn't include the value in logs to avoid any security issue.
After fix, the error message is:
"java.io.IOException: Invalid value of key '/foo/test_key_3'. Expected type: JSON String, Number, Array, Object or token 'null', 'true' or 'false'"