c754df1 · helix · git.iko.soy

git.iko.soy/helix

on master help sign in

lsp: Check bounds when converting lsp positions (#204)

c754df1Wojciech Kępka5 years ago

helix-lsp/src/lib.rs+79−7

compare with master

unifiedsplit

@@ -55,23 +55,54 @@ pub mod util {

55 use super::*;

56 use helix_core::{Range, Rope, Transaction};

58+ /// Converts [`lsp::Position`] to a position in the document.

59+ ///

60+ /// Returns `None` if position exceeds document length or an operation overflows.

58 pub fn lsp_pos_to_pos(

61 pub fn lsp_pos_to_pos(

59 doc: &Rope,

62 doc: &Rope,

60 pos: lsp::Position,

63 pos: lsp::Position,

61 offset_encoding: OffsetEncoding,

64 offset_encoding: OffsetEncoding,

62− ) -> usize {

65+ ) -> Option<usize> {

66+ let max_line = doc.lines().count().saturating_sub(1);

67+ let pos_line = pos.line as usize;

68+ let pos_line = if pos_line > max_line {

69+ return None;

70+ } else {

71+ pos_line

72+ };

63 match offset_encoding {

73 match offset_encoding {

64 OffsetEncoding::Utf8 => {

74 OffsetEncoding::Utf8 => {

75+ let max_char = doc

65− let line = doc.line_to_char(pos.line as usize);

76+ .line_to_char(max_line)

77+ .checked_add(doc.line(max_line).len_chars())?;

78+ let line = doc.line_to_char(pos_line);

66− line + pos.character as usize

79+ let pos = line.checked_add(pos.character as usize)?;

80+ if pos <= max_char {

81+ Some(pos)

82+ } else {

83+ None

84+ }

67 }

85 }

68 OffsetEncoding::Utf16 => {

86 OffsetEncoding::Utf16 => {

87+ let max_char = doc

69− let line = doc.line_to_char(pos.line as usize);

88+ .line_to_char(max_line)

89+ .checked_add(doc.line(max_line).len_chars())?;

90+ let max_cu = doc.char_to_utf16_cu(max_char);

91+ let line = doc.line_to_char(pos_line);

70 let line_start = doc.char_to_utf16_cu(line);

92 let line_start = doc.char_to_utf16_cu(line);

71− doc.utf16_cu_to_char(line_start + pos.character as usize)

93+ let pos = line_start.checked_add(pos.character as usize)?;

94+ if pos <= max_cu {

95+ Some(doc.utf16_cu_to_char(pos))

96+ } else {

97+ None

98+ }

72 }

99 }

73 }

100 }

74 }

101 }

102+

103+ /// Converts position in the document to [`lsp::Position`].

104+ ///

105+ /// Panics when `pos` is out of `doc` bounds or operation overflows.

75 pub fn pos_to_lsp_pos(

106 pub fn pos_to_lsp_pos(

76 doc: &Rope,

107 doc: &Rope,

77 pos: usize,

108 pos: usize,

@@ -95,6 +126,7 @@ pub mod util {

95 }

126 }

96 }

127 }

128

129+ /// Converts a range in the document to [`lsp::Range`].

98 pub fn range_to_lsp_range(

130 pub fn range_to_lsp_range(

99 doc: &Rope,

131 doc: &Rope,

100 range: Range,

132 range: Range,

@@ -121,8 +153,17 @@ pub mod util {

121 None

153 None

122 };

154 };

123

155

156+ let start =

124− let start = lsp_pos_to_pos(doc, edit.range.start, offset_encoding);

157+ if let Some(start) = lsp_pos_to_pos(doc, edit.range.start, offset_encoding) {

158+ start

159+ } else {

160+ return (0, 0, None);

161+ };

125− let end = lsp_pos_to_pos(doc, edit.range.end, offset_encoding);

162+ let end = if let Some(end) = lsp_pos_to_pos(doc, edit.range.end, offset_encoding) {

163+ end

164+ } else {

165+ return (0, 0, None);

166+ };

126 (start, end, replacement)

167 (start, end, replacement)

127 }),

168 }),

128 )

169 )

