Skip to content

Format string parser does not fail on invalid input #71088

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

Closed
strega-nil opened this issue Apr 13, 2020 · 2 comments · Fixed by #96568
Closed

Format string parser does not fail on invalid input #71088

strega-nil opened this issue Apr 13, 2020 · 2 comments · Fixed by #96568
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@strega-nil
Copy link
Contributor

This code:

struct Foo;

impl Display for Foo {
  fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
    write!(f, "{}", f.fill() as u32)
  }
}

fn main() {
  println!("{:\t}", Foo)
}

Compiles, and prints 32. According to the std::fmt docs,

[[fill]align][sign]['#']['0'][width]['.' precision][type]

is the syntax of format specifications; however, \t does not fulfill any of these elements. I assumed that it might accidentally be implementing the syntax

[fill][align][sign]['#']['0'][width]['.' precision][type]

however, if that were true, then this should print 9 (and not 32). I haven't looked into the parsing code, but will after filing this issue.

@strega-nil strega-nil added the C-bug Category: This is a bug. label Apr 13, 2020
@sfackler
Copy link
Member

This is just because the fmt parser ignores whitespace I believe: https://github.com/rust-lang/rust/blob/master/src/libfmt_macros/lib.rs#L397-L406.

@strega-nil
Copy link
Contributor Author

strega-nil commented Apr 13, 2020

It turns out, it's implementing the syntax

[[fill]align][sign]['#']['0'][width]['.' precision][type]ws*

due to a bug in this commit from 7 years ago: a447c3c

@sfackler whitespace is only ignored before the final }, and this doesn't do anything, so I would say it's at least a docs bug; it should also probably warn, as this should never be intentional.

@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 15, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this issue May 2, 2022
…triplett

std::fmt: Various fixes and improvements to documentation

This PR contains the following changes:

- **Added argument index comments to examples for specifying precision**

  The examples for specifying the precision have comments explaining which
  argument the specifier is referring to. However, for implicit positional
  arguments, the examples simply refer to "next arg". To simplify following the
  comments, "next arg" was supplemented with the actual resulting argument index.

- **Fixed documentation for specifying precision via `.*`**

  The documentation stated that in case of the syntax `{<arg>:<spec>.*}`, "the
  `<arg>` part refers to the value to print, and the precision must come in the
  input preceding `<arg>`". This is not correct: the <arg> part does indeed refer
  to the value to print, but the precision does not come in the input preciding
  arg, but in the next implicit input (as if specified with {}).

  Fixes rust-lang#96413.

- **Fix the grammar documentation**

  According to the grammar documented, the format specifier `{: }` should not be
  legal because of the whitespace it contains. However, in reality, this is
  perfectly fine because the actual implementation allows spaces before the
  closing brace. Fixes rust-lang#71088.

  Also, the exact meaning of most of the terminal symbols was not specified, for
  example the meaning of `identifier`.

- **Removed reference to Formatter::buf and other private fields**

  Formatter::buf is not a public field and therefore isn't very helpful in user-
  facing documentation. Also, the other public fields of Formatter were removed
  during stabilization of std::fmt (4af3494) and can only be accessed via
  getters.

- **Improved list of formatting macros**

  Two improvements:
  1. write! can not only receive a `io::Write`, but also a `fmt::Write` as first argument.
  2. The description texts now contain links to the actual macros for easier
     navigation.
@bors bors closed this as completed in 79d9afd May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
3 participants