Adding support to Unicode characters over codepoint 0xffff by peterkmurphy · Pull Request #63 · yaml/pyyaml

peterkmurphy · 2017-05-10T00:00:51Z

This patch is aimed at solving issue #25 . As a side effect, the testing code has been trimmed to accommodate the fixes. Why have I done that? I might as well repeat what I said on gmail:

The problem I am having is with the testing code, which will need a lot of unwrangling. There are some frankly ... "bizarre" assumptions in what won't work in parseable code, except it sometimes will be parseable by accident. For example:


def test_unicode_input_errors(unicode_filename, verbose=False):
    data = open(unicode_filename, 'rb').read().decode('utf-8')
    for input in [data.encode('latin1', 'ignore'), # <--- Look at this!
                    data.encode('utf-16-be'), data.encode('utf-16-le'),
                    codecs.BOM_UTF8+data.encode('utf-16-be'),
                    codecs.BOM_UTF16_BE+data.encode('utf-16-le'),
                    codecs.BOM_UTF16_LE+data.encode('utf-8')+'!']:
        try:
            yaml.load(input)
        except yaml.YAMLError, exc:
            if verbose:
                print exc
        else:
            raise AssertionError("expected an exception")

The idea: let's cause some bizarre combinations of byte sequences, attempt to parse it, and if it doesn't throw a YAMLError, raise an exception. Except that when one does data.encode('latin1', 'ignore') on data, one results in ten line breaks, which is happily parseable as YAML. So no exception raised, so AssertionError.

What should I do in this case - remove test_unicode_input_errors from the PyYaml testing code? Yes, the number of tests will go down, which is generally not a good thing, but if the tests are based on dodgy assumptions...

In some cases I have altered testing code; others I have removed them.

…point 0xffff yaml#25

adamchainz · 2017-05-10T09:18:27Z

README


 PyYAML is written by Kirill Simonov <xi@resolvent.net>.  It is released
 under the MIT license. See the file LICENSE for more details.
-


removing trailing newlines is bad form as it's diff noise. I'd undo it for all the files you've added it for, which seems to be every file you've touched. Also files should end with a trailing newline to be POSIX valid

adamchainz · 2017-05-10T09:18:43Z

lib/yaml/emitter.py

            if index == 0:
                # Leading indicators are special characters.
-                if ch in u'#,[]{}&*!|>\'\"%@`': 
+                if ch in u'#,[]{}&*!|>\'\"%@`':


trimming trailing whitespace is bad form as it's diff noise

peterkmurphy · 2017-05-10T10:02:17Z

Adam, Please feel free to do the changes as you see fit. I don't know why they happened at all. Perhaps the editor I was using (Atom on Linux Mint x64) is to blame. Best regards, Peter

…

On Wed, May 10, 2017 at 7:21 PM, Adam Johnson ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In README <#63 (comment)>: > @@ -32,4 +32,3 @@ Submit bug reports and feature requests to the PyYAML bug tracker: PyYAML is written by Kirill Simonov ***@***.***>. It is released under the MIT license. See the file LICENSE for more details. - removing trailing newlines is bad form as it's diff noise. I'd undo it for all the files you've added it for, which seems to be every file you've touched. Also files should end with a trailing newline to be POSIX valid ------------------------------ In lib/yaml/emitter.py <#63 (comment)>: > @@ -674,7 +678,7 @@ def analyze_scalar(self, scalar): # Check for indicators. if index == 0: # Leading indicators are special characters. - if ch in u'#,[]{}&*!|>\'\"%@`': + if ch in u'#,[]{}&*!|>\'\"%@`': trimming trailing whitespace is bad form as it's diff noise — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#63 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-AtYWG4MJjDFP9prMvmTILeunl8Nkiks5r4YGDgaJpZM4NWBnY> .

-- Email: peterkmurphy@gmail.com WWW: http://www.pkmurphy.com.au/

adamchainz · 2017-05-10T16:08:36Z

@peterkmurphy I'm not an admin on this project, I can't do anything to your PR.

samdmarshall · 2017-05-16T02:15:36Z

you may need to ping @sigmavirus24 or another commiter for this project to get this merged and a new release made.

Fixes yaml#25. Rebase and tidy up of yaml#63.

adamchainz · 2017-05-16T09:38:42Z

I copied this and tidied it up in #65

peterkmurphy · 2017-05-16T10:15:38Z

Adam, That's probably for the best. I will keep my pull request up until yours is integrated into the main branch; it contains some explanatory notes. After that, I'll withdraw mine. Thanks for cleaning up. Best regards, Peter

…

On Tue, May 16, 2017 at 7:38 PM, Adam Johnson ***@***.***> wrote: I copied this and tidied it up in #65 <#65> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#63 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-Ates03dkvncNnSz2JgYVMklsghcJTks5r6W6jgaJpZM4NWBnY> .

