-
-
Notifications
You must be signed in to change notification settings - Fork 127
fix: centralize timestamp handling via wacore::time and fix signed parsing #532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b2013c1
ac2353d
e29e9f5
57d53ce
7933363
eb1f038
8115993
02c4dbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ use wacore_binary::Node; | |
| pub fn notification_timestamp(node: &Node) -> chrono::DateTime<chrono::Utc> { | ||
| node.attrs() | ||
| .optional_u64("t") | ||
| .and_then(|t| chrono::DateTime::from_timestamp(t as i64, 0)) | ||
| .and_then(|t| crate::time::from_secs(t as i64)) | ||
| .unwrap_or_else(crate::time::now_utc) | ||
|
Comment on lines
21
to
24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: When converting a u64 to i64 using the as cast where the u64 value exceeds i64::MAX (9223372036854775807), it performs a reinterpretation of the bit pattern. Since both types are 64 bits and Rust uses two's complement for signed integers, casting u64 to i64 is a no-op on the bits—it reinterprets the unsigned bits as a signed value. For values <= i64::MAX, the numerical value is preserved. For values > i64::MAX, the result is a negative i64. Specifically, u64::MAX (all bits 1) becomes -1i64, as confirmed by the official documentation: u64::cast_signed example shows u64::MAX.cast_signed == -1i64, and this produces the same result as as. This is neither saturation (clamping to i64::MAX), nor does it error or panic. It "wraps" in the sense of bit reinterpretation, producing a negative number equivalent to the two's complement interpretation. Official Rust Reference confirms: "Casting between two integers of the same size (e.g. i32 -> u32) is a no-op (Rust uses 2's complement for negative numbers)." Rust By Example notes: "When casting to a signed type, the (bitwise) result is the same as first casting to the corresponding unsigned type." It does not panic on overflow like arithmetic operations in debug mode; casts are always defined and do not trigger overflow checks. Citations:
🏁 Script executed: cat -n wacore/src/stanza/notification.rs | head -50Repository: jlucaso1/whatsapp-rust Length of output: 2503 🏁 Script executed: rg "parse_disappearing_mode" wacore/src/stanza/notification.rs -A 10Repository: jlucaso1/whatsapp-rust Length of output: 2290 🏁 Script executed: rg "fn from_secs" wacore/src/time.rs -A 5Repository: jlucaso1/whatsapp-rust Length of output: 521 🏁 Script executed: rg "notification_timestamp" wacore/src -B 2 -A 5Repository: jlucaso1/whatsapp-rust Length of output: 1924 🏁 Script executed: rg "from_timestamp" wacore/src -B 2 -A 2Repository: jlucaso1/whatsapp-rust Length of output: 813 🏁 Script executed: # Check if there are any tests for notification_timestamp that show the wrapping issue
rg "test.*notification_timestamp" wacore/src/stanza/notification.rs -A 15Repository: jlucaso1/whatsapp-rust Length of output: 48 Use signed parsing or checked conversion for the timestamp. The current code uses Suggested change pub fn notification_timestamp(node: &Node) -> chrono::DateTime<chrono::Utc> {
node.attrs()
- .optional_u64("t")
- .and_then(|t| crate::time::from_secs(t as i64))
+ .optional_string("t")
+ .and_then(|t| t.parse::<i64>().ok())
+ .and_then(crate::time::from_secs)
.unwrap_or_else(crate::time::now_utc)
}Parsing as 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: jlucaso1/whatsapp-rust
Length of output: 689
🏁 Script executed:
Repository: jlucaso1/whatsapp-rust
Length of output: 92
🏁 Script executed:
Repository: jlucaso1/whatsapp-rust
Length of output: 257
🏁 Script executed:
Repository: jlucaso1/whatsapp-rust
Length of output: 92
🏁 Script executed:
Repository: jlucaso1/whatsapp-rust
Length of output: 240
🏁 Script executed:
Repository: jlucaso1/whatsapp-rust
Length of output: 681
🏁 Script executed:
Repository: jlucaso1/whatsapp-rust
Length of output: 7301
🏁 Script executed:
Repository: jlucaso1/whatsapp-rust
Length of output: 3823
Avoid
u64 as i64forafterto reject oversized timestamps.Rust wraps on overflow, so a malformed
afterabovei64::MAXbecomes a negative timestamp (before 1970) instead ofNone, likely unintended for a sync request attribute.💡 Suggested fix
let after = child .attrs() .optional_u64("after") - .and_then(|after| wacore::time::from_secs(after as i64)); + .and_then(|after| i64::try_from(after).ok()) + .and_then(wacore::time::from_secs);📝 Committable suggestion
🤖 Prompt for AI Agents