You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "Kriskras99 (via GitHub)" <gi...@apache.org> on 2026/09/06 15:20:56 UTC
[PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Kriskras99 opened a new pull request, #666:
URL: https://github.com/apache/avro-rs/pull/666
This is done by removing the logging completely and returing an error instead of a boolean. The code will also no longer panic when it fails to resolve the schema.
This is a breaking change, users can fix their code by adding `.is_ok()` to get the previous behaviour.
Reported-by: CodeQL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Posted by "Kriskras99 (via GitHub)" <gi...@apache.org>.
Kriskras99 merged PR #666:
URL: https://github.com/apache/avro-rs/pull/666
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #666:
URL: https://github.com/apache/avro-rs/pull/666#discussion_r4119046931
##########
avro/src/types.rs:
##########
@@ -562,8 +744,13 @@ impl Value {
// TODO: check precision against n
(&Value::Fixed(_n, _), &Schema::Decimal { .. }) => None,
(Value::String(s), Schema::Enum(EnumSchema { symbols, .. })) => {
+ let value_path = ValuePath::Value(self, value_path);
if !symbols.contains(s) {
- Some(format!("'{s}' is not a member of the possible symbols"))
+ // By doing s:? we get an escaped string
+ Some(format!(
+ "{value_path}({s:?}) is not a symbol in {}",
Review Comment:
Could `s` be sensitive ?!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Posted by "Kriskras99 (via GitHub)" <gi...@apache.org>.
Kriskras99 commented on PR #666:
URL: https://github.com/apache/avro-rs/pull/666#issuecomment-5575143240
I still need to do schema, but here is a sample of improved error messages (top is new, bottom is old):
```
The value is invalid for the given schema: Unsupported value-schema combination! Value: Record.b.Record.z.Null, schema: Int
The value is invalid for the given schema: Unsupported value-schema combination! Value: Null, schema: Int
```
```
The value is invalid for the given schema: Unsupported value-schema combination! Value: Record.b.String, schema: Record(RecordSchema { name: Name { name: "Inner", .. }, fields: [RecordField { name: "z", schema: Int, .. }], .. })
The value is invalid for the given schema: Unsupported value-schema combination! Value: String("testing"), schema: Record(RecordSchema { name: Name { name: "Inner", .. }, fields: [RecordField { name: "z", schema: Int, .. }], .. })
```
```
The value is invalid for the given schema: Unsupported value-schema combination! Value: Record.a.Boolean, schema: Long
The value is invalid for the given schema: Unsupported value-schema combination! Value: Boolean(false), schema: Long
```
```
The value is invalid for the given schema: Enum("spades") is not at position 1 in the schema
The value is invalid for the given schema: Symbol 'spades' is not at position '1'
```
```
The value is invalid for the given schema: Size of Fixed (5) is different than the schema's size (4)
The value is invalid for the given schema: The value's size (5) is different than the schema's size (4)
```
```
Invalid value: Int(42) for schema: Boolean. Reason: Unsupported value-schema combination! Value: Int(42), schema: Boolean
Invalid value: Int(42) for schema: Boolean. Reason: Unsupported value-schema combination! Value: Int, schema: Boolean
```
```
Value Int(1) does not match schema String: Reason: Unsupported value-schema combination! Value: Int, schema: String
Value Int(1) does not match schema String: Reason: Unsupported value-schema combination! Value: Int(1), schema: String
```
You'll notice that when the actual value (`1`, `True`, `"Some string"`) is not relevant to the error, it is not in included.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #666:
URL: https://github.com/apache/avro-rs/pull/666#issuecomment-5566093383
IIRC this has been requested by someone to be able to debug validation issues. They said that it is hard to debug such issues when using big/huge schemas. The actual request was to print the path to the failing field but this was rather complex to implement.
Maybe log it as a `trace` ?! Or log only when `debug_assertions` is enabled ?!
Or make it configurable ?!
Or use custom Debug/Display/our-own impl that skips (sensitive) values ?!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #666:
URL: https://github.com/apache/avro-rs/pull/666#issuecomment-5575687151
Would be possible to print the `name` for named schemas ? E.g. `Record[ns.myname].a.Boolean`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Posted by "Kriskras99 (via GitHub)" <gi...@apache.org>.
Kriskras99 commented on PR #666:
URL: https://github.com/apache/avro-rs/pull/666#issuecomment-5585739957
Schema path is now also added. I think this would also be useful in the errors for the serializer and deserializer. Let me know what you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Posted by "Kriskras99 (via GitHub)" <gi...@apache.org>.
Kriskras99 commented on PR #666:
URL: https://github.com/apache/avro-rs/pull/666#issuecomment-5566125224
I was already thinking about adding the path to the error. So I'll try that!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Posted by "Kriskras99 (via GitHub)" <gi...@apache.org>.
Kriskras99 commented on PR #666:
URL: https://github.com/apache/avro-rs/pull/666#issuecomment-5579586538
Absolutely, I was thinking of `Record{name}`. But that is only possible for the schema oath as `Value` doesn't store a name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix!: Don't log potentially sensitive value in `Value::validate` [avro-rs]
Posted by "Kriskras99 (via GitHub)" <gi...@apache.org>.
Kriskras99 commented on code in PR #666:
URL: https://github.com/apache/avro-rs/pull/666#discussion_r4121555079
##########
avro/src/types.rs:
##########
@@ -562,8 +744,13 @@ impl Value {
// TODO: check precision against n
(&Value::Fixed(_n, _), &Schema::Decimal { .. }) => None,
(Value::String(s), Schema::Enum(EnumSchema { symbols, .. })) => {
+ let value_path = ValuePath::Value(self, value_path);
if !symbols.contains(s) {
- Some(format!("'{s}' is not a member of the possible symbols"))
+ // By doing s:? we get an escaped string
+ Some(format!(
+ "{value_path}({s:?}) is not a symbol in {}",
Review Comment:
It can be, but we don't log it. It's up to the user of the library to handle data with the needed care.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org