Ignore malformed environment variables on Windows too #29901
Conversation
|
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
| }; | ||
| Some((OsStringExt::from_wide(k), OsStringExt::from_wide(v))) | ||
| } | ||
| if s.is_empty() { |
There was a problem hiding this comment.
This check doesn't need to be here as it was covered above via if *self.cur == 0, that's what indicates the end of the environment variable block.
|
I'm a little confused after some investigation here. According to Windows the name of a variable cannot contain an equal sign but in practice it looks like the first character can be an equal sign but no others can be. That would explain what's going on here in MinGW because the actual variable name is I think that this may want to update the parsing to account for that (e.g. allow a leading |
|
I was under the impression that I read about that leading equals sign from official documentation of Microsoft, but I can't find it anymore. :/ |
|
@alexcrichton What should we do with malformed strings? There's nothing meaningful we can output. Upon encountering a string like |
|
Also, I think my parsing already treats a leading |
|
I'm not really sure what the best strategy here is, I don't know what the system is actually generating. For example I'm not sure if it's even possible to see a string like |
|
So what do you suggest to do in the "missing equals sign" case? The previous code special-cased it to put the whole thing into the key. My code special-cases it to skip the string. |
|
I guess ignoring it isn't so bad yeah |
|
@alexcrichton Addressed your comments. |
| self.cur = self.cur.offset(len + 1); | ||
|
|
||
| // `s` has at least length 1 at this point, because the empty | ||
| // string terminates the array of environment variables. |
There was a problem hiding this comment.
Can you update this comment to mention that the purpose of this slice-after-one is not because we know it has length one but because we want to skip any leading = character?
|
I think this can re-un-ignore the test in run-pass as well, right? |
|
Done. |
|
@bors: r+ e1740221461089106612fd45ffad80f8805abba6 |
|
⌛ Testing commit e174022 with merge 4ef5e2a... |
|
💔 Test failed - auto-linux-64-nopt-t |
Leading equals symbols are treated as part of the variable name, if there is no other equality symbol or none at all, the environment string is ignored.
tbu-
force-pushed
the
pr_env_ignore_malformed_windows
branch
from
e174022 to
9b4f16b
Compare
|
Fixed the error, compiled it locally (didn't know you could uncover compilation errors in a Windows module on Linux), squashed the commits. This might take a few test cycles to complete, I don't have a Windows machine where I can test the build. |
See also #29297.