Escape quotes in the filename within the location pragmas #16549
Conversation
Sija
force-pushed
the
fix-12928
branch
2 times, most recently
from
ca0a678 to
a01cf06
Compare
|
I've added a suggestion from #12928 (comment) and filename validation to ensure there ain't anything funky going on, since now when the filename is treated as a regular string, it creates a new abuse vector. |
| private def valid_filename?(filename) | ||
| filename | ||
| .each_char | ||
| .none?(&.in?( | ||
| '\0', '\a', '\b', '\n', '\r', '\t', '\v', '\f', '\e', | ||
| *UNICODE_BIDI_CONTROL_CHARACTERS, | ||
| )) | ||
| end |
There was a problem hiding this comment.
question: Do we really need this validation? I would expect any invalid character leads to a decent error message already?
FWIW, these characters could already be used in their literal (non-escaped) form inside a loc pragma.
There was a problem hiding this comment.
question: Do we really need this validation? I would expect any invalid character leads to a decent error message already?
Yes, we do; otherwise, these characters might become part of the filename string - as they're not invalid (from the string PoV), just unwanted in this particular context.
FWIW, these characters could already be used in their literal (non-escaped) form inside a loc pragma.
Some of them, yes (which depend on the parsing logic), but not anymore.
There was a problem hiding this comment.
Yeah, these characters may be unwanted yet valid. But do we need to ensure that? What kind of bad things could happen if we leave this validation out?
| private def append_loc(str, filename, line_number, column_number) | ||
| str << %(#<loc:") | ||
| str << filename | ||
| str << escape_filename(filename) |
There was a problem hiding this comment.
suggestion: We should be able to use String#inspect here. It produces a properly escaped string literal.
| str << escape_filename(filename) | |
| filename.inspect(str) |
2 participants
Resolves #12928