Fix empty keys in BlobDataProvider
#3551
Conversation
| for (locale, idx) in cursor.iter1_copied() { | ||
| debug_assert!(idx < self.buffers.len() || locale == Index32U8::SENTINEL); |
There was a problem hiding this comment.
Issue: This is problematic because a newer blob won't pass the invariants of an older blob reader
There was a problem hiding this comment.
Personally ok with breaking debug assertions for new-blob-old-reader.
There was a problem hiding this comment.
We could also instead set the index to 0; it's fine because it'll never be loaded anyway (and if someone does attempt to load it raw, they'll get a deserialize error or potentially GIGO behavior.
There was a problem hiding this comment.
The problem is that for a blob with just an empty key, 0 is also invalid as there will be zero buffers.
There was a problem hiding this comment.
I'm willing to have debug assertions break in that relatively niche case
There was a problem hiding this comment.
(but also I'm not against breaking debug assertions with the =len sentinel in the first place. would prefer to avoid it, really do not care about the empty-blob case)
| for (locale, idx) in cursor.iter1_copied() { | ||
| debug_assert!(idx < self.buffers.len() || locale == Index32U8::SENTINEL); |
There was a problem hiding this comment.
Personally ok with breaking debug assertions for new-blob-old-reader.
| for (locale, idx) in cursor.iter1_copied() { | ||
| debug_assert!(idx < self.buffers.len() || locale == Index32U8::SENTINEL); |
There was a problem hiding this comment.
We could also instead set the index to 0; it's fine because it'll never be loaded anyway (and if someone does attempt to load it raw, they'll get a deserialize error or potentially GIGO behavior.
Fixes #3545