andfoy / winpty-rs

Create and spawn processes inside a pseudoterminal in Windows from Rust
Other
23 stars 7 forks source link

Fix write corruption caused by writing an unintended NUL byte #16

Closed segevfiner closed 2 years ago

segevfiner commented 2 years ago

Closes spyder-ide/pywinpty#210

codecov-commenter commented 2 years ago

Codecov Report

Merging #16 (7d0ec8c) into main (3b08c65) will decrease coverage by 0.02%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #16      +/-   ##
==========================================
- Coverage   74.50%   74.48%   -0.03%     
==========================================
  Files           8        8              
  Lines        1208     1207       -1     
==========================================
- Hits          900      899       -1     
  Misses        308      308              
Flag Coverage Δ
unittests 74.48% <100.00%> (-0.03%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/pty/base.rs 71.84% <100.00%> (-0.09%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3b08c65...7d0ec8c. Read the comment docs.

andfoy commented 2 years ago

Thanks for your contribution @segevfiner, I have only one question: When writing the PSTR into the file, the NUL character is no longer necessary? Also, is this problem also happening when reading bytes?

segevfiner commented 2 years ago

Yeah. Just the bytes without a NUL byte, as this is a "file" write and you are passing the size of the byte array/string to be written. Same as in a normal file, if you write a NUL byte into it, the NUL byte itself will also appear in the file rather than just the text. I haven't noticed a problem on read, as in, I don't see any extra NUL bytes there.

andfoy commented 2 years ago

Thanks for the fix! I'll publish a new release