Insufficient test coverage for Stdin::Read on Windows "console mode" #93055

Activity

1. Patrick-Poitras commented on Jan 19, 2022

   

   Patrick-Poitras

   on Jan 19, 2022

   Last edited by Patrick-Poitras

   ContributorAuthor

   More actions

   @rustbot label +A-code-coverage +A-io +O-windows +C-bug +E-needs-test +E-hard +T-libs
2. rustbot

   added

   A-code-coverageArea: Source-based code coverage (-Cinstrument-coverage)Area: Source-based code coverage (-Cinstrument-coverage)

   A-ioArea: `std::io`, `std::fs`, `std::net` and `std::path`Area: `std::io`, `std::fs`, `std::net` and `std::path`

   C-bugCategory: This is a bug.Category: This is a bug.

   E-needs-testCall for participation: An issue has been fixed and does not reproduce, but no test has been added.Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

   O-windowsOperating system: WindowsOperating system: Windows

   on Jan 19, 2022
3. Patrick-Poitras commented on Jan 19, 2022

   

   Patrick-Poitras

   on Jan 19, 2022

   Last edited by Patrick-Poitras

   ContributorAuthor

   More actions

   Relevant code bits: (in library/std/src/sys/windows/stdio.rs)

   fn is_console(handle: c::HANDLE) -> bool {
       // `GetConsoleMode` will return false (0) if this is a pipe (we don't care about the reported
       // mode). This will only detect Windows Console, not other terminals connected to a pipe like
       // MSYS. Which is exactly what we need, as only Windows Console needs a conversion to UTF-16.
       let mut mode = 0;
       unsafe { c::GetConsoleMode(handle, &mut mode) != 0 }
   }

   impl io::Read for Stdin {
       fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
           let handle = get_handle(c::STD_INPUT_HANDLE)?;
           if !is_console(handle) {
               unsafe {
                   let handle = Handle::from_raw_handle(handle);
                   let ret = handle.read(buf);
                   handle.into_raw_handle(); // Don't close the handle
                   return ret;
               }
           }
           // If there are bytes in the incomplete utf-8, start with those.
           // (No-op if there is nothing in the buffer.)
           let mut bytes_copied = self.incomplete_utf8.read(buf);
   
           if bytes_copied == buf.len() {
               return Ok(bytes_copied);
           } else if buf.len() - bytes_copied < 4 {
               // Not enough space to get a UTF-8 byte. We will use the incomplete UTF8.
               let mut utf16_buf = [0u16; 1];
               // Read one u16 character.
               let read = read_u16s_fixup_surrogates(handle, &mut utf16_buf, 1, &mut self.surrogate)?;
               // Read bytes, using the (now-empty) self.incomplete_utf8 as extra space.
               let read_bytes = utf16_to_utf8(&utf16_buf[..read], &mut self.incomplete_utf8.bytes)?;
   
               // Read in the bytes from incomplete_utf8 until the buffer is full.
               self.incomplete_utf8.len = read_bytes as u8;
               // No-op if no bytes.
               bytes_copied += self.incomplete_utf8.read(&mut buf[bytes_copied..]);
               Ok(bytes_copied)
           } else {
               let mut utf16_buf = [0u16; MAX_BUFFER_SIZE / 2];
               // In the worst case, a UTF-8 string can take 3 bytes for every `u16` of a UTF-16. So
               // we can read at most a third of `buf.len()` chars and uphold the guarantee no data gets
               // lost.
               let amount = cmp::min(buf.len() / 3, utf16_buf.len());
               let read =
                   read_u16s_fixup_surrogates(handle, &mut utf16_buf, amount, &mut self.surrogate)?;
   
               match utf16_to_utf8(&utf16_buf[..read], buf) {
                   Ok(value) => return Ok(bytes_copied + value),
                   Err(e) => return Err(e),
               }
           }
       }
   }
4. rustbot

   added

   E-hardCall for participation: Hard difficulty. Experience needed to fix: A lot.Call for participation: Hard difficulty. Experience needed to fix: A lot.

   T-libsRelevant to the library team, which will review and decide on the PR/issue.Relevant to the library team, which will review and decide on the PR/issue.

   on Jan 19, 2022
5. ChrisDenton commented on Jul 2, 2023

   

   ChrisDenton

   on Jul 2, 2023

   Last edited by ChrisDenton

   Member

   More actions

   The way forward here would likely be to create a pseudoconsole. Because this involves spawning a process, this will probably need to be a UI test (so in tests/ui/std or maybe tests/ui/process).