@@ -248,3 +289,34 @@ impl Registry {

248// there needs to be a way to process incoming lsp messages from all clients.

289// there needs to be a way to process incoming lsp messages from all clients.

249// -> notifications need to be dispatched to wherever

290// -> notifications need to be dispatched to wherever

250// -> requests need to generate a reply and travel back to the same lsp!

291// -> requests need to generate a reply and travel back to the same lsp!

292+

293+#[cfg(test)]

294+mod tests {

295+ use super::{lsp, util::*, OffsetEncoding};

296+ use helix_core::Rope;

297+

298+ #[test]

299+ fn converts_lsp_pos_to_pos() {

300+ macro_rules! test_case {

301+ ($doc:expr, ($x:expr, $y:expr) => $want:expr) => {

302+ let doc = Rope::from($doc);

303+ let pos = lsp::Position::new($x, $y);

304+ assert_eq!($want, lsp_pos_to_pos(&doc, pos, OffsetEncoding::Utf16));

305+ assert_eq!($want, lsp_pos_to_pos(&doc, pos, OffsetEncoding::Utf8))

306+ };

307+ }

308+

309+ test_case!("", (0, 0) => Some(0));

310+ test_case!("", (0, 1) => None);

311+ test_case!("", (1, 0) => None);

312+ test_case!("\n\n", (0, 0) => Some(0));

313+ test_case!("\n\n", (1, 0) => Some(1));

314+ test_case!("\n\n", (1, 1) => Some(2));

315+ test_case!("\n\n", (2, 0) => Some(2));

316+ test_case!("\n\n", (3, 0) => None);

317+ test_case!("test\n\n\n\ncase", (4, 3) => Some(11));

318+ test_case!("test\n\n\n\ncase", (4, 4) => Some(12));

319+ test_case!("test\n\n\n\ncase", (4, 5) => None);

320+ test_case!("", (u32::MAX, u32::MAX) => None);

321+ }

322+}

helix-term/src/application.rs+18−7

@@ -178,7 +178,7 @@ impl Application {

178 let diagnostics = params

179 .diagnostics

180 .into_iter()

181− .map(|diagnostic| {

181+ .filter_map(|diagnostic| {

182 use helix_core::{

183 diagnostic::{Range, Severity, Severity::*},

184 Diagnostic,

@@ -189,18 +189,29 @@ impl Application {

189 let language_server = doc.language_server().unwrap();

190

191 // TODO: convert inside server

192− let start = lsp_pos_to_pos(

192+ let start = if let Some(start) = lsp_pos_to_pos(

193 text,

194 diagnostic.range.start,

195 language_server.offset_encoding(),

196− );

196+ ) {

197+ start

198+ } else {

199+ log::warn!("lsp position out of bounds - {:?}", diagnostic);

200+ return None;

201+ };

202+

197− let end = lsp_pos_to_pos(

203+ let end = if let Some(end) = lsp_pos_to_pos(

198 text,

204 text,

199 diagnostic.range.end,

205 diagnostic.range.end,

200 language_server.offset_encoding(),

206 language_server.offset_encoding(),

201− );

207+ ) {

208+ end

209+ } else {

210+ log::warn!("lsp position out of bounds - {:?}", diagnostic);

211+ return None;

212+ };

202

213

203− Diagnostic {

214+ Some(Diagnostic {

204 range: Range { start, end },

215 range: Range { start, end },

205 line: diagnostic.range.start.line as usize,

216 line: diagnostic.range.start.line as usize,

206 message: diagnostic.message,

217 message: diagnostic.message,

@@ -214,7 +225,7 @@ impl Application {

214 ),

225 ),

215 // code

226 // code

216 // source

227 // source

217− }

228+ })

218 })

229 })

219 .collect();

230 .collect();

220

231

helix-term/src/commands.rs+7−6

@@ -1407,7 +1407,12 @@ fn _goto(

1407 let (view, doc) = editor.current();

1408 let definition_pos = location.range.start;

1409 // TODO: convert inside server

1410+ let new_pos =

1410− let new_pos = lsp_pos_to_pos(doc.text(), definition_pos, offset_encoding);

1411+ if let Some(new_pos) = lsp_pos_to_pos(doc.text(), definition_pos, offset_encoding) {

1412+ new_pos

1413+ } else {

1414+ return;

1415+ };

1411 doc.set_selection(view.id, Selection::point(new_pos));

1416 doc.set_selection(view.id, Selection::point(new_pos));

1412 align_view(doc, view, Align::Center);

1417 align_view(doc, view, Align::Center);

1413 }

1418 }

@@ -2297,11 +2302,7 @@ pub fn completion(cx: &mut Context) {

2297

2302

2298 let offset_encoding = language_server.offset_encoding();

2303 let offset_encoding = language_server.offset_encoding();

2299

2304

2300− let pos = pos_to_lsp_pos(

2301− doc.text(),

2302− doc.selection(view.id).cursor(),

2305+ let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding);

2303− language_server.offset_encoding(),

2304− );

2305

2306

2306 // TODO: handle fails

2307 // TODO: handle fails

2307 let future = language_server.completion(doc.identifier(), pos);

2308 let future = language_server.completion(doc.identifier(), pos);