Skip to content

Commit d719bd1

Browse files
authored
Fix property parsing when a property contains a struct (#210)
Properties are parsed lazily by deserializing the data into an `IgnoredAny` and then returning the bytes that have been read. We do parse structs as an enum in order to emit the struct tag as enum tag. The `IgnoredAny` then deserializes a newtype enum variant, expecting a single item. The `VariantAccess` impl for the packstream deserializer would then only emit the next field in the struct, introducing the bug. The fix is to emit all fields when the list of items is from a struct and we are parsing properties.
1 parent a56e2d2 commit d719bd1

File tree

3 files changed

+80
-13
lines changed

3 files changed

+80
-13
lines changed

lib/src/bolt/structs/node.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,10 @@ mod tests {
188188
use serde_test::{assert_de_tokens, Token};
189189
use test_case::{test_case, test_matrix};
190190

191-
use crate::packstream::{bolt, from_bytes_ref, BoltBytesBuilder, Data};
191+
use crate::{
192+
bolt::LegacyDateTime,
193+
packstream::{bolt, from_bytes_ref, BoltBytesBuilder, Data},
194+
};
192195

193196
use super::*;
194197

@@ -335,4 +338,33 @@ mod tests {
335338
.tiny_string("Alice")
336339
.build()
337340
}
341+
342+
fn deserialize_properties_with_structs() {
343+
let mut data = Data::new(
344+
bolt()
345+
.structure(3, 0x4E)
346+
.tiny_int(42)
347+
.tiny_list(1)
348+
.tiny_string("Label")
349+
.tiny_map(1)
350+
.tiny_string("p")
351+
.structure(3, b'F')
352+
.int32(42)
353+
.tiny_int(0)
354+
.tiny_int(0)
355+
.build(),
356+
);
357+
let mut node: NodeRef = from_bytes_ref(&mut data).unwrap();
358+
359+
assert_eq!(node.id(), 42);
360+
assert_eq!(node.labels(), &["Label"]);
361+
assert_eq!(node.element_id(), None);
362+
363+
assert_eq!(node.keys(), &["p"]);
364+
365+
let p = node.get::<LegacyDateTime>("p").unwrap().unwrap();
366+
assert_eq!(p.seconds_since_epoch(), 42);
367+
assert_eq!(p.nanoseconds(), 0);
368+
assert_eq!(p.timezone_offset_seconds(), 0);
369+
}
338370
}

lib/src/packstream/de.rs

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
use std::{fmt, marker::PhantomData};
1+
use std::{any::type_name, fmt, marker::PhantomData};
22

33
use bytes::{Buf, Bytes};
44
use serde::{
55
de::{
6-
self, value::SeqDeserializer, DeserializeSeed, EnumAccess, IntoDeserializer as _,
7-
MapAccess, SeqAccess, VariantAccess, Visitor,
6+
self, value::SeqDeserializer, DeserializeSeed, EnumAccess, IgnoredAny,
7+
IntoDeserializer as _, MapAccess, SeqAccess, VariantAccess, Visitor,
88
},
99
forward_to_deserialize_any,
1010
};
@@ -207,8 +207,10 @@ impl<'de> Deserializer<'de> {
207207
let start = full_bytes.as_ptr();
208208
let end = self.bytes.as_ptr();
209209

210+
// SAFETY: both pointers are from the same allocation and end is >= start
210211
let len = unsafe { end.offset_from(start) };
211212
full_bytes.truncate(len.unsigned_abs());
213+
212214
Ok(full_bytes)
213215
}
214216

@@ -358,20 +360,25 @@ impl<'a> ItemsParser<'a> {
358360
self.excess = excess;
359361
self
360362
}
363+
364+
fn drain(&mut self) -> Result<(), Error> {
365+
let bytes = self.bytes.get();
366+
for _ in 0..(self.len + self.excess) {
367+
Deserializer { bytes }.skip_next_item()?;
368+
}
369+
Ok(())
370+
}
361371
}
362372

363-
impl<'a, 'de> SeqAccess<'de> for ItemsParser<'a> {
373+
impl<'de> SeqAccess<'de> for ItemsParser<'_> {
364374
type Error = Error;
365375

366376
fn next_element_seed<T>(&mut self, seed: T) -> Result<Option<T::Value>, Self::Error>
367377
where
368378
T: DeserializeSeed<'de>,
369379
{
370380
if self.len == 0 {
371-
let bytes = self.bytes.get();
372-
for _ in 0..self.excess {
373-
Deserializer { bytes }.skip_next_item()?;
374-
}
381+
self.drain()?;
375382
return Ok(None);
376383
}
377384
self.len -= 1;
@@ -388,7 +395,7 @@ impl<'a, 'de> SeqAccess<'de> for ItemsParser<'a> {
388395
}
389396
}
390397

391-
impl<'a, 'de> MapAccess<'de> for ItemsParser<'a> {
398+
impl<'de> MapAccess<'de> for ItemsParser<'_> {
392399
type Error = Error;
393400

394401
fn next_key_seed<K>(&mut self, seed: K) -> Result<Option<K::Value>, Self::Error>
@@ -420,7 +427,7 @@ impl<'a, 'de> MapAccess<'de> for ItemsParser<'a> {
420427
}
421428
}
422429

423-
impl<'a, 'de> VariantAccess<'de> for ItemsParser<'a> {
430+
impl<'de> VariantAccess<'de> for ItemsParser<'_> {
424431
type Error = Error;
425432

426433
fn unit_variant(mut self) -> Result<(), Self::Error> {
@@ -431,6 +438,11 @@ impl<'a, 'de> VariantAccess<'de> for ItemsParser<'a> {
431438
where
432439
T: DeserializeSeed<'de>,
433440
{
441+
if type_name::<T>() == type_name::<PhantomData<IgnoredAny>>() {
442+
self.drain()?;
443+
return seed.deserialize(().into_deserializer());
444+
}
445+
434446
self.next_value_seed(seed)
435447
}
436448

@@ -498,14 +510,14 @@ struct SharedBytes<'a> {
498510
}
499511

500512
#[cfg(all(test, debug_assertions))]
501-
impl<'a> fmt::Debug for SharedBytes<'a> {
513+
impl fmt::Debug for SharedBytes<'_> {
502514
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
503515
crate::packstream::Dbg(unsafe { &*self.bytes }).fmt(f)
504516
}
505517
}
506518

507519
#[cfg(not(all(test, debug_assertions)))]
508-
impl<'a> fmt::Debug for SharedBytes<'a> {
520+
impl fmt::Debug for SharedBytes<'_> {
509521
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
510522
f.debug_struct("SharedBytes").finish_non_exhaustive()
511523
}

lib/tests/node_property_parsing.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
use chrono::{DateTime, FixedOffset};
2+
use neo4rs::{query, Node};
3+
4+
mod container;
5+
6+
#[tokio::test]
7+
async fn node_property_parsing() {
8+
let neo4j = container::Neo4jContainer::new().await;
9+
let graph = neo4j.graph();
10+
11+
graph
12+
.run(query("CREATE (:A {p1:DATETIME('2024-12-31T08:10:35')})"))
13+
.await
14+
.unwrap();
15+
16+
let mut result = graph.execute(query("MATCH (p:A) RETURN p")).await.unwrap();
17+
18+
while let Ok(Some(row)) = result.next().await {
19+
let node: Node = row.get("p").unwrap();
20+
let p1 = node.get::<DateTime<FixedOffset>>("p1").unwrap();
21+
assert_eq!(p1.timestamp(), 1735632635);
22+
}
23+
}

0 commit comments

Comments
 (0)