Skip to content

Commit 64881fe

Browse files
committed
Fix segfault on invalid SMARTS and add RDKIT_PREFIX env var support
- Add null pointer check in RWMol::from_smarts() to prevent segfault on invalid SMARTS input (addresses #18, supersedes #19). Introduces RWMolError type mirroring the existing ROMolError pattern. - Support RDKIT_PREFIX env var to allow specifying a custom RDKit install prefix, overriding all other detection logic (supersedes #45). Includes cargo:rerun-if-env-changed and empty string handling. https://claude.ai/code/session_01QNQtwSN1RrM5XCnXZoA6pb
1 parent 79a26ea commit 64881fe

3 files changed

Lines changed: 51 additions & 13 deletions

File tree

rdkit-sys/build.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,27 @@ fn main() {
88
env_logger::init();
99

1010
let use_conda = std::env::var("CARGO_FEATURE_DYNAMIC_LINKING_FROM_CONDA").is_ok();
11+
let explicit_prefix = std::env::var("RDKIT_PREFIX")
12+
.ok()
13+
.filter(|s| !s.is_empty());
14+
15+
println!("cargo:rerun-if-env-changed=RDKIT_PREFIX");
1116

1217
let mut lib_paths = vec![];
1318
let mut include_paths = vec![];
1419

15-
match (std::env::consts::OS, std::env::consts::ARCH, use_conda) {
16-
(_, _, true) => {
20+
match (
21+
std::env::consts::OS,
22+
std::env::consts::ARCH,
23+
&explicit_prefix,
24+
use_conda,
25+
) {
26+
(_, _, Some(prefix), _) => {
27+
include_paths.push(format!("{prefix}/include"));
28+
include_paths.push(format!("{prefix}/include/rdkit"));
29+
lib_paths.push(format!("{prefix}/lib"));
30+
}
31+
(_, _, _, true) => {
1732
// prefer the prefix env var, if not, fall back to the base from the CLI
1833
match std::env::var("CONDA_PREFIX") {
1934
Ok(prefix) => {
@@ -38,24 +53,24 @@ fn main() {
3853
}
3954
}
4055
}
41-
("macos", "x86_64", _) => {
56+
("macos", "x86_64", _, _) => {
4257
include_paths.push("/usr/local/include".to_string());
4358
include_paths.push("/usr/local/include/rdkit".to_string());
4459
}
45-
("macos", "aarch64", _) => {
60+
("macos", "aarch64", _, _) => {
4661
include_paths.push("/opt/homebrew/include".to_string());
4762
include_paths.push("/opt/homebrew/include/rdkit".to_string());
4863
lib_paths.push("/opt/homebrew/lib".to_string())
4964
}
50-
("linux", _, _) => {
65+
("linux", _, _, _) => {
5166
include_paths.push("/usr/local/include".to_string());
5267
include_paths.push("/usr/local/include/rdkit".to_string());
5368
include_paths.push("/usr/include".to_string());
5469
include_paths.push("/usr/include/rdkit".to_string());
5570
}
56-
(unsupported_os, unsupported_arch, use_conda) => panic!(
57-
"sorry, rdkit-sys doesn't support {}/{}/use_conda={} at this time",
58-
unsupported_os, unsupported_arch, use_conda
71+
(unsupported_os, unsupported_arch, explicit_prefix, use_conda) => panic!(
72+
"sorry, rdkit-sys doesn't support {}/{}/prefix={:?}/use_conda={} at this time",
73+
unsupported_os, unsupported_arch, explicit_prefix, use_conda
5974
),
6075
};
6176

src/graphmol/rw_mol.rs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@ use rdkit_sys::*;
55

66
use crate::ROMol;
77

8+
#[derive(Debug, PartialEq, thiserror::Error)]
9+
pub enum RWMolError {
10+
#[error("could not convert smarts to rwmol (nullptr)")]
11+
UnknownConversionError,
12+
#[error("could not convert smarts to rwmol (exception)")]
13+
ConversionException(String),
14+
}
15+
816
pub struct RWMol {
917
pub(crate) ptr: SharedPtr<rdkit_sys::rw_mol_ffi::RWMol>,
1018
}
@@ -48,11 +56,19 @@ impl RWMol {
4856
ROMol { ptr }
4957
}
5058

51-
pub fn from_smarts(smarts: &str) -> Result<Self, Box<dyn std::error::Error>> {
59+
pub fn from_smarts(smarts: &str) -> Result<Self, RWMolError> {
5260
let_cxx_string!(smarts = smarts);
5361

54-
let ptr = rdkit_sys::rw_mol_ffi::smarts_to_mol(&smarts)?;
55-
Ok(RWMol { ptr })
62+
match rw_mol_ffi::smarts_to_mol(&smarts) {
63+
Ok(ptr) => {
64+
if ptr.is_null() {
65+
Err(RWMolError::UnknownConversionError)
66+
} else {
67+
Ok(RWMol { ptr })
68+
}
69+
}
70+
Err(e) => Err(RWMolError::ConversionException(e.to_string())),
71+
}
5672
}
5773
}
5874

tests/test_graphmol.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use rdkit::{
22
detect_chemistry_problems, fragment_parent, substruct_match, CleanupParameters,
3-
MolSanitizeException, ROMol, ROMolError, RWMol, SmilesParserParams, SubstructMatchParameters,
4-
TautomerEnumerator, Uncharger,
3+
MolSanitizeException, ROMol, ROMolError, RWMol, RWMolError, SmilesParserParams,
4+
SubstructMatchParameters, TautomerEnumerator, Uncharger,
55
};
66

77
#[test]
@@ -308,6 +308,13 @@ fn test_building_rwmol_from_smarts() {
308308
assert_eq!(result.len(), 1);
309309
}
310310

311+
#[test]
312+
fn test_building_rwmol_from_invalid_smarts() {
313+
let smarts = "invalid_smarts";
314+
let rwmol = RWMol::from_smarts(smarts);
315+
assert_eq!(rwmol.err(), Some(RWMolError::UnknownConversionError));
316+
}
317+
311318
#[test]
312319
fn test_build_romol_from_really_bad_smiles() {
313320
let smiles = "smiles";

0 commit comments

Comments
 (0)