Skip to content

Commit 08a0557

Browse files
marcoxyz123peso
authored andcommitted
Fix crash with single-commit repositories
Multiple places used len() - 1 which causes unsigned integer underflow when length is 0, wrapping to usize::MAX. This caused invalid comparisons in bounds checks and subsequent out-of-bounds index access. Changes: - Added guard for empty indices in graph_view render function - Changed len() - 1 to len().saturating_sub(1) throughout - Used safe .get(idx).copied() instead of direct indexing - Pre-computed max index values to avoid repeated calculations
1 parent fd5b8a1 commit 08a0557

3 files changed

Lines changed: 36 additions & 21 deletions

File tree

src/dialogs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ impl<'a> FileDialog<'a> {
3131

3232
pub fn fwd(&mut self, steps: usize) {
3333
let i = match self.state.selected() {
34-
Some(i) => std::cmp::min(i.saturating_add(steps), self.dirs.len() - 1),
34+
Some(i) => std::cmp::min(i.saturating_add(steps), self.dirs.len().saturating_sub(1)),
3535
None => 0,
3636
};
3737
self.state.select(Some(i));

src/widgets/graph_view.rs

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@ impl GraphViewState {
2929
pub fn move_selection(&mut self, steps: usize, down: bool) -> bool {
3030
let changed = if let Some(sel) = self.selected {
3131
let new_idx = if down {
32-
std::cmp::min(sel.saturating_add(steps), self.indices.len() - 1)
32+
std::cmp::min(
33+
sel.saturating_add(steps),
34+
self.indices.len().saturating_sub(1),
35+
)
3336
} else {
34-
std::cmp::max(sel.saturating_sub(steps), 0)
37+
sel.saturating_sub(steps)
3538
};
3639
self.selected = Some(new_idx);
3740
new_idx != sel
@@ -49,18 +52,24 @@ impl GraphViewState {
4952
pub fn move_secondary_selection(&mut self, steps: usize, down: bool) -> bool {
5053
let changed = if let Some(sel) = self.secondary_selected {
5154
let new_idx = if down {
52-
std::cmp::min(sel.saturating_add(steps), self.indices.len() - 1)
55+
std::cmp::min(
56+
sel.saturating_add(steps),
57+
self.indices.len().saturating_sub(1),
58+
)
5359
} else {
54-
std::cmp::max(sel.saturating_sub(steps), 0)
60+
sel.saturating_sub(steps)
5561
};
5662
self.secondary_selected = Some(new_idx);
5763
new_idx != sel
5864
} else if !self.graph_lines.is_empty() {
5965
if let Some(sel) = self.selected {
6066
let new_idx = if down {
61-
std::cmp::min(sel.saturating_add(steps), self.indices.len() - 1)
67+
std::cmp::min(
68+
sel.saturating_add(steps),
69+
self.indices.len().saturating_sub(1),
70+
)
6271
} else {
63-
std::cmp::max(sel.saturating_sub(steps), 0)
72+
sel.saturating_sub(steps)
6473
};
6574
self.secondary_selected = Some(new_idx);
6675
new_idx != sel
@@ -131,7 +140,7 @@ impl StatefulWidget for GraphView<'_> {
131140
return;
132141
}
133142

134-
if state.graph_lines.is_empty() {
143+
if state.graph_lines.is_empty() || state.indices.is_empty() {
135144
return;
136145
}
137146
let list_height = list_area.height as usize;
@@ -144,31 +153,37 @@ impl StatefulWidget for GraphView<'_> {
144153
);
145154
let mut end = start + height;
146155

147-
let selected_row = state.selected.map(|idx| state.indices[idx]);
148-
let selected = selected_row.unwrap_or(0).min(state.graph_lines.len() - 1);
156+
let max_graph_idx = state.graph_lines.len().saturating_sub(1);
157+
let max_indices_idx = state.indices.len().saturating_sub(1);
149158

150-
let secondary_selected_row = state.secondary_selected.map(|idx| state.indices[idx]);
151-
let secondary_selected = secondary_selected_row
152-
.unwrap_or(0)
153-
.min(state.graph_lines.len() - 1);
159+
let selected_row = state
160+
.selected
161+
.and_then(|idx| state.indices.get(idx).copied());
162+
let selected = selected_row.unwrap_or(0).min(max_graph_idx);
163+
164+
let secondary_selected_row = state
165+
.secondary_selected
166+
.and_then(|idx| state.indices.get(idx).copied());
167+
let secondary_selected = secondary_selected_row.unwrap_or(0).min(max_graph_idx);
154168

155169
let selected_index = if state.secondary_changed {
156-
state.secondary_selected.unwrap_or(0)
170+
state.secondary_selected.unwrap_or(0).min(max_indices_idx)
157171
} else {
158-
state.selected.unwrap_or(0)
172+
state.selected.unwrap_or(0).min(max_indices_idx)
159173
};
160174
let move_to_selected = if state.secondary_changed {
161175
secondary_selected
162176
} else {
163177
selected
164178
};
165179

166-
let move_to_end = if selected_index >= state.indices.len() - 1 {
167-
state.graph_lines.len() - 1
180+
let move_to_end = if selected_index >= max_indices_idx {
181+
max_graph_idx
168182
} else {
169-
(state.indices[selected_index + 1] - 1)
183+
state.indices[selected_index + 1]
184+
.saturating_sub(1)
170185
.max(move_to_selected + SCROLL_MARGIN)
171-
.min(state.graph_lines.len() - 1)
186+
.min(max_graph_idx)
172187
};
173188
let move_to_start = move_to_selected.saturating_sub(SCROLL_MARGIN);
174189

src/widgets/models_view.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ impl ModelListState {
1717

1818
pub fn fwd(&mut self, steps: usize) {
1919
let i = match self.state.selected() {
20-
Some(i) => std::cmp::min(i.saturating_add(steps), self.models.len() - 1),
20+
Some(i) => std::cmp::min(i.saturating_add(steps), self.models.len().saturating_sub(1)),
2121
None => 0,
2222
};
2323
self.state.select(Some(i));

0 commit comments

Comments
 (0)