Skip to content

Commit ad3518c

Browse files
Add overflow check to duration parsing logic
1 parent 2992870 commit ad3518c

File tree

1 file changed

+82
-3
lines changed
  • datafusion/core/src/execution/context

1 file changed

+82
-3
lines changed

datafusion/core/src/execution/context/mod.rs

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,9 +1348,8 @@ impl SessionContext {
13481348
}
13491349
}
13501350

1351-
let duration = Duration::from_secs(
1352-
minutes.unwrap_or_default() * 60 + seconds.unwrap_or_default(),
1353-
);
1351+
let secs = Self::check_overflow(config_name, minutes, 60, seconds)?;
1352+
let duration = Duration::from_secs(secs);
13541353

13551354
if duration.is_zero() {
13561355
return plan_err!(
@@ -1361,6 +1360,29 @@ impl SessionContext {
13611360
Ok(duration)
13621361
}
13631362

1363+
fn check_overflow(
1364+
config_name: &str,
1365+
mins: Option<u64>,
1366+
multiplier: u64,
1367+
secs: Option<u64>,
1368+
) -> Result<u64> {
1369+
let first_part_of_secs = mins.unwrap_or_default().checked_mul(multiplier);
1370+
if first_part_of_secs.is_none() {
1371+
plan_err!(
1372+
"Duration is overflowed allowed maximum limit due to 'mins * {multiplier}' when setting '{config_name}'"
1373+
)?
1374+
}
1375+
let second_part_of_secs = first_part_of_secs
1376+
.unwrap()
1377+
.checked_add(secs.unwrap_or_default());
1378+
if second_part_of_secs.is_none() {
1379+
plan_err!(
1380+
"Duration is overflowed allowed maximum limit due to 'mins * {multiplier} + secs' when setting '{config_name}'"
1381+
)?
1382+
}
1383+
Ok(second_part_of_secs.unwrap())
1384+
}
1385+
13641386
async fn create_custom_table(
13651387
&self,
13661388
cmd: &CreateExternalTable,
@@ -2841,6 +2863,63 @@ mod tests {
28412863
}
28422864
}
28432865

2866+
#[test]
2867+
fn test_parse_duration_with_overflow_check() {
2868+
const LIST_FILES_CACHE_TTL: &str = "datafusion.runtime.list_files_cache_ttl";
2869+
2870+
// Valid durations which are close to max allowed limit
2871+
for (duration, want) in [
2872+
(
2873+
"18446744073709551615s",
2874+
Duration::from_secs(18446744073709551615),
2875+
),
2876+
(
2877+
"307445734561825860m",
2878+
Duration::from_secs(307445734561825860 * 60),
2879+
),
2880+
(
2881+
"307445734561825860m10s",
2882+
Duration::from_secs(307445734561825860 * 60 + 10),
2883+
),
2884+
(
2885+
"1m18446744073709551555s",
2886+
Duration::from_secs(60 + 18446744073709551555),
2887+
),
2888+
] {
2889+
let have =
2890+
SessionContext::parse_duration(LIST_FILES_CACHE_TTL, duration).unwrap();
2891+
assert_eq!(want, have);
2892+
}
2893+
2894+
// Invalid durations which overflow max allowed limit
2895+
for (duration, error_message_prefix) in [
2896+
(
2897+
"18446744073709551616s",
2898+
"Failed to parse number from duration",
2899+
),
2900+
(
2901+
"307445734561825861m",
2902+
"Duration is overflowed allowed maximum limit due to",
2903+
),
2904+
(
2905+
"307445734561825860m60s",
2906+
"Duration is overflowed allowed maximum limit due to",
2907+
),
2908+
(
2909+
"1m18446744073709551556s",
2910+
"Duration is overflowed allowed maximum limit due to",
2911+
),
2912+
] {
2913+
let have = SessionContext::parse_duration(LIST_FILES_CACHE_TTL, duration);
2914+
assert!(have.is_err());
2915+
let error_message = have.unwrap_err().message().to_string();
2916+
assert!(
2917+
error_message.contains(error_message_prefix)
2918+
&& error_message.contains(LIST_FILES_CACHE_TTL)
2919+
);
2920+
}
2921+
}
2922+
28442923
#[test]
28452924
fn test_parse_memory_limit() {
28462925
// Valid memory_limit

0 commit comments

Comments
 (0)