Reject non-char-boundary ranges in Literal::subspan - #156302
Conversation
rustbot
added
S-waiting-on-review
|
r? @wesleywiser rustbot has assigned @wesleywiser. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| let TokenTree::Literal(lit) = input.into_iter().next().unwrap() else { | ||
| panic!("expected a string literal"); | ||
| }; | ||
| let bad = lit.subspan(2..3).expect("subspan"); |
There was a problem hiding this comment.
Why does the .subspan() not return an error when it isn't on a character boundary? A span which doesn't start and end at a character boundary is nonsensical IMHO. Returning an error here would mean that code dealing with spans never has to deal with such malformed spans. I'm pretty sure start_point is not the only method that crashes when a span doesn't start and end at a character boundary.
There was a problem hiding this comment.
Let me look into that.
There was a problem hiding this comment.
Fixed on the proc_macro side. Reverted the previous change since it wasn't a targeted fix.
el-ev
changed the title
SourceMap::start_point with non-char-boundary spansLiteral::subspan
|
Ping. |
| Ok(src.is_char_boundary(span_start + start) | ||
| && src.is_char_boundary(span_start + end)) |
There was a problem hiding this comment.
Maybe this is a bit nicer?
| Ok(src.is_char_boundary(span_start + start) | |
| && src.is_char_boundary(span_start + end)) | |
| Ok(src.get(span_start + start..span_start + end).is_some()) |
| Ok(src.is_char_boundary(span_start + start) | ||
| && src.is_char_boundary(span_start + end)) | ||
| }) | ||
| .unwrap_or(true); |
There was a problem hiding this comment.
I think this can result in invalid spans getting embedded in the crate metadata if the source is not available while compiling the current crate, and if the source is then available when compiling a dependent crate that could result in a crash again. The SourceFile contains the locations of multi byte characters in the multibyte_chars field even if the source is unavailable, but there doesn't seem to be any api exposed to use this for span offset validation.
Enselic
added
S-waiting-on-author
5 participants
Resolves #156049.
Prevent creation of subspans whose start or end falls inside a UTF-8 character in the source.
Before:
After: (doesn't ICE)