forked from extern/nushell
Add boundary check for str index-of (#11190)
<!-- if this PR closes one or more issues, you can automatically link the PR with them by using one of the [*linking keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), e.g. - this PR should close #xxxx - fixes #xxxx you can also mention related issues, PRs or discussions! --> # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> This PR tries to fix #10774 # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> * before ![Screenshot from 2023-11-30 20-18-56](https://github.com/nushell/nushell/assets/15247421/4559e114-0757-4a73-91e7-c7536a8ce5f1) ![Screenshot from 2023-11-30 20-19-23](https://github.com/nushell/nushell/assets/15247421/30ab1b5a-3ec3-48a8-ae76-65721b650fcf) * after ![Screenshot from 2023-11-30 20-21-04](https://github.com/nushell/nushell/assets/15247421/f34fd276-dccf-4328-b9d2-bf368b5b3ae5) ![Screenshot from 2023-11-30 20-20-39](https://github.com/nushell/nushell/assets/15247421/653e47d8-8d68-4ef3-adef-0865179c4f9b) # Tests + Formatting <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. -->
This commit is contained in:
parent
54d73748e4
commit
250ee62e16
@ -13,7 +13,7 @@ use unicode_segmentation::UnicodeSegmentation;
|
|||||||
struct Arguments {
|
struct Arguments {
|
||||||
end: bool,
|
end: bool,
|
||||||
substring: String,
|
substring: String,
|
||||||
range: Option<Range>,
|
range: Option<Spanned<Range>>,
|
||||||
cell_paths: Option<Vec<CellPath>>,
|
cell_paths: Option<Vec<CellPath>>,
|
||||||
graphemes: bool,
|
graphemes: bool,
|
||||||
}
|
}
|
||||||
@ -147,7 +147,10 @@ fn action(
|
|||||||
) -> Value {
|
) -> Value {
|
||||||
match input {
|
match input {
|
||||||
Value::String { val: s, .. } => {
|
Value::String { val: s, .. } => {
|
||||||
let (start_index, end_index) = if let Some(range) = range {
|
let mut range_span = head;
|
||||||
|
let (start_index, end_index) = if let Some(spanned_range) = range {
|
||||||
|
range_span = spanned_range.span;
|
||||||
|
let range = &spanned_range.item;
|
||||||
match util::process_range(range) {
|
match util::process_range(range) {
|
||||||
Ok(r) => {
|
Ok(r) => {
|
||||||
// `process_range()` returns `isize::MAX` if the range is open-ended,
|
// `process_range()` returns `isize::MAX` if the range is open-ended,
|
||||||
@ -168,6 +171,17 @@ fn action(
|
|||||||
(0usize, s.len())
|
(0usize, s.len())
|
||||||
};
|
};
|
||||||
|
|
||||||
|
if s.get(start_index..end_index).is_none() {
|
||||||
|
return Value::error(
|
||||||
|
ShellError::InvalidRange {
|
||||||
|
left_flank: start_index.to_string(),
|
||||||
|
right_flank: end_index.to_string(),
|
||||||
|
span: range_span,
|
||||||
|
},
|
||||||
|
head,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// When the -e flag is present, search using rfind instead of find.s
|
// When the -e flag is present, search using rfind instead of find.s
|
||||||
if let Some(result) = if *end {
|
if let Some(result) = if *end {
|
||||||
s[start_index..end_index].rfind(&**substring)
|
s[start_index..end_index].rfind(&**substring)
|
||||||
@ -265,10 +279,15 @@ mod tests {
|
|||||||
inclusion: RangeInclusion::Inclusive,
|
inclusion: RangeInclusion::Inclusive,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
let spanned_range = Spanned {
|
||||||
|
item: range,
|
||||||
|
span: Span::test_data(),
|
||||||
|
};
|
||||||
|
|
||||||
let options = Arguments {
|
let options = Arguments {
|
||||||
substring: String::from("Cargo"),
|
substring: String::from("Cargo"),
|
||||||
|
|
||||||
range: Some(range),
|
range: Some(spanned_range),
|
||||||
cell_paths: None,
|
cell_paths: None,
|
||||||
end: false,
|
end: false,
|
||||||
graphemes: false,
|
graphemes: false,
|
||||||
@ -288,10 +307,15 @@ mod tests {
|
|||||||
to: Value::int(5, Span::test_data()),
|
to: Value::int(5, Span::test_data()),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
let spanned_range = Spanned {
|
||||||
|
item: range,
|
||||||
|
span: Span::test_data(),
|
||||||
|
};
|
||||||
|
|
||||||
let options = Arguments {
|
let options = Arguments {
|
||||||
substring: String::from("Banana"),
|
substring: String::from("Banana"),
|
||||||
|
|
||||||
range: Some(range),
|
range: Some(spanned_range),
|
||||||
cell_paths: None,
|
cell_paths: None,
|
||||||
end: false,
|
end: false,
|
||||||
graphemes: false,
|
graphemes: false,
|
||||||
@ -311,10 +335,15 @@ mod tests {
|
|||||||
inclusion: RangeInclusion::Inclusive,
|
inclusion: RangeInclusion::Inclusive,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
let spanned_range = Spanned {
|
||||||
|
item: range,
|
||||||
|
span: Span::test_data(),
|
||||||
|
};
|
||||||
|
|
||||||
let options = Arguments {
|
let options = Arguments {
|
||||||
substring: String::from("123"),
|
substring: String::from("123"),
|
||||||
|
|
||||||
range: Some(range),
|
range: Some(spanned_range),
|
||||||
cell_paths: None,
|
cell_paths: None,
|
||||||
end: false,
|
end: false,
|
||||||
graphemes: false,
|
graphemes: false,
|
||||||
@ -334,10 +363,15 @@ mod tests {
|
|||||||
inclusion: RangeInclusion::RightExclusive,
|
inclusion: RangeInclusion::RightExclusive,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
let spanned_range = Spanned {
|
||||||
|
item: range,
|
||||||
|
span: Span::test_data(),
|
||||||
|
};
|
||||||
|
|
||||||
let options = Arguments {
|
let options = Arguments {
|
||||||
substring: String::from("1"),
|
substring: String::from("1"),
|
||||||
|
|
||||||
range: Some(range),
|
range: Some(spanned_range),
|
||||||
cell_paths: None,
|
cell_paths: None,
|
||||||
end: false,
|
end: false,
|
||||||
graphemes: false,
|
graphemes: false,
|
||||||
@ -363,4 +397,62 @@ mod tests {
|
|||||||
let actual = action(&word, &options, Span::test_data());
|
let actual = action(&word, &options, Span::test_data());
|
||||||
assert_eq!(actual, Value::test_int(15));
|
assert_eq!(actual, Value::test_int(15));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn index_is_not_a_char_boundary() {
|
||||||
|
let word = Value::string(String::from("💛"), Span::test_data());
|
||||||
|
|
||||||
|
let range = Range {
|
||||||
|
from: Value::int(0, Span::test_data()),
|
||||||
|
incr: Value::int(1, Span::test_data()),
|
||||||
|
to: Value::int(3, Span::test_data()),
|
||||||
|
inclusion: RangeInclusion::Inclusive,
|
||||||
|
};
|
||||||
|
|
||||||
|
let spanned_range = Spanned {
|
||||||
|
item: range,
|
||||||
|
span: Span::test_data(),
|
||||||
|
};
|
||||||
|
|
||||||
|
let options = Arguments {
|
||||||
|
substring: String::new(),
|
||||||
|
|
||||||
|
range: Some(spanned_range),
|
||||||
|
cell_paths: None,
|
||||||
|
end: false,
|
||||||
|
graphemes: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
let actual = action(&word, &options, Span::test_data());
|
||||||
|
assert!(actual.is_error());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn index_is_out_of_bounds() {
|
||||||
|
let word = Value::string(String::from("hello"), Span::test_data());
|
||||||
|
|
||||||
|
let range = Range {
|
||||||
|
from: Value::int(-1, Span::test_data()),
|
||||||
|
incr: Value::int(1, Span::test_data()),
|
||||||
|
to: Value::int(3, Span::test_data()),
|
||||||
|
inclusion: RangeInclusion::Inclusive,
|
||||||
|
};
|
||||||
|
|
||||||
|
let spanned_range = Spanned {
|
||||||
|
item: range,
|
||||||
|
span: Span::test_data(),
|
||||||
|
};
|
||||||
|
|
||||||
|
let options = Arguments {
|
||||||
|
substring: String::from("h"),
|
||||||
|
|
||||||
|
range: Some(spanned_range),
|
||||||
|
cell_paths: None,
|
||||||
|
end: false,
|
||||||
|
graphemes: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
let actual = action(&word, &options, Span::test_data());
|
||||||
|
assert!(actual.is_error());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user