Skip to content

Commit c70bdc8

Browse files
committed
fix: encode & and + in modelarmor create-template query value
build_create_template_url embeds template_id into a query string via raw format!(), not a .query() builder. encode_path_segment (this PR's prior commit) deliberately leaves & and + unencoded since both are legal in a bare path segment — but neither is safe in a query string: an unencoded & splits the value into two parameters (silently truncating the intended template_id), and + is commonly decoded server-side as a space. validate_resource_name (which template_id passes through first) does not reject & or +, so this was reachable from --template-id input. Confirmed by compiling and running url::Url::parse against the actual encode set before this fix. Add a fork-local encode_query_value in modelarmor.rs rather than reusing/exporting validate::encode_path_segment's encode set, so that function can stay a byte-for-byte match with the upstream fix (googleworkspace#867) it backports.
1 parent 1f44901 commit c70bdc8

1 file changed

Lines changed: 58 additions & 1 deletion

File tree

crates/google-workspace-cli/src/helpers/modelarmor.rs

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,14 +366,53 @@ fn parse_create_template_args(matches: &ArgMatches) -> Result<CreateTemplateConf
366366
})
367367
}
368368

369+
/// Percent-encode a value for use as a query parameter value in a URL built
370+
/// by string formatting (rather than a `.query()` builder). `encode_path_segment`
371+
/// is NOT safe here: `&` and `+` are legal in a bare path segment (so it
372+
/// leaves them intact), but both are structurally significant in a query
373+
/// string — an unencoded `&` splits the value into two parameters, and `+` is
374+
/// commonly decoded server-side as a space. This mirrors
375+
/// `validate::encode_path_segment`'s encode set plus those two extra bytes
376+
/// (duplicated rather than shared, so that function can stay a byte-for-byte
377+
/// match with the equivalent upstream fix).
378+
fn encode_query_value(s: &str) -> String {
379+
use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS};
380+
381+
const QUERY_VALUE_ENCODE_SET: &AsciiSet = &CONTROLS
382+
.add(b' ')
383+
.add(b'"')
384+
.add(b'#')
385+
.add(b'%')
386+
.add(b'&')
387+
.add(b'+')
388+
.add(b'<')
389+
.add(b'>')
390+
.add(b'?')
391+
.add(b'`')
392+
.add(b'{')
393+
.add(b'}')
394+
.add(b'/')
395+
.add(b':')
396+
.add(b';')
397+
.add(b'=')
398+
.add(b'@')
399+
.add(b'[')
400+
.add(b'\\')
401+
.add(b']')
402+
.add(b'^')
403+
.add(b'|');
404+
405+
utf8_percent_encode(s, QUERY_VALUE_ENCODE_SET).to_string()
406+
}
407+
369408
pub fn build_create_template_url(config: &CreateTemplateConfig) -> String {
370409
let base = regional_base_url(&config.location);
371410
let project = crate::validate::encode_path_segment(&config.project);
372411
let location = crate::validate::encode_path_segment(&config.location);
373412
let parent = format!("projects/{project}/locations/{location}");
374413
format!(
375414
"{base}/{parent}/templates?templateId={}",
376-
crate::validate::encode_path_segment(&config.template_id)
415+
encode_query_value(&config.template_id)
377416
)
378417
}
379418

@@ -763,6 +802,24 @@ mod parsing_tests {
763802
assert!(url.contains("templateId=my%20template"));
764803
}
765804

805+
#[test]
806+
fn test_build_create_template_url_encodes_query_delimiters_in_template_id() {
807+
// Regression test: template_id lands in a query string built by raw
808+
// string formatting, not a `.query()` builder. `encode_path_segment`
809+
// deliberately leaves `&`/`+` intact (they're legal in a bare path
810+
// segment), so using it here would let `&` split templateId into two
811+
// query parameters, and `+` would be decoded server-side as a space.
812+
let config = CreateTemplateConfig {
813+
project: "p".to_string(),
814+
location: "us-central1".to_string(),
815+
template_id: "my&extra+id".to_string(),
816+
body: "{}".to_string(),
817+
};
818+
let url = build_create_template_url(&config);
819+
assert!(url.contains("templateId=my%26extra%2Bid"));
820+
assert!(!url.contains("templateId=my&extra"));
821+
}
822+
766823
#[test]
767824
fn test_parse_create_template_args_rejects_traversal() {
768825
let matches = make_matches_create(&[

0 commit comments

Comments
 (0)