-- Email: peterkmurphy@gmail.com WWW: http://www.pkmurphy.com.au/

sigmavirus24 · 2017-08-08T00:38:01Z

Thanks @peterkmurphy! 🎉 ✨

jborean93 · 2018-10-23T21:20:33Z

@ingydotnet is there any chance this fix could be backported to the 3.x branch and a new release made? I can install the pre-release 4.x builds but I haven't seen any action recently that indicates a full release will be made on those changes.

cc @nitzmahone

nitzmahone · 2018-10-23T21:49:12Z

I'd be +1 for that

ingydotnet · 2018-10-24T04:03:26Z

@perlpunk and I are meeting up in a week. We might be able to discuss it then.

Fixes #25. Rebase and tidy up of #63.

peterkmurphy added 5 commits May 7, 2017 08:53

A change to a message

a06c1f6

First attack at pyyaml does not support literals in unicode over code…

cf1c86c

…point 0xffff yaml#25

Added emoticon test data files (which will probably break testing)

c60232d

Suspicious 'expected an exception' messages trimmed

c67d8df

Reverting README to old copy

94c3f07

peterkmurphy mentioned this pull request May 10, 2017

pyyaml does not support literals in unicode over codepoint 0xffff #25

Closed

adamchainz suggested changes May 10, 2017

View reviewed changes

adamchainz pushed a commit to adamchainz/pyyaml that referenced this pull request May 16, 2017

Support unicode literals over codepoint 0xffff

421c9b3

Fixes yaml#25. Rebase and tidy up of yaml#63.

adamchainz mentioned this pull request May 16, 2017

Support unicode literals over codepoint 0xffff #65

Closed

adamchainz pushed a commit to adamchainz/pyyaml that referenced this pull request May 16, 2017

Support unicode literals over codepoint 0xffff

e7272cc

Fixes yaml#25. Rebase and tidy up of yaml#63.

sigmavirus24 merged commit 94c3f07 into yaml:master Aug 8, 2017

perlpunk mentioned this pull request Mar 14, 2019

Incompatible changes in version 5.1 #265

Open

ingydotnet pushed a commit that referenced this pull request Mar 14, 2019

Support unicode literals over codepoint 0xffff

3a1452c

Fixes #25. Rebase and tidy up of #63.

adrienverge mentioned this pull request Mar 17, 2019

Crashes on encountering emojis adrienverge/yamllint#48

Closed

perlpunk mentioned this pull request Mar 17, 2019

Version 5.1 dumps strings differently on different platforms #275

Closed

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

This was referenced Mar 9, 2021

build(deps-dev): bump pyyaml from 3.12 to 5.1 in /requirements bilalzee/serverless-application-model#2

Closed

Bump pyyaml from 3.13 to 5.1 lifeready/dvpwa#3

Closed

Bump pyyaml from 3.13 to 5.1 DimitriGerson/appFlask#9

Closed

dtolnay mentioned this pull request Mar 5, 2023

Add support for emitting Unicode characters over codepoint U+FFFF dtolnay/unsafe-libyaml#20

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support to Unicode characters over codepoint 0xffff#63

Adding support to Unicode characters over codepoint 0xffff#63
sigmavirus24 merged 5 commits intoyaml:masterfrom
peterkmurphy:master

peterkmurphy commented May 10, 2017

Uh oh!

adamchainz May 10, 2017

Uh oh!

adamchainz May 10, 2017

Uh oh!

peterkmurphy commented May 10, 2017 via email

Uh oh!

adamchainz commented May 10, 2017

Uh oh!

samdmarshall commented May 16, 2017

Uh oh!

adamchainz commented May 16, 2017

Uh oh!

peterkmurphy commented May 16, 2017 via email

Uh oh!

sigmavirus24 commented Aug 8, 2017

Uh oh!

jborean93 commented Oct 23, 2018

Uh oh!

nitzmahone commented Oct 23, 2018

Uh oh!

ingydotnet commented Oct 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants


		PyYAML is written by Kirill Simonov <xi@resolvent.net>. It is released
		under the MIT license. See the file LICENSE for more details.

Conversation

peterkmurphy commented May 10, 2017

Uh oh!

adamchainz May 10, 2017

Choose a reason for hiding this comment

Uh oh!

adamchainz May 10, 2017

Choose a reason for hiding this comment

Uh oh!

peterkmurphy commented May 10, 2017 via email

Uh oh!

adamchainz commented May 10, 2017

Uh oh!

samdmarshall commented May 16, 2017

Uh oh!

adamchainz commented May 16, 2017

Uh oh!

peterkmurphy commented May 16, 2017 via email

Uh oh!

sigmavirus24 commented Aug 8, 2017

Uh oh!

jborean93 commented Oct 23, 2018

Uh oh!

nitzmahone commented Oct 23, 2018

Uh oh!

ingydotnet commented Oct 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants