Allow colon in a plain scalar in a flow context by dbeer1 · Pull Request #45 · yaml/pyyaml

dbeer1 · 2016-12-07T19:54:48Z

This pull is analogous to yaml/libyaml#28. It changes the behavior of pyyaml the same way the other pull changes the behavior of libyaml.

sigmavirus24 · 2016-12-07T22:13:54Z

Hey @dbeer1!

Refresh my memory, is this valid for scalars even in 1.1 or just 1.2? At the moment, pyyaml and libyaml only support 1.1.

sigmavirus24 · 2016-12-07T22:14:47Z

tests/data/spec-08-13.canonical

 ---
 !!map {
-  ? !!str "foo"
+  ? !!str "foo :"


Did this fail prior to this change?

It passed, but the key was parsed as "foo". Note that pyyaml and libyaml behave differently in this edge case, libyaml rejects any colon in a flow context without a space afterwards, while pyyaml allows ',[]{}' after a colon as well.

Compare https://github.com/yaml/pyyaml/blob/master/lib/yaml/scanner.py#L1301 vs https://github.com/yaml/libyaml/blob/master/src/scanner.c#L3438

I reverted this test and behavior with regard to empty values in flow mappings in the latest commit, fyi.

adrienverge · 2016-12-09T10:27:04Z

I just check in the specs, and it appears to be the same for 1.1 and 1.2. From 1.1:

Plain scalars can never contain any tab characters. They also must not contain the “: ” and “ #” character sequences as these combinations cause ambiguity with key: value pairs and comments. Inside flow collections, plain scalars are further restricted to avoid containing the “[”, “]”, “{”, “}” and “,” characters as these would cause ambiguity with the flow collection structure (hence the need for the flow-in context and the flow-out context).

So ": " (notice the space) is forbidden, but ":" isn't.

Related issue: adrienverge/yamllint#30

adrienverge · 2017-02-06T13:34:06Z

Hi @sigmavirus24, do you have any update on this?

ingydotnet · 2017-02-08T19:42:08Z

Kirill strayed from the spec a few times when he thought it was really not the right thing to do. I suspect this might be the case. {foo:bar} is pretty ambiguous. Even {url:http://foo.io} is weird looking at best. However [ http://127.0.0.1:1234, http://127.0.0.1:2345 ] is not.

I just tested this using the new YAML Editor: http://pasteboard.co/vYxs4oX8D.png

Seems like most frameworks accept it, except the libyaml ones. Since it is in the spec and EveryonesDoingIt™, I move to add this to libyaml and all its bindings.

sigmavirus24 · 2017-02-08T19:51:07Z

Thanks for confirming that detail @ingydotnet

dbeer1 · 2017-02-08T20:21:50Z

Thanks! Note that the C binding pull request (yaml/libyaml#28) needs to be updated to follow the modified logic here with regard to a missing value after a colon, eg {a:} -> { "a" : null } instead of {a:} -> { "a:" : null }

bobvandevijver · 2017-02-27T17:47:50Z

@sigmavirus24 Can a new release be drafted with this change? I'm waiting for it so my I can update my docker containers without moving from the simple pip install command 😃

tobias-93 · 2017-07-26T20:54:17Z

@sigmavirus24 Would it be possible to make a new release with these changes? Thank you!

asomov · 2017-12-23T15:10:06Z

Hi all, I see that the change was already merged.
I think that it is still incomplete. It should be finished before releasing.

No new tests have been added. There should be at least a test for {a:1} and {a:} (because they are counter intuitive)
The file spec-08-13.skip-ext should be removed to let the test fail. Then either the implementation or spec-08-13.canonical should be adapted.

We have a similar request in SnakeYAML
I would prefer to solve the questions before SnakeYAML follows the change.

asomov · 2018-03-03T11:05:16Z

The same change is implemented in SnakeYAML: https://bitbucket.org/asomov/snakeyaml/issues/397

ingydotnet · 2018-06-24T22:29:49Z

@asomov The PyYAML release of this is being planned to happen in the next 48 hours. Would you mind dropping by #pyyaml on irc.freenode.net to discuss?

koitsu · 2020-03-18T00:26:47Z

Commit c5b135f causes a syntax error on Python 3.2.3. Yes I know, 3.2.3 is over 9 years old, but I wanted to at least note this "regression" of sort somewhere in case others come across it. I have not tested Python 2.x.

$ python3
Python 3.2.3 (default, Nov 17 2016, 01:04:00)
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import yaml
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3/yaml/__init__.py", line 8, in <module>
    from .loader import *
  File "/usr/lib/python3/yaml/loader.py", line 5, in <module>
    from .scanner import *
  File "/usr/lib/python3/yaml/scanner.py", line 1294
    + (u',[]{}' if self.flow_level else u''))\
              ^
SyntaxError: invalid syntax

@tamere-allo-peter

Commit c268a82 "key-duplicates: Don't crash on redundant closing brackets or braces" fixed a problem but introduced another one: it crashes on systems with (I guess) an old version of PyYAML. This is probably linked to the "Allow colon in a plain scalar in a flow context" issue on PyYAML [1]. For example, this problem happens on CentOS 8: FAIL: test_disabled (tests.rules.test_key_duplicates.KeyDuplicatesTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "…/tests/rules/test_key_duplicates.py", line 90, in test_disabled '{a:1, b:2}}\n', conf, problem=(2, 11, 'syntax')) File "…/tests/common.py", line 54, in check self.assertEqual(real_problems, expected_problems) AssertionError: Lists differ: … - [2:3: syntax error: found unexpected ':' (syntax)] + [2:11: <no description>] I propose to simply fix the *space following a colon* problem, since it's not related to what the original author @tamere-allo-peter tried to fix. [1]: yaml/pyyaml#45

@tamere-allo-peter

Commit c268a82 "key-duplicates: Don't crash on redundant closing brackets or braces" fixed a problem but introduced another one: it crashes on systems with (I guess) an old version of PyYAML. This is probably linked to the "Allow colon in a plain scalar in a flow context" issue on PyYAML [1]. For example, this problem happens on CentOS 8: FAIL: test_disabled (tests.rules.test_key_duplicates.KeyDuplicatesTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "…/tests/rules/test_key_duplicates.py", line 90, in test_disabled '{a:1, b:2}}\n', conf, problem=(2, 11, 'syntax')) File "…/tests/common.py", line 54, in check self.assertEqual(real_problems, expected_problems) AssertionError: Lists differ: … - [2:3: syntax error: found unexpected ':' (syntax)] + [2:11: <no description>] I propose to simply fix the *space following a colon* problem, since it's not related to what the original author @tamere-allo-peter tried to fix. [1]: yaml/pyyaml#45

Quote scalars with colons in flow style collection SUMMARY Older versions of libyaml and pyyaml don't support colons in scalars that appear in flow style collections. This adds quotes to the handful of cases where they are used in the integration tests. This was uncovered by downstream testing. I am only able to reproduce the problem on the supported ee-minimal-rhel8 image. I believe this is because even though pyyaml should be recent enough, it has likely been compiled against an older version of libyaml that does not have the fix. See: yaml/libyaml#104 yaml/pyyaml#45 It's worth noting that running ansible-lint --fix will undo these changes, which is how I think they were made in the first place. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ADDITIONAL INFORMATION Reviewed-by: Bikouo Aubin Reviewed-by: GomathiselviS

Quote scalars with colons in flow style collection SUMMARY Older versions of libyaml and pyyaml don't support colons in scalars that appear in flow style collections. This adds quotes to the handful of cases where they are used in the integration tests. This was uncovered by downstream testing. I am only able to reproduce the problem on the supported ee-minimal-rhel8 image. I believe this is because even though pyyaml should be recent enough, it has likely been compiled against an older version of libyaml that does not have the fix. See: yaml/libyaml#104 yaml/pyyaml#45 It's worth noting that running ansible-lint --fix will undo these changes, which is how I think they were made in the first place. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ADDITIONAL INFORMATION Reviewed-by: Bikouo Aubin Reviewed-by: GomathiselviS (cherry picked from commit 68fb0a3)

[PR #2014/68fb0a31 backport][stable-7] Quote scalars with colons in flow style collection This is a backport of PR #2014 as merged into main (68fb0a3). SUMMARY Older versions of libyaml and pyyaml don't support colons in scalars that appear in flow style collections. This adds quotes to the handful of cases where they are used in the integration tests. This was uncovered by downstream testing. I am only able to reproduce the problem on the supported ee-minimal-rhel8 image. I believe this is because even though pyyaml should be recent enough, it has likely been compiled against an older version of libyaml that does not have the fix. See: yaml/libyaml#104 yaml/pyyaml#45 It's worth noting that running ansible-lint --fix will undo these changes, which is how I think they were made in the first place. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ADDITIONAL INFORMATION Reviewed-by: Mark Chappell

Allow colon in a plain scalar in a flow context

91f38ed

sigmavirus24 self-assigned this Dec 7, 2016

sigmavirus24 reviewed Dec 7, 2016

View reviewed changes

Restore behavior of flow mapping with empty value

53e22ad

dbeer1 mentioned this pull request Dec 8, 2016

colon can be in a plain scalar yaml/libyaml#28

Closed

adrienverge mentioned this pull request Dec 9, 2016

Symfony routing-file validation adrienverge/yamllint#30

Closed

sigmavirus24 merged commit c5b135f into yaml:master Feb 8, 2017

dbeer1 deleted the feature/colon_in_flow_context branch July 19, 2017 17:12

asomov mentioned this pull request Mar 2, 2018

Revert "Allow colon in a plain scalar in a flow context (#45)" #140

Closed

perlpunk mentioned this pull request Mar 14, 2019

Incompatible changes in version 5.1 #265

Open

dandavison mentioned this pull request Oct 21, 2019

Bump pyyaml from 3.13 to 5.1 dandavison/facet#5

Merged

RhythmicGaming mentioned this pull request Jan 15, 2020

Bump pyyaml from 3.13 to 5.1 in /data_builders/edhrec UncannyBingo/magique#9

Closed

Ciemaar mentioned this pull request May 30, 2020

Bump pyyaml from 3.12 to 5.1 Ciemaar/flock#5

Merged

kddejong mentioned this pull request Apr 19, 2021

Issue parsing colons on Linux aws-cloudformation/cfn-lint#1984

Closed

perlpunk mentioned this pull request Feb 21, 2022

Syntax Error (unexpected colon) for list containing a URL pycontribs/ruyaml#86

Closed

adrienverge mentioned this pull request Jul 8, 2022

key-duplicates: Fix failing test for missing space after colon adrienverge/yamllint#479

Merged

gravesm mentioned this pull request Mar 6, 2024

Quote scalars with colons in flow style collection ansible-collections/amazon.aws#2014

Merged

patchback bot mentioned this pull request Mar 13, 2024

[PR #2014/68fb0a31 backport][stable-7] Quote scalars with colons in flow style collection ansible-collections/amazon.aws#2017

Merged

perlpunk mentioned this pull request May 20, 2024

Strange issue with invalid YAML #801

Closed

keysmashes mentioned this pull request Jan 9, 2025

v0.18 regression: colons in flow mapping values are no longer accepted remarshal-project/remarshal#55

Closed

Conversation

dbeer1 commented Dec 7, 2016

Uh oh!

sigmavirus24 commented Dec 7, 2016

Uh oh!

sigmavirus24 Dec 7, 2016

Choose a reason for hiding this comment

Uh oh!

dbeer1 Dec 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbeer1 Dec 8, 2016

Choose a reason for hiding this comment

Uh oh!

adrienverge commented Dec 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adrienverge commented Feb 6, 2017

Uh oh!

ingydotnet commented Feb 8, 2017

Uh oh!

sigmavirus24 commented Feb 8, 2017

Uh oh!

dbeer1 commented Feb 8, 2017

Uh oh!

bobvandevijver commented Feb 27, 2017

Uh oh!

tobias-93 commented Jul 26, 2017

Uh oh!

asomov commented Dec 23, 2017

Uh oh!

asomov commented Mar 3, 2018

Uh oh!

ingydotnet commented Jun 24, 2018

Uh oh!

koitsu commented Mar 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

dbeer1 Dec 8, 2016 •

edited

Loading

adrienverge commented Dec 9, 2016 •

edited

Loading