Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(txnames): Avoid replacing url encoded transaction names #1839

Merged
merged 10 commits into from
Feb 13, 2023
Merged

fix(txnames): Avoid replacing url encoded transaction names #1839

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8b3424a
fix(txnames): avoid replacing url encoded transaction names
olksdr Feb 10, 2023
6962f3c
fix: regex for hex
olksdr Feb 10, 2023
36cb301
Update regex
olksdr Feb 10, 2023
a20c1b0
Merge branch 'master' into fix/do-not-match-urlencoded-string
olksdr Feb 10, 2023
d17826b
Update regex for ints
olksdr Feb 10, 2023
9ec6703
Merge branch 'fix/do-not-match-urlencoded-string' of github.com:getse…
olksdr Feb 10, 2023
090fd3a
fix meta
olksdr Feb 10, 2023
6bada52
Merge branch 'master' into fix/do-not-match-urlencoded-string
olksdr Feb 13, 2023
85a517d
Give the capture a better name
olksdr Feb 13, 2023
1a7bb7b
rename all "m" to captures
olksdr Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions relay-general/src/store/regexes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,14 @@ pub static TRANSACTION_NAME_NORMALIZER_REGEX: Lazy<Regex> = Lazy::new(|| {
(?P<hex>[^/\\]*
\b0[xX][0-9a-fA-F]+\b
[^/\\]*) |
(?P<int>[^/\\]*
\b\d{2,}\b
[^/\\]*)"#,
(?:
(?:[^/\\]*
%[0-9A-F]{2}
[^/\\]*) |
(?P<int>[^/\\]*
\b\d{2,}\b
[^/\\]*)
)"#,
)
.unwrap()
});
32 changes: 25 additions & 7 deletions relay-general/src/store/transactions/processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,26 +266,34 @@ fn normalize_transaction_name(transaction: &mut Annotated<String>) -> Processing
.collect::<Vec<_>>();

transaction.apply(|trans, meta| {
let mut caps = Vec::new();
// Collect all the remarks if anything matches.
for matches in TRANSACTION_NAME_NORMALIZER_REGEX.captures_iter(trans) {
for name in &capture_names {
if let Some(m) = matches.name(name) {
let remark =
Remark::with_range(RemarkType::Substituted, *name, (m.start(), m.end()));
meta.add_remark(remark);
caps.push((m, remark));
break;
}
}
}

let changed = TRANSACTION_NAME_NORMALIZER_REGEX
.replace_all(trans, "*")
.to_string();
if *trans != changed && changed != "*" {
// Sort by the longest match end.
caps.sort_by_key(|(m, _)| m.end());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Rename m to match everywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

match is actually the reserved word 😄. But I also can change it to capture, since it's basically what it is. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 85a517d

let mut changed = String::with_capacity(trans.len());
let mut last_end = 0usize;
for (m, remark) in caps {
changed.push_str(&trans[last_end..m.start()]);
changed.push('*');
last_end = m.end();
meta.add_remark(remark);
}
changed.push_str(&trans[last_end..]);

if !changed.is_empty() && changed != "*" {
meta.set_original_value(Some(trans.to_string()));
*trans = changed
} else {
meta.clear_remarks();
}
Ok(())
})
Expand Down Expand Up @@ -1882,6 +1890,16 @@ mod tests {
"/testing/open-19-close/1",
"/testing/*/1"
);
transaction_name_test!(
test_transaction_name_normalize_url_encode_1,
"/%2Ftest%2Fopen%20and%20help%2F1%0A",
"/%2Ftest%2Fopen%20and%20help%2F1%0A"
);
transaction_name_test!(
test_transaction_name_normalize_url_encode_2,
"/this/1234/%E2%9C%85/foo/bar/098123908213",
"/this/*/%E2%9C%85/foo/bar/*"
);
jan-auer marked this conversation as resolved.
Show resolved Hide resolved
transaction_name_test!(
test_transaction_name_normalize_sha,
"/hash/4c79f60c11214eb38604f4ae0781bfb2/diff",
Expand Down