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: float scanning can consume more than required and generate incorrect error #23506

Closed
omeid opened this issue Jan 22, 2018 · 11 comments
Closed

fmt: float scanning can consume more than required and generate incorrect error #23506

omeid opened this issue Jan 22, 2018 · 11 comments
Assignees
Milestone

Comments

@omeid
Copy link

@omeid omeid commented Jan 22, 2018

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

Does not apply.

Does this issue reproduce with the latest release?

Does not apply.

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

Does not apply.

What did you do?

There does not seem to be a way to scan a float that is followed by a literal char p.
eg. 7.23p to mean float value 7.23 followed by char p.

What did you expect to see?

It should be possible to scan a float value that is followed by a literal char p which is not part of float (i.e does not indicate a binary exponent).

What did you see instead?

val := 0.0
_, err := fmt.Sscanf("7.23p", "%fp", &val)
if err != nil {
	fmt.Println(err) // strconv.Atoi: parsing "7.23p": invalid syntax
}

Binary-exponent floating format support was added to Go in 029bbe1 but it is not clear whatever it was intented to be only for %b or %f as well.

Binary-exponent floating format support in %f doesn't seem to be documented or have any escaping mechanism.

@ALTree ALTree changed the title fmt.Scan: No way to escape binary exponent for floats fmt: no way to escape binary exponent for floats in Scan Jan 22, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Jan 23, 2018
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 23, 2018

@robpike
Copy link
Contributor

@robpike robpike commented Jan 23, 2018

I don't believe there is a clean way to fix this, and I'm not sure but I think that C's scanf, which is the unfortunate predecessor of this unfortunate facility in Go, offers no solution either.

Absent evidence otherwise, I would close this issue as "unfortunate". Scanf and friends are not good general-purpose parsers, and will never be so.

@omeid
Copy link
Author

@omeid omeid commented Jan 23, 2018

If I understand correctly, C's scanf (as implemented by strtof) does not support binary exponents format for decimal floating-point expressions.

The following C code works as expected.

float f;
sscanf("7.23p", "%fp", &f);
printf("%f\n", f); // 7.230000
@robpike
Copy link
Contributor

@robpike robpike commented Jan 24, 2018

If you try to scan "123e" into a float, that also errors. It's not the existence of binary exponents that's the issue, it's a general problem with floats that does not occur with integers: https://play.golang.org/p/gqzbRPWPebU and https://play.golang.org/p/01gHSOypDwT

Marking this as a bug, changing the title, and setting it for go1.11. Still not sure I can find a clean fix, but it might be possible. The inconsistency should be resolved.

@robpike robpike changed the title fmt: no way to escape binary exponent for floats in Scan fmt: float scanning can consume more than required and generate incorrect error Jan 24, 2018
@robpike robpike self-assigned this Jan 24, 2018
@robpike robpike removed the NeedsDecision label Jan 24, 2018
@robpike robpike modified the milestones: Unplanned, Go1.11 Jan 24, 2018
@robpike
Copy link
Contributor

@robpike robpike commented Jan 25, 2018

Copying a comment @rsc made on the wrong issue:

FWIW, at least on my Mac, sscanf doesn't care how long an integer is, even if it's too long:

#include <stdio.h>

int main() {
int d;
d = 0;
printf("%d\n", sscanf("11111111111111111e", "%de", &d));
printf("%d\n", d);
return 0;
}

prints

1
651588039
@omeid
Copy link
Author

@omeid omeid commented Jan 25, 2018

I see the same results with GCC on Linux.

I believe the reason that happens is because sscanf, for whatever reason, uses strtol for scanning int, and as strtol returns a long it does bound checks accordingly, so the overflow happens when the long returned by strtol is assigned to an int.

So, if one uses long instead of int for d, and gives a large enough value, it errs as expected:

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>

int main() {

  long d = 0;
  printf("sscanf: %d\n", sscanf("11111111111111111111111111e", "%lde", &d));
  printf("d:      %ld\n", d);
  printf("err:    %s\n", strerror(errno));

}
sscanf: 1
d:      9223372036854775807
err:    Numerical result out of range

This is all very unholy. No int with scanf.

@robpike
Copy link
Contributor

@robpike robpike commented Mar 19, 2018

To fix this would require three-rune lookahead, which is an unbearable burden. You could get "3e+x" and need to read all the way to the "x" to learn that you should stop scanning at the 3 for a legal float.

This is not practical given the way Scanf is meant to work with an arbitrary input feed. One rune lookahead is fine, but three runes is too hard.

Closing as unfortunate.

@robpike robpike closed this Mar 19, 2018
@omeid
Copy link
Author

@omeid omeid commented Mar 19, 2018

Fair enough, some documentation of this limitation would be good still, at least there should be a mention of how p is supported. It could save some other unfortunate soul some frustration.

@robpike
Copy link
Contributor

@robpike robpike commented Mar 19, 2018

It's got nothing to do with 'p' beyond 'p' being an exponent marker for floats.

@omeid
Copy link
Author

@omeid omeid commented Mar 19, 2018

The fmt package does not mention anything about the binary-exponent (p) in floating-points. Only e and E.

@robpike
Copy link
Contributor

@robpike robpike commented Mar 19, 2018

@golang golang locked and limited conversation to collaborators Mar 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.