Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

### Fixed

- Deserialization no longer preallocates based on an unbounded size hint. Length-prefixed formats such as bincode and postcard derive their size hint from the input, so a small hostile payload claiming a huge number of elements could previously cause an excessively large allocation before any element was read. Preallocation is now capped at 1 MiB worth of items, matching what `serde` does for the standard library's collections.

- The `insert_overwrite` path on `IdHashMap` no longer aborts when an allocation fails, matching the existing guarantee on `BiHashMap` and `TriHashMap`. Instead, it results in a catchable panic. (The map is left unchanged, similar to `BiHashMap` and `TriHashMap`.)

Note that `BTreeMap::insert_overwrite` will abort on allocation failure, because it calls into `std` which doesn't have an equivalent to `HashMap::try_reserve`.
Expand Down
44 changes: 19 additions & 25 deletions crates/iddqd/src/bi_hash_map/serde_impls.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::{
BiHashItem, BiHashMap, DefaultHashBuilder,
support::alloc::{Allocator, Global},
support::{
alloc::{Allocator, Global},
size_hint::cautious,
},
};
use core::{fmt, hash::BuildHasher, marker::PhantomData};
use serde_core::{
Expand Down Expand Up @@ -193,14 +196,11 @@ where
where
Access: SeqAccess<'de>,
{
let mut map = match seq.size_hint() {
Some(size) => BiHashMap::with_capacity_and_hasher_in(
size,
self.hasher,
self.alloc,
),
None => BiHashMap::with_hasher_in(self.hasher, self.alloc),
};
let mut map = BiHashMap::with_capacity_and_hasher_in(
cautious::<T>(seq.size_hint()),
self.hasher,
self.alloc,
);

while let Some(element) = seq.next_element()? {
map.insert_unique(element)
Expand All @@ -217,14 +217,11 @@ where
where
Access: MapAccess<'de>,
{
let mut map = match map_access.size_hint() {
Some(size) => BiHashMap::with_capacity_and_hasher_in(
size,
self.hasher,
self.alloc,
),
None => BiHashMap::with_hasher_in(self.hasher, self.alloc),
};
let mut map = BiHashMap::with_capacity_and_hasher_in(
cautious::<T>(map_access.size_hint()),
self.hasher,
self.alloc,
);

while let Some((_, value)) =
map_access.next_entry::<serde_core::de::IgnoredAny, T>()?
Expand Down Expand Up @@ -312,14 +309,11 @@ where
where
Access: MapAccess<'de>,
{
let mut map = match map_access.size_hint() {
Some(size) => BiHashMap::with_capacity_and_hasher_in(
size,
self.hasher,
self.alloc,
),
None => BiHashMap::with_hasher_in(self.hasher, self.alloc),
};
let mut map = BiHashMap::with_capacity_and_hasher_in(
cautious::<T>(map_access.size_hint()),
self.hasher,
self.alloc,
);

while let Some((_, value)) =
map_access.next_entry::<serde_core::de::IgnoredAny, T>()?
Expand Down
44 changes: 19 additions & 25 deletions crates/iddqd/src/id_hash_map/serde_impls.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::{
DefaultHashBuilder, IdHashItem, IdHashMap,
support::alloc::{Allocator, Global},
support::{
alloc::{Allocator, Global},
size_hint::cautious,
},
};
use core::{fmt, hash::BuildHasher, marker::PhantomData};
use serde_core::{
Expand Down Expand Up @@ -188,14 +191,11 @@ where
where
Access: SeqAccess<'de>,
{
let mut map = match seq.size_hint() {
Some(size) => IdHashMap::with_capacity_and_hasher_in(
size,
self.hasher,
self.alloc,
),
None => IdHashMap::with_hasher_in(self.hasher, self.alloc),
};
let mut map = IdHashMap::with_capacity_and_hasher_in(
cautious::<T>(seq.size_hint()),
self.hasher,
self.alloc,
);

while let Some(element) = seq.next_element()? {
map.insert_unique(element)
Expand All @@ -212,14 +212,11 @@ where
where
Access: MapAccess<'de>,
{
let mut map = match map_access.size_hint() {
Some(size) => IdHashMap::with_capacity_and_hasher_in(
size,
self.hasher,
self.alloc,
),
None => IdHashMap::with_hasher_in(self.hasher, self.alloc),
};
let mut map = IdHashMap::with_capacity_and_hasher_in(
cautious::<T>(map_access.size_hint()),
self.hasher,
self.alloc,
);

while let Some((_, value)) =
map_access.next_entry::<serde_core::de::IgnoredAny, T>()?
Expand Down Expand Up @@ -301,14 +298,11 @@ where
where
Access: MapAccess<'de>,
{
let mut map = match map_access.size_hint() {
Some(size) => IdHashMap::with_capacity_and_hasher_in(
size,
self.hasher,
self.alloc,
),
None => IdHashMap::with_hasher_in(self.hasher, self.alloc),
};
let mut map = IdHashMap::with_capacity_and_hasher_in(
cautious::<T>(map_access.size_hint()),
self.hasher,
self.alloc,
);

while let Some((_, value)) =
map_access.next_entry::<serde_core::de::IgnoredAny, T>()?
Expand Down
12 changes: 6 additions & 6 deletions crates/iddqd/src/id_ord_map/serde_impls.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::{IdOrdItem, IdOrdMap};
use crate::support::size_hint::cautious;
use core::{fmt, marker::PhantomData};
use serde_core::{
Deserialize, Deserializer, Serialize, Serializer,
Expand Down Expand Up @@ -110,10 +111,7 @@ where
where
Access: SeqAccess<'de>,
{
let mut map = match seq.size_hint() {
Some(size) => IdOrdMap::with_capacity(size),
None => IdOrdMap::new(),
};
let mut map = IdOrdMap::with_capacity(cautious::<T>(seq.size_hint()));

while let Some(element) = seq.next_element()? {
map.insert_unique(element)
Expand All @@ -130,7 +128,8 @@ where
where
Access: MapAccess<'de>,
{
let mut map = IdOrdMap::new();
let mut map =
IdOrdMap::with_capacity(cautious::<T>(map_access.size_hint()));

while let Some((_, value)) =
map_access.next_entry::<serde_core::de::IgnoredAny, T>()?
Expand Down Expand Up @@ -203,7 +202,8 @@ where
where
Access: MapAccess<'de>,
{
let mut map = IdOrdMap::new();
let mut map =
IdOrdMap::with_capacity(cautious::<T>(map_access.size_hint()));

while let Some((_, value)) =
map_access.next_entry::<serde_core::de::IgnoredAny, T>()?
Expand Down
2 changes: 2 additions & 0 deletions crates/iddqd/src/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ pub(crate) mod item_set;
pub(crate) mod map_hash;
#[cfg(feature = "schemars08")]
pub(crate) mod schemars_utils;
#[cfg(feature = "serde")]
pub(crate) mod size_hint;

pub(crate) use item_index::ItemIndex;
14 changes: 14 additions & 0 deletions crates/iddqd/src/support/size_hint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use core::{cmp, mem};

// A mirror of
// https://github.com/serde-rs/serde/blob/7fc3b4c30c94f73a96ebd1553f2b090d928fc3a8/serde_core/src/private/size_hint.rs#L12,
// used to cap size_hint-based preallocation to a reasonable value.
pub(crate) fn cautious<T>(hint: Option<usize>) -> usize {
const MAX_PREALLOC_BYTES: usize = 1024 * 1024;

if mem::size_of::<T>() == 0 {
0
} else {
cmp::min(hint.unwrap_or(0), MAX_PREALLOC_BYTES / mem::size_of::<T>())
}
}
44 changes: 19 additions & 25 deletions crates/iddqd/src/tri_hash_map/serde_impls.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::{
DefaultHashBuilder, TriHashItem, TriHashMap,
support::alloc::{Allocator, Global},
support::{
alloc::{Allocator, Global},
size_hint::cautious,
},
};
use core::{fmt, hash::BuildHasher, marker::PhantomData};
use serde_core::{
Expand Down Expand Up @@ -197,14 +200,11 @@ where
where
Access: SeqAccess<'de>,
{
let mut map = match seq.size_hint() {
Some(size) => TriHashMap::with_capacity_and_hasher_in(
size,
self.hasher,
self.alloc,
),
None => TriHashMap::with_hasher_in(self.hasher, self.alloc),
};
let mut map = TriHashMap::with_capacity_and_hasher_in(
cautious::<T>(seq.size_hint()),
self.hasher,
self.alloc,
);

while let Some(element) = seq.next_element()? {
map.insert_unique(element)
Expand All @@ -221,14 +221,11 @@ where
where
Access: MapAccess<'de>,
{
let mut map = match map_access.size_hint() {
Some(size) => TriHashMap::with_capacity_and_hasher_in(
size,
self.hasher,
self.alloc,
),
None => TriHashMap::with_hasher_in(self.hasher, self.alloc),
};
let mut map = TriHashMap::with_capacity_and_hasher_in(
cautious::<T>(map_access.size_hint()),
self.hasher,
self.alloc,
);

while let Some((_, value)) =
map_access.next_entry::<serde_core::de::IgnoredAny, T>()?
Expand Down Expand Up @@ -321,14 +318,11 @@ where
where
Access: MapAccess<'de>,
{
let mut map = match map_access.size_hint() {
Some(size) => TriHashMap::with_capacity_and_hasher_in(
size,
self.hasher,
self.alloc,
),
None => TriHashMap::with_hasher_in(self.hasher, self.alloc),
};
let mut map = TriHashMap::with_capacity_and_hasher_in(
cautious::<T>(map_access.size_hint()),
self.hasher,
self.alloc,
);

while let Some((_, value)) =
map_access.next_entry::<serde_core::de::IgnoredAny, T>()?
Expand Down
2 changes: 2 additions & 0 deletions crates/iddqd/tests/integration/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ mod id_ord_map;
mod pathological;
#[cfg(feature = "schemars08")]
mod schemars_tests;
#[cfg(all(feature = "serde", feature = "std", feature = "default-hasher"))]
mod serde_size_hint;
#[cfg(all(
feature = "std",
feature = "default-hasher",
Expand Down
Loading
Loading