Fix for issue #157 - go-yaml should handle invalid integers as strings #223

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

vinzenz commented Nov 3, 2016

No description provided.

dsnet and others added some commits Nov 1, 2016

Fix decode test for Go 1.8
An upstream change (http://golang.org/cl/31144) alters the time package
to be more consistent about the representation of UTC location.
We adjust the time.Time used in the test to work on both 1.7 and 1.8.
Contributor

niemeyer commented Jan 24, 2017

It's not clear to me how this is fixing the referenced issue in a proper way. What's the actual specification of the float format in yaml, and how is this handling it? Where are the tests for it showing details of the problem being fixed?

vinzenz commented Jan 25, 2017

It's not clear to me how this is fixing the referenced issue in a proper way.

ParseFloat is behaving oddly by discarding the leading 0 and make the rest of the number a float.

What's the actual specification of the float format in yaml, and how is this handling it?

http://yaml.org/spec/1.2/spec.html#tag/repository/float
A leading 0 is only allowed when followed by a dot, which it checks for and then passes the string on to really try a float. (tryFloat)

Where are the tests for it showing details of the problem being fixed?

I will add some test that would fail before

Contributor

niemeyer commented Jan 25, 2017

Note that this changed from yaml 1.1 to 1.2. It might be okay, but we may also end up breaking existing code. In Python:

>>> import yaml
>>> yaml.load("01.1")
1.1

Thanks for the tests.

Contributor

niemeyer commented Jan 25, 2017

Curiously, Python float resolution seems broken in other ways:

>>> yaml.load("-2E+05")
'-2E+05'

vinzenz commented Jan 25, 2017

#171 seems to actually fix this already

vinzenz commented Jan 25, 2017

And we can also add this as a parser option (Strict floats) like it was introduced as another PR

Contributor

niemeyer commented Jan 25, 2017

Indeed, merged #171. Thanks for your help on this.

@niemeyer niemeyer closed this Jan 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment