Skip to content

Commit

Permalink
fix segfault with empty numrange during from_datum() (#1918)
Browse files Browse the repository at this point in the history
An empty `numrange` such as `'[10.5, 10.5)'::numrange` would lead to a
segfault during a `Range<AnyNumeric>::from_datum()`.

This also hardcodes CI to `runs-on: ubuntu-22.04` due to the upgrade in
-latest (actions/runner-images#10636) that I
don't have the patience to work out today.
  • Loading branch information
eeeebbbbrrrr authored Oct 14, 2024
1 parent d0b3f87 commit 26ad5db
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 16 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/package-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ name: verify package can build

on:
push:
branches: [develop]
branches: [ develop ]
pull_request:
branches: [develop]
branches: [ develop ]

env:
CARGO_TERM_COLOR: always
Expand All @@ -14,7 +14,7 @@ env:

jobs:
ubuntu:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04

steps:
- uses: actions/checkout@v4
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/runas.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ name: cargo pgrx test --runas

on:
push:
branches: [develop]
branches: [ develop ]
pull_request:
branches: [develop]
branches: [ develop ]

env:
CARGO_TERM_COLOR: always
RUST_BACKTRACE: 1

jobs:
ubuntu:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04

steps:
- uses: actions/checkout@v4
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ env:
jobs:
lintck:
name: rustfmt, clippy, et al.
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
if: "!contains(github.event.head_commit.message, 'nogha')"
env:
RUSTC_WRAPPER: sccache
Expand Down Expand Up @@ -97,7 +97,7 @@ jobs:
pgrx_tests:
name: Linux tests & examples
needs: lintck
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
if: "!contains(github.event.head_commit.message, 'nogha')"
env:
RUSTC_WRAPPER: sccache
Expand Down Expand Up @@ -363,7 +363,7 @@ jobs:

cargo_pgrx_init:
name: cargo pgrx init
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
if: "!contains(github.event.head_commit.message, 'nogha')"
env:
RUSTC_WRAPPER: sccache
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/will-it-blend-develop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ env:
jobs:
distro_tests:
name: Distro tests
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
strategy:
fail-fast: false # We want all of them to run, even if one fails
matrix:
Expand All @@ -36,7 +36,7 @@ jobs:

cargo_unlocked_tests:
name: Cargo unlocked tests
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
strategy:
fail-fast: false # We want all of them to run, even if one fails
matrix:
Expand Down
8 changes: 8 additions & 0 deletions pgrx-tests/src/tests/range_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ mod tests {
assert_eq!(matched, Ok(Some(true)));
}

#[pg_test]
fn test_empty_anynumeric_range() {
let matched = Spi::get_one::<Range<AnyNumeric>>(
"SELECT accept_range_numeric('[10.5, 10.5)'::numrange)",
);
assert_eq!(matched, Ok(Some(Range::empty())));
}

#[pg_test]
fn test_accept_range_date() {
let matched =
Expand Down
18 changes: 13 additions & 5 deletions pgrx/src/datum/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,18 +348,26 @@ where
&mut is_empty,
);

// SAFETY: The lower_bound/upper_bound RangeBound value's .val will be a valid Datum of the T type
// If the range is_empty or either bound is infinite then .val = (Datum) 0
let lower = RangeBound::from_pg(lower_bound);
let upper = RangeBound::from_pg(upper_bound);
let range = if is_empty {
// empty ranges cannot go through `RangeBound::from_pg()` as the bound's `.val`
// Datum will be zero, and for a pass-by-reference range type, that's an instant segfault
Range::empty()
} else {
// SAFETY: The lower_bound/upper_bound RangeBound value's .val will be a valid Datum of the T type
// If the range is_empty or either bound is infinite then .val = (Datum) 0, which we handled above
let lower = RangeBound::from_pg(lower_bound);
let upper = RangeBound::from_pg(upper_bound);

Range { inner: Some((lower, upper)) }
};

if !std::ptr::eq(ptr, range_type.cast()) {
// SAFETY: range_type was allocated by Postgres in the call to
// pg_detoast_datum above, so we know it's a valid pointer and needs to be freed
pg_sys::pfree(range_type.cast());
}

Some(Range { inner: if is_empty { None } else { Some((lower, upper)) } })
Some(range)
}
}
}
Expand Down

0 comments on commit 26ad5db

Please sign in to comment.