From b626fd43d8bfa52ad5bfc836543008b6dde26b6c Mon Sep 17 00:00:00 2001 From: Eric Swanson <64809312+ericswanson-dfinity@users.noreply.github.com> Date: Mon, 4 Mar 2024 13:32:25 -0800 Subject: [PATCH] fix: .ic-assets.json configuration entries no longer overwrite the default for allow_raw_access (#3635) For for all entries in .ic-assets.json being treated as if they specify allow_raw_access: true if not explicitly set Fixes: https://dfinity.atlassian.net/browse/SDK-1238 --- CHANGELOG.md | 28 +++ e2e/tests-dfx/assetscanister.bash | 63 +++++-- .../frontend/ic-asset/src/asset/config.rs | 163 +++++++++++++++++- 3 files changed, 239 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6dd661dd2..da41370154 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,34 @@ # UNRELEASED +# fix: .ic-assets.json configuration entries no longer overwrite the default for `allow_raw_access` + +Previously, any configuration element in .ic-assets.json functioned as if a setting of +`"allow_raw_access": true` were present in the json object. + +For example, given the following configuration, all files would be configured +with `allow_raw_access` set to `true`, as if the second entry specified +`"allow_raw_access": true` (which is the default), even though it does not. + +```json +[ + { + "match": "**/*", + "allow_raw_access": false + }, + { + "match": "**/*", + "headers": { + "X-Anything": "Something" + } + } +] +``` + +Now, given the same configuration, all files would be configured with `allow_raw_access` set to false, as expected. + +Note that the default value of `allow_raw_access` is still `true`. + # 0.18.0 ### fix!: removed the `dfx upgrade` command diff --git a/e2e/tests-dfx/assetscanister.bash b/e2e/tests-dfx/assetscanister.bash index 56e6c3aca3..6cf4ecc551 100644 --- a/e2e/tests-dfx/assetscanister.bash +++ b/e2e/tests-dfx/assetscanister.bash @@ -1163,6 +1163,54 @@ CHERRIES" "$stdout" # /.well-known/thing.json 1/1 (0 bytes) } +@test "asset configuration via .ic-assets.json5 - default properties" { + install_asset assetscanister + rm src/e2e_project_frontend/assets/.ic-assets.json5 + dfx_start + assert_command dfx deploy + assert_command dfx canister call e2e_project_frontend get_asset_properties '("/binary/noise.txt")' + assert_contains '( + record { + headers = null; + is_aliased = null; + allow_raw_access = opt true; + max_age = null; + }, +)' +} + +@test "asset configuration via .ic-assets.json5 - property override order does not matter" { + install_asset assetscanister + # Ensure that setting one property does not overwrite a different property set earlier + cat > src/e2e_project_frontend/assets/.ic-assets.json5 <, } -fn default_raw_access() -> Option { - Some(true) -} - /// A single configuration object, from `.ic-assets.json` config file #[derive(Derivative, Clone, Serialize)] #[derivative(Debug, PartialEq)] @@ -348,7 +344,6 @@ mod rule_utils { headers: Maybe, ignore: Option, enable_aliasing: Option, - #[serde(default = "super::default_raw_access")] allow_raw_access: Option, } @@ -427,6 +422,16 @@ mod rule_utils { s.push_str(&format!(" - HTTP cache max-age: {}\n", max_age)); } } + if let Some(allow_raw_access) = self.allow_raw_access { + s.push_str(&format!( + " - enable raw access: {}\n", + if allow_raw_access { + "enabled" + } else { + "disabled" + } + )); + } if let Some(aliasing) = self.enable_aliasing { s.push_str(&format!( " - URL path aliasing: {}\n", @@ -936,4 +941,152 @@ mod with_tempdir { }, ); } + + #[test] + fn the_order_does_not_matter() { + let cfg = Some(HashMap::from([( + "".to_string(), + r#"[ + { + "match": "**/deep/**/*", + "allow_raw_access": false, + "cache": { + "max_age": 22 + }, + "enable_aliasing": true, + "ignore": true + }, + { + "match": "**/*", + "headers": { + "X-Frame-Options": "DENY" + } + } + ] +"# + .to_string(), + )])); + let cfg2 = Some(HashMap::from([( + "".to_string(), + r#"[ + { + "match": "**/*", + "headers": { + "X-Frame-Options": "DENY" + } + }, + { + "match": "**/deep/**/*", + "allow_raw_access": false, + "cache": { + "max_age": 22 + }, + "enable_aliasing": true, + "ignore": true + } + ] + "# + .to_string(), + )])); + + let x = { + let assets_temp_dir = create_temporary_assets_directory(cfg, 0); + let assets_dir = assets_temp_dir.path().canonicalize().unwrap(); + let mut assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir).unwrap(); + assets_config + .get_asset_config(assets_dir.join("nested/deep/the-next-thing.toml").as_path()) + .unwrap() + }; + let y = { + let assets_temp_dir = create_temporary_assets_directory(cfg2, 0); + let assets_dir = assets_temp_dir.path().canonicalize().unwrap(); + let mut assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir).unwrap(); + assets_config + .get_asset_config(assets_dir.join("nested/deep/the-next-thing.toml").as_path()) + .unwrap() + }; + + dbg!(&x, &y); + assert_eq!(x.allow_raw_access, Some(false)); + assert_eq!(y.allow_raw_access, Some(false)); + assert_eq!(x.enable_aliasing, Some(true)); + assert_eq!(y.enable_aliasing, Some(true)); + assert_eq!(x.ignore, Some(true)); + assert_eq!(y.ignore, Some(true)); + assert_eq!(x.cache.clone().unwrap().max_age, Some(22)); + assert_eq!(y.cache.clone().unwrap().max_age, Some(22)); + + // same as above but with different values + let cfg = Some(HashMap::from([( + "".to_string(), + r#"[ + { + "match": "**/deep/**/*", + "allow_raw_access": true, + "enable_aliasing": false, + "ignore": false, + "headers": { + "X-Frame-Options": "ALLOW" + } + }, + { + "match": "**/*", + "cache": { + "max_age": 22 + } + } + ] +"# + .to_string(), + )])); + let cfg2 = Some(HashMap::from([( + "".to_string(), + r#"[ + { + "match": "**/*", + "cache": { + "max_age": 22 + } + }, + { + "match": "**/deep/**/*", + "allow_raw_access": true, + "enable_aliasing": false, + "ignore": false, + "headers": { + "X-Frame-Options": "ALLOW" + } + } + ] + "# + .to_string(), + )])); + + let x = { + let assets_temp_dir = create_temporary_assets_directory(cfg, 0); + let assets_dir = assets_temp_dir.path().canonicalize().unwrap(); + let mut assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir).unwrap(); + assets_config + .get_asset_config(assets_dir.join("nested/deep/the-next-thing.toml").as_path()) + .unwrap() + }; + let y = { + let assets_temp_dir = create_temporary_assets_directory(cfg2, 0); + let assets_dir = assets_temp_dir.path().canonicalize().unwrap(); + let mut assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir).unwrap(); + assets_config + .get_asset_config(assets_dir.join("nested/deep/the-next-thing.toml").as_path()) + .unwrap() + }; + + dbg!(&x, &y); + assert_eq!(x.allow_raw_access, Some(true)); + assert_eq!(y.allow_raw_access, Some(true)); + assert_eq!(x.enable_aliasing, Some(false)); + assert_eq!(y.enable_aliasing, Some(false)); + assert_eq!(x.ignore, Some(false)); + assert_eq!(y.ignore, Some(false)); + assert_eq!(x.cache.clone().unwrap().max_age, Some(22)); + assert_eq!(y.cache.clone().unwrap().max_age, Some(22)); + } }