Skip to content
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

fmt: "%%" placeholder accepts incorrect index values #22868

Open
octomarat opened this issue Nov 24, 2017 · 5 comments
Open

fmt: "%%" placeholder accepts incorrect index values #22868

octomarat opened this issue Nov 24, 2017 · 5 comments
Labels
Milestone

Comments

@octomarat
Copy link

@octomarat octomarat commented Nov 24, 2017

What version of Go are you using (go version)?

go1.9.2

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

macOS Sierra 10.12.6, darwin/amd64

What did you do?

Run the following code

package main

import (
	"fmt"
)

func main() {
	fmt.Printf("%[100]%")
	fmt.Printf("%[-100]%")
	fmt.Printf("%[not a mumber]%")
}

What did you expect to see?

In all cases, errors about incorrect index is printed.

What did you see instead?

In all cases, '%' is printed. This behaviour is unexpected and, at least, not documented here:
https://golang.org/pkg/fmt/

@martisch
Copy link
Contributor

@martisch martisch commented Nov 24, 2017

Since documentation says "%% a literal percent sign; consumes no value" the index for the non consumed value is not checked. This could be explicity documented or easily changed by changing the order in which this switch statement in fmt is evaluated:

case verb == '%': // Percent does not absorb operands and ignores f.wid and f.prec.

However changing this is a backwards incompatible change that can break code (even if unlikely case) that worked fine before.

@octomarat
Copy link
Author

@octomarat octomarat commented Nov 24, 2017

Speaking about 'consumes no value'.

	fmt.Printf("%% %d \n", 1) // consumes no value, '% 1' is printed

	fmt.Printf("%*% %d \n", 1, 2) // consumes '1' for width, '% 2' is printed
	fmt.Printf("%[100]*% %d \n", 1, 2) // still consumes '1' for width, '% 2' is printed
	fmt.Printf("%[a]*% %d \n", 1, 2) // still consumes '1' for width, '% 2' is printed

So if we use index for width or precision, value is consumed and still index is not checked.

@robpike
Copy link
Contributor

@robpike robpike commented Nov 24, 2017

Your examples all look like they are behaving correctly to me.

One could argue that [100] should give an error, but since it fetches no argument it's also fine to say it's a no-op.

@octomarat
Copy link
Author

@octomarat octomarat commented Nov 24, 2017

And what about indices for width and precision? Value is consumed, so this is not a no-op.

I found another interesting case connected to width and precision:

fmt.Printf("%[1]*% %d \n", 1, 2) // consumes values 1 and 2, prints '% 2'
fmt.Printf("%[2]*% %d \n", 1, 2) // reports error as for '%d' we have no value
fmt.Printf("%[3]*% %d \n", 1, 2) // consumes values 1 and 2, prints '% 2'
fmt.Printf("%[4]*% %d \n", 1, 2) // consumes values 1 and 2, prints '% 2'
fmt.Printf("%[5]*% %d \n", 1, 2) // consumes values 1 and 2, prints '% 2'
// ... and so on

So if width/precision index is correct, %% behaves like any other verb and consumes corresponding value. If index is incorrect (not a number, < 1, or > arguments number), it consumes the first value.

@robpike
Copy link
Contributor

@robpike robpike commented Nov 24, 2017

OK, that does look like a bug, although it's hard to get excited about it. %[1]*% is a pointless thing to do.

@robpike robpike added this to the Unreleased milestone Nov 24, 2017
@ALTree ALTree changed the title fmt.Printf: "%%" placeholder accepts incorrect index values fmt: "%%" placeholder accepts incorrect index values Jan 22, 2018
@ALTree ALTree added the NeedsFix label Mar 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.