Skip to content

Commit

Permalink
Rollup merge of rust-lang#65237 - KodrAus:fix/map-entry-err, r=sfackler
Browse files Browse the repository at this point in the history
Move debug_map assertions after check for err

Fixes rust-lang#65231

We have some assertions in `DebugMap` to catch broken implementations of `Debug` that produce malformed entries. These checks don't make sense if formatting fails partway through. This PR moves those assertions to within the `and_then` closures along with the other formatting logic, so they're only checked if the map hasn't failed to format an entry already.
  • Loading branch information
Centril authored Oct 17, 2019
2 parents a16dca3 + 959a6c1 commit 0059411
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 7 deletions.
16 changes: 9 additions & 7 deletions src/libcore/fmt/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -775,10 +775,10 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
reason = "recently added",
issue = "62482")]
pub fn key(&mut self, key: &dyn fmt::Debug) -> &mut DebugMap<'a, 'b> {
assert!(!self.has_key, "attempted to begin a new map entry \
without completing the previous one");

self.result = self.result.and_then(|_| {
assert!(!self.has_key, "attempted to begin a new map entry \
without completing the previous one");

if self.is_pretty() {
if !self.has_fields {
self.fmt.write_str("\n")?;
Expand Down Expand Up @@ -839,9 +839,9 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
reason = "recently added",
issue = "62482")]
pub fn value(&mut self, value: &dyn fmt::Debug) -> &mut DebugMap<'a, 'b> {
assert!(self.has_key, "attempted to format a map value before its key");

self.result = self.result.and_then(|_| {
assert!(self.has_key, "attempted to format a map value before its key");

if self.is_pretty() {
let mut slot = None;
let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot, &mut self.state);
Expand Down Expand Up @@ -924,9 +924,11 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
/// ```
#[stable(feature = "debug_builders", since = "1.2.0")]
pub fn finish(&mut self) -> fmt::Result {
assert!(!self.has_key, "attempted to finish a map with a partial entry");
self.result.and_then(|_| {
assert!(!self.has_key, "attempted to finish a map with a partial entry");

self.result.and_then(|_| self.fmt.write_str("}"))
self.fmt.write_str("}")
})
}

fn is_pretty(&self) -> bool {
Expand Down
40 changes: 40 additions & 0 deletions src/libcore/tests/fmt/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,46 @@ mod debug_map {
format!("{:#?}", Bar));
}

#[test]
fn test_entry_err() {
// Ensure errors in a map entry don't trigger panics (#65231)
use std::fmt::Write;

struct ErrorFmt;

impl fmt::Debug for ErrorFmt {
fn fmt(&self, _: &mut fmt::Formatter<'_>) -> fmt::Result {
Err(fmt::Error)
}
}

struct KeyValue<K, V>(usize, K, V);

impl<K, V> fmt::Debug for KeyValue<K, V>
where
K: fmt::Debug,
V: fmt::Debug,
{
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut map = fmt.debug_map();

for _ in 0..self.0 {
map.entry(&self.1, &self.2);
}

map.finish()
}
}

let mut buf = String::new();

assert!(write!(&mut buf, "{:?}", KeyValue(1, ErrorFmt, "bar")).is_err());
assert!(write!(&mut buf, "{:?}", KeyValue(1, "foo", ErrorFmt)).is_err());

assert!(write!(&mut buf, "{:?}", KeyValue(2, ErrorFmt, "bar")).is_err());
assert!(write!(&mut buf, "{:?}", KeyValue(2, "foo", ErrorFmt)).is_err());
}

#[test]
#[should_panic]
fn test_invalid_key_when_entry_is_incomplete() {
Expand Down

0 comments on commit 0059411

Please sign in to comment.