From 6943d0c1f07037dd63f34f30d3985b0c753e2445 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 1 Nov 2023 07:37:11 -0700 Subject: [PATCH 1/6] Wipe the env when compiling the user function --- plrust-tests/src/trusted.rs | 11 +++++------ plrustc/plrustc/src/main.rs | 11 +++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/plrust-tests/src/trusted.rs b/plrust-tests/src/trusted.rs index 33b373a6..b5ffa5a8 100644 --- a/plrust-tests/src/trusted.rs +++ b/plrust-tests/src/trusted.rs @@ -190,7 +190,7 @@ mod tests { #[pg_test] #[search_path(@extschema@)] - #[should_panic = "error: the `env` and `option_env` macros are forbidden"] + #[should_panic = "environment variable `PATH` not defined at compile time."] #[cfg(feature = "trusted")] fn plrust_block_env() -> spi::Result<()> { let definition = r#" @@ -204,15 +204,14 @@ mod tests { #[pg_test] #[search_path(@extschema@)] - #[should_panic = "error: the `env` and `option_env` macros are forbidden"] + #[should_panic = "the `option_env` macro always returns `None` in the PL/Rust user function"] #[cfg(feature = "trusted")] fn plrust_block_option_env() -> spi::Result<()> { let definition = r#" CREATE FUNCTION try_get_path() RETURNS text AS $$ - match option_env!("PATH") { - None => Ok(None), - Some(s) => Ok(Some(s.to_string())) - } + let v = option_env!("PATH") + .expect("the `option_env` macro always returns `None` in the PL/Rust user function"); + Ok(Some(v.to_string())) $$ LANGUAGE plrust; "#; Spi::run(definition) diff --git a/plrustc/plrustc/src/main.rs b/plrustc/plrustc/src/main.rs index 2d712d6e..b69b8175 100644 --- a/plrustc/plrustc/src/main.rs +++ b/plrustc/plrustc/src/main.rs @@ -49,6 +49,14 @@ impl Callbacks for PlrustcCallbacks { } } } +fn clear_env() { + let all_var_names = std::env::vars_os() + .map(|(name, _)| name) + .collect::>(); + for name in all_var_names { + std::env::remove_var(name); + } +} fn main() { rustc_driver::install_ice_hook("https://github.com/tcdi/plrust/issues/new", |_| ()); @@ -58,6 +66,9 @@ fn main() { let args = rustc_driver::args::arg_expand_all(handler, &std::env::args().collect::>()); let config = PlrustcConfig::from_env_and_args(&args); + if config.compiling_user_crate() { + clear_env(); + } run_compiler( args, &mut PlrustcCallbacks { From 046a49220928a3545f4b8b0ffeca7007ca194644 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 1 Nov 2023 10:21:04 -0700 Subject: [PATCH 2/6] hack: set vars to the empty string rather than removing them (this still exposes that they were set, though...) --- plrustc/plrustc/src/main.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plrustc/plrustc/src/main.rs b/plrustc/plrustc/src/main.rs index b69b8175..025cacc3 100644 --- a/plrustc/plrustc/src/main.rs +++ b/plrustc/plrustc/src/main.rs @@ -52,9 +52,13 @@ impl Callbacks for PlrustcCallbacks { fn clear_env() { let all_var_names = std::env::vars_os() .map(|(name, _)| name) + .filter(|name| { + let name = name.to_string_lossy().to_lowercase(); + !(name.starts_with("rust") || name.starts_with("plrust")) + }) .collect::>(); for name in all_var_names { - std::env::remove_var(name); + std::env::set_var(name, ""); } } From 89dfb13a44cfd701d1dd398d6a6065f88405acb7 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 1 Nov 2023 11:36:07 -0700 Subject: [PATCH 3/6] DNM Don't filter path --- plrustc/plrustc/src/main.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plrustc/plrustc/src/main.rs b/plrustc/plrustc/src/main.rs index 025cacc3..d9cc5728 100644 --- a/plrustc/plrustc/src/main.rs +++ b/plrustc/plrustc/src/main.rs @@ -54,11 +54,14 @@ fn clear_env() { .map(|(name, _)| name) .filter(|name| { let name = name.to_string_lossy().to_lowercase(); - !(name.starts_with("rust") || name.starts_with("plrust")) + !(name.starts_with("rust") + || name.starts_with("plrust") + || name.starts_with("cargo") + || name == "path") }) .collect::>(); for name in all_var_names { - std::env::set_var(name, ""); + std::env::remove_var(name); } } From 680875bf6bb4de90306b066d9bc842f1ccb45054 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 1 Nov 2023 16:15:15 -0700 Subject: [PATCH 4/6] Fix tests --- plrust-tests/src/trusted.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/plrust-tests/src/trusted.rs b/plrust-tests/src/trusted.rs index b5ffa5a8..04986420 100644 --- a/plrust-tests/src/trusted.rs +++ b/plrust-tests/src/trusted.rs @@ -190,15 +190,16 @@ mod tests { #[pg_test] #[search_path(@extschema@)] - #[should_panic = "environment variable `PATH` not defined at compile time."] + #[should_panic = "environment variable `FOOBAR` not defined at compile time"] #[cfg(feature = "trusted")] fn plrust_block_env() -> spi::Result<()> { let definition = r#" - CREATE FUNCTION get_path() RETURNS text AS $$ - let path = env!("PATH"); - Ok(Some(path.to_string())) + CREATE FUNCTION get_foobar() RETURNS text AS $$ + let foo = env!("FOOBAR"); + Ok(Some(foo.to_string())) $$ LANGUAGE plrust; "#; + std::env::set_var("FOOBAR", "1"); Spi::run(definition) } @@ -208,12 +209,13 @@ mod tests { #[cfg(feature = "trusted")] fn plrust_block_option_env() -> spi::Result<()> { let definition = r#" - CREATE FUNCTION try_get_path() RETURNS text AS $$ - let v = option_env!("PATH") + CREATE FUNCTION try_get_foobar2() RETURNS text AS $$ + let v = option_env!("FOOBAR2") .expect("the `option_env` macro always returns `None` in the PL/Rust user function"); Ok(Some(v.to_string())) $$ LANGUAGE plrust; "#; + std::env::set_var("FOOBAR2", "1"); Spi::run(definition) } From f8c32bef362f45875a560024999e4b4842119129 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Thu, 2 Nov 2023 10:33:28 -0700 Subject: [PATCH 5/6] Try filtering a few more vars --- plrustc/plrustc/src/main.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/plrustc/plrustc/src/main.rs b/plrustc/plrustc/src/main.rs index d9cc5728..67a1e40d 100644 --- a/plrustc/plrustc/src/main.rs +++ b/plrustc/plrustc/src/main.rs @@ -54,10 +54,13 @@ fn clear_env() { .map(|(name, _)| name) .filter(|name| { let name = name.to_string_lossy().to_lowercase(); - !(name.starts_with("rust") - || name.starts_with("plrust") - || name.starts_with("cargo") - || name == "path") + !( + name.starts_with("plrust") + // || name.starts_with("rust") + // || name.starts_with("cargo") + || name == "path" + // || name == "rustflags" + ) }) .collect::>(); for name in all_var_names { From e992c28f0622eeb94fe97b898c90bd2d3934cd08 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Thu, 2 Nov 2023 12:09:30 -0700 Subject: [PATCH 6/6] Only keep PATH --- plrustc/plrustc/src/main.rs | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/plrustc/plrustc/src/main.rs b/plrustc/plrustc/src/main.rs index 67a1e40d..47ac19bb 100644 --- a/plrustc/plrustc/src/main.rs +++ b/plrustc/plrustc/src/main.rs @@ -49,21 +49,13 @@ impl Callbacks for PlrustcCallbacks { } } } + fn clear_env() { - let all_var_names = std::env::vars_os() - .map(|(name, _)| name) - .filter(|name| { - let name = name.to_string_lossy().to_lowercase(); - !( - name.starts_with("plrust") - // || name.starts_with("rust") - // || name.starts_with("cargo") - || name == "path" - // || name == "rustflags" - ) - }) - .collect::>(); - for name in all_var_names { + for (name, _) in std::env::vars_os() { + // Can't remove `PATH`, need it to locate linker and such. + if name == "PATH" { + continue; + } std::env::remove_var(name); } }