Update documentation related to the recent cmd.exe fix #123709
Conversation
Fix some grammar nits, change `bat` (extension) -> `batch` (file), and make line wrapping more consistent.
|
rustbot has assigned @workingjubilee. Use |
rustbot
added
O-windows
|
r? @ChrisDenton |
| /// On Windows, use caution with untrusted inputs. Most applications use the | ||
| /// standard convention for decoding arguments passed to them. These are safe to | ||
| /// use with `arg`. However, some applications such as `cmd.exe` and `.bat` files | ||
| /// use a non-standard way of decoding arguments. They are therefore vulnerable | ||
| /// to malicious input. | ||
| /// | ||
| /// In the case of `cmd.exe` this is especially important because a malicious | ||
| /// argument can potentially run arbitrary shell commands. | ||
| /// | ||
| /// See [Windows argument splitting][windows-args] for more details | ||
| /// or [`raw_arg`] for manually implementing non-standard argument encoding. |
There was a problem hiding this comment.
Neither this nor the linked page really answer "I have untrusted input, how do I know if I can use it with .arg?". I think this could be improved but not really sure how (I didn't change it, just rewrapped).
There was a problem hiding this comment.
Yeah, and that's a tricky one to answer. I guess the short answer is you just have to know and trust the application/script that you're using uses MSVC compatible argument parsing rules. Which, I mean, is annoying. On the other hand sending untrusted arguments to an unknown application sounds wrong to me in any case?
There was a problem hiding this comment.
I can put something like that into words, just explaining how Rust does quoting would help.
We now use the cmd quoting if the executable for Command::new is cmd.exe, *.bat or *.cmd, correct?
There was a problem hiding this comment.
We don't for an explicit call to cmd.exe. We do for *.bat or *.cmd (case insensitive) as that's an implicit call to cmd.exe.
Note that we also don't prevent Linux users from using sh to execute things in the shell so that was not a goal here.
| //! On Unix systems arguments are passed to a new process as an array of strings | ||
| //! but on Windows arguments are passed as a single commandline string and it's | ||
| //! On Unix systems arguments are passed to a new process as an array of strings, | ||
| //! but on Windows arguments are passed as a single commandline string and it is |
There was a problem hiding this comment.
Commander Data, is that you?
There was a problem hiding this comment.
Lol, the grammar lords always said to avoid short contractions in technical writing
| @@ -107,26 +107,26 @@ | |||
| //! * Use [`raw_arg`] to build a custom commandline. This bypasses the escaping | |||
| //! rules used by [`arg`] so should be used with due caution. | |||
| //! | |||
| //! `cmd.exe` and `.bat` use non-standard argument parsing and are especially | |||
| //! `cmd.exe` and `.bat` files use non-standard argument parsing and are especially | |||
There was a problem hiding this comment.
This makes it sound like "cmd.exe files" is a valid decomposition of the sentence's parts.
| /// the standard C run-time escaping rules, such as `cmd.exe /c`. | ||
| /// | ||
| /// # Bat files | ||
| /// # Batch files |
There was a problem hiding this comment.
There are many kinds of batch files used by many different job control languages. I believe only the .bat files of Windows are under discussion here. This retitling seems to be less correct.
There was a problem hiding this comment.
I think the proper name is "DOS Batch file" but figured the DOS part is implied when discussing Windows. I have indeed seen "BAT File" though, just not 🦇
There was a problem hiding this comment.
BAT file seems like the one we might prefer here, yes.
However, I must wonder... do all of these caveats apply to files with the .cmd extension also?
There was a problem hiding this comment.
The documentation so far doesn't seem to mention .cmd at all, despite that also being an applicable extension for Windows batch files with basically-the-same command language. I won't pretend to know what the exact caveats and corner cases are, though.
There was a problem hiding this comment.
However, I must wonder... do all of these caveats apply to files with the
.cmdextension also?
Hey, according to wiki .cmd is just another "Batch File" 😄 https://en.wikipedia.org/wiki/Batch_file
(I'll take another read through and make sure both are covered appropriately)
There was a problem hiding this comment.
Indeed!
I had a frisson of annoyance at that, as technically, there are many JCLs etc. etc. and then I realized that while indeed even in 2024 some people do use mainframes with such batch files that are not in the MS-DOS command language... everyone understands the meaning here from context, obviously.
|
I'll get to reading this tomorrow but in the meantime I'd just like to say I'm grateful for all feedback. This was obviously written in secret with necessarily limited oversight so it didn't get the usual back-and-forth discussion from everyone. |
Fix some grammar nits, change
bat(extension) ->batch(file), and make line wrapping more consistent.