Skip to content

Commit 70d21a9

Browse files
diegodoxarchseersudormrfbin
authored
Prevent preview binary or large file (#939)
* Prevent preview binary or large file (#847) * fix wrong method name * fix add use trait * update lock file * rename MAX_PREVIEW_SIZE from MAX_BYTE_PREVIEW * read small bytes to determine cotent type * [WIP] add preview struct to represent calcurated preveiw * Refactor content type detection - Remove unwraps - Reuse a single read buffer to avoid 1kb reallocations between previews * Refactor preview rendering so we don't construct docs when not necessary * Replace unwarap whit Preview::NotFound * Use index access to hide unwrap Co-authored-by: Blaž Hrastnik <blaz@mxxn.io> * fix Get and unwarp equivalent to referce of Index acess * better preview implementation * Rename Preview enum and vairant Co-authored-by: Gokul Soumya <gokulps15@gmail.com> * fixup! Rename Preview enum and vairant * simplify long match * Center text, add docs, fix formatting, refactor Co-authored-by: Blaž Hrastnik <blaz@mxxn.io> Co-authored-by: Gokul Soumya <gokulps15@gmail.com>
1 parent 5b5d1b9 commit 70d21a9

File tree

3 files changed

+115
-20
lines changed

3 files changed

+115
-20
lines changed

Cargo.lock

+10
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

helix-term/Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ fuzzy-matcher = "0.3"
4646
ignore = "0.4"
4747
# markdown doc rendering
4848
pulldown-cmark = { version = "0.8", default-features = false }
49+
# file type detection
50+
content_inspector = "0.2.4"
4951

5052
# config
5153
toml = "0.5"

helix-term/src/ui/picker.rs

+103-20
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ use fuzzy_matcher::skim::SkimMatcherV2 as Matcher;
1212
use fuzzy_matcher::FuzzyMatcher;
1313
use tui::widgets::Widget;
1414

15-
use std::{borrow::Cow, collections::HashMap, path::PathBuf};
15+
use std::{
16+
borrow::Cow,
17+
collections::HashMap,
18+
io::Read,
19+
path::{Path, PathBuf},
20+
};
1621

1722
use crate::ui::{Prompt, PromptEvent};
1823
use helix_core::Position;
@@ -23,18 +28,58 @@ use helix_view::{
2328
};
2429

2530
pub const MIN_SCREEN_WIDTH_FOR_PREVIEW: u16 = 80;
31+
/// Biggest file size to preview in bytes
32+
pub const MAX_FILE_SIZE_FOR_PREVIEW: u64 = 10 * 1024 * 1024;
2633

27-
/// File path and line number (used to align and highlight a line)
34+
/// File path and range of lines (used to align and highlight lines)
2835
type FileLocation = (PathBuf, Option<(usize, usize)>);
2936

3037
pub struct FilePicker<T> {
3138
picker: Picker<T>,
3239
/// Caches paths to documents
33-
preview_cache: HashMap<PathBuf, Document>,
40+
preview_cache: HashMap<PathBuf, CachedPreview>,
41+
read_buffer: Vec<u8>,
3442
/// Given an item in the picker, return the file path and line number to display.
3543
file_fn: Box<dyn Fn(&Editor, &T) -> Option<FileLocation>>,
3644
}
3745

46+
pub enum CachedPreview {
47+
Document(Document),
48+
Binary,
49+
LargeFile,
50+
NotFound,
51+
}
52+
53+
// We don't store this enum in the cache so as to avoid lifetime constraints
54+
// from borrowing a document already opened in the editor.
55+
pub enum Preview<'picker, 'editor> {
56+
Cached(&'picker CachedPreview),
57+
EditorDocument(&'editor Document),
58+
}
59+
60+
impl Preview<'_, '_> {
61+
fn document(&self) -> Option<&Document> {
62+
match self {
63+
Preview::EditorDocument(doc) => Some(doc),
64+
Preview::Cached(CachedPreview::Document(doc)) => Some(doc),
65+
_ => None,
66+
}
67+
}
68+
69+
/// Alternate text to show for the preview.
70+
fn placeholder(&self) -> &str {
71+
match *self {
72+
Self::EditorDocument(_) => "<File preview>",
73+
Self::Cached(preview) => match preview {
74+
CachedPreview::Document(_) => "<File preview>",
75+
CachedPreview::Binary => "<Binary file>",
76+
CachedPreview::LargeFile => "<File too large to preview>",
77+
CachedPreview::NotFound => "<File not found>",
78+
},
79+
}
80+
}
81+
}
82+
3883
impl<T> FilePicker<T> {
3984
pub fn new(
4085
options: Vec<T>,
@@ -45,6 +90,7 @@ impl<T> FilePicker<T> {
4590
Self {
4691
picker: Picker::new(false, options, format_fn, callback_fn),
4792
preview_cache: HashMap::new(),
93+
read_buffer: Vec::with_capacity(1024),
4894
file_fn: Box::new(preview_fn),
4995
}
5096
}
@@ -60,14 +106,45 @@ impl<T> FilePicker<T> {
60106
})
61107
}
62108

63-
fn calculate_preview(&mut self, editor: &Editor) {
64-
if let Some((path, _line)) = self.current_file(editor) {
65-
if !self.preview_cache.contains_key(&path) && editor.document_by_path(&path).is_none() {
66-
// TODO: enable syntax highlighting; blocked by async rendering
67-
let doc = Document::open(&path, None, Some(&editor.theme), None).unwrap();
68-
self.preview_cache.insert(path, doc);
69-
}
109+
/// Get (cached) preview for a given path. If a document corresponding
110+
/// to the path is already open in the editor, it is used instead.
111+
fn get_preview<'picker, 'editor>(
112+
&'picker mut self,
113+
path: &Path,
114+
editor: &'editor Editor,
115+
) -> Preview<'picker, 'editor> {
116+
if let Some(doc) = editor.document_by_path(path) {
117+
return Preview::EditorDocument(doc);
118+
}
119+
120+
if self.preview_cache.contains_key(path) {
121+
return Preview::Cached(&self.preview_cache[path]);
70122
}
123+
124+
let data = std::fs::File::open(path).and_then(|file| {
125+
let metadata = file.metadata()?;
126+
// Read up to 1kb to detect the content type
127+
let n = file.take(1024).read_to_end(&mut self.read_buffer)?;
128+
let content_type = content_inspector::inspect(&self.read_buffer[..n]);
129+
self.read_buffer.clear();
130+
Ok((metadata, content_type))
131+
});
132+
let preview = data
133+
.map(
134+
|(metadata, content_type)| match (metadata.len(), content_type) {
135+
(_, content_inspector::ContentType::BINARY) => CachedPreview::Binary,
136+
(size, _) if size > MAX_FILE_SIZE_FOR_PREVIEW => CachedPreview::LargeFile,
137+
_ => {
138+
// TODO: enable syntax highlighting; blocked by async rendering
139+
Document::open(path, None, Some(&editor.theme), None)
140+
.map(CachedPreview::Document)
141+
.unwrap_or(CachedPreview::NotFound)
142+
}
143+
},
144+
)
145+
.unwrap_or(CachedPreview::NotFound);
146+
self.preview_cache.insert(path.to_owned(), preview);
147+
Preview::Cached(&self.preview_cache[path])
71148
}
72149
}
73150

@@ -79,12 +156,12 @@ impl<T: 'static> Component for FilePicker<T> {
79156
// |picker | | |
80157
// | | | |
81158
// +---------+ +---------+
82-
self.calculate_preview(cx.editor);
83159
let render_preview = area.width > MIN_SCREEN_WIDTH_FOR_PREVIEW;
84160
let area = inner_rect(area);
85161
// -- Render the frame:
86162
// clear area
87163
let background = cx.editor.theme.get("ui.background");
164+
let text = cx.editor.theme.get("ui.text");
88165
surface.clear_with(area, background);
89166

90167
let picker_width = if render_preview {
@@ -113,17 +190,23 @@ impl<T: 'static> Component for FilePicker<T> {
113190
horizontal: 1,
114191
};
115192
let inner = inner.inner(&margin);
116-
117193
block.render(preview_area, surface);
118194

119-
if let Some((doc, line)) = self.current_file(cx.editor).and_then(|(path, range)| {
120-
cx.editor
121-
.document_by_path(&path)
122-
.or_else(|| self.preview_cache.get(&path))
123-
.zip(Some(range))
124-
}) {
195+
if let Some((path, range)) = self.current_file(cx.editor) {
196+
let preview = self.get_preview(&path, cx.editor);
197+
let doc = match preview.document() {
198+
Some(doc) => doc,
199+
None => {
200+
let alt_text = preview.placeholder();
201+
let x = inner.x + inner.width.saturating_sub(alt_text.len() as u16) / 2;
202+
let y = inner.y + inner.height / 2;
203+
surface.set_stringn(x, y, alt_text, inner.width as usize, text);
204+
return;
205+
}
206+
};
207+
125208
// align to middle
126-
let first_line = line
209+
let first_line = range
127210
.map(|(start, end)| {
128211
let height = end.saturating_sub(start) + 1;
129212
let middle = start + (height.saturating_sub(1) / 2);
@@ -150,7 +233,7 @@ impl<T: 'static> Component for FilePicker<T> {
150233
);
151234

152235
// highlight the line
153-
if let Some((start, end)) = line {
236+
if let Some((start, end)) = range {
154237
let offset = start.saturating_sub(first_line) as u16;
155238
surface.set_style(
156239
Rect::new(

0 commit comments

Comments
 (0)