-
-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
msys check fails with long paths #34
Comments
Can you show a backtrace? I can't see where the panic originates. |
My apologies. I'm not sure any more if this will apply to the atty crate. I'm using the deprecated isatty crate instead (code is part of a larger code base, so I can't change this) and I didn't realize that is a truly different crate that is not deprecated (and I assume this is the replacement). The code is similar, but not exactly the same, as this crate does not access the slice, but does raw pointer access. That said, it seems the from_raw_parts may UB due to reading past the end of the MAX_PATH allocated Here is the backtrace from the
The slog-term bug is slog-rs/slog#214 Feel free to close if you don't consider this applicable. |
Yeah, that's exactly what I was thinking, in that it would be UB and not a panic, which is worse. I'd consider this a bug in this crate, definitely. |
I think atty is safe because it handles wchars properly, whereas isatty assumes chars. When atty hits the windows call, it has allocated a struct of size 8 + 2*260 = 528 bytes, whereas isatty has allocated 268. When GetFileInformationByHandleEx encounters a path > what would fit in the buffer, it returns error 234 (more data available) and we hit the early return. when the path is exactly 262 wchars (524 bytes): for isatty, the same early return happens at 133 wchars and onwards Ref: I'm going to consider this closed, as slog-term needs to use this crate. |
if you want to play with this - https://gist.github.com/nikhilm/34d156f58ea353ed3275b434dcaf5808 |
Ah okay, great, thanks for the analysis! |
atty/src/lib.rs
Line 118 in c92ca03
Windows 10 supports longer paths than MAXPATH. When stdout is attached to such a file, the above code panics because it tries to index beyond MAXPATH + 8.
I think this code should be modified to allocate up to 32,767 plus some buffer (see the Note at https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file).
The text was updated successfully, but these errors were encountered: