Bug #9300

YAML Regression Concerning Escaping of Strings

Added by Richard Schneeman about 1 year ago. Updated about 1 year ago.

[ruby-core:59316]
Status:Closed
Priority:Normal
Assignee:Aaron Patterson
ruby -v:2.1.0 Backport:1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: DONE

Description

=begin
When you run this code against Ruby 2.1.0 and previous versions you get different results:

require 'YAML'
yaml1 = {"key" => %Q{<%= ENV["PATH"] %>} }.to_yaml
puts yaml1

Ruby 2.1.0 incorrectly escapes the quote


key: "<%= ENV[\"PATH\"] %>"

While previous versions do not. Here is my original document for debugging this issue:

https://gist.github.com/schneems/8127922

=end

Associated revisions

Revision 44531
Added by tenderlove about 1 year ago

  • ext/psych/lib/psych/visitors/yaml_tree.rb: dumping strings with
    quotes should not have changed. [Bug #9300]

  • ext/psych/lib/psych.rb: fixed missing require.

  • test/psych/test_string.rb: test

Revision 44531
Added by tenderlove about 1 year ago

  • ext/psych/lib/psych/visitors/yaml_tree.rb: dumping strings with
    quotes should not have changed. [Bug #9300]

  • ext/psych/lib/psych.rb: fixed missing require.

  • test/psych/test_string.rb: test

History

#1 Updated by Hiroshi SHIBATA about 1 year ago

  • Assignee set to Aaron Patterson
  • Target version changed from 2.1.0 to current: 2.2.0

#2 Updated by Charlie Somerville about 1 year ago

It seems legit to me:

>> YAML.load(YAML.dump({"key"=>%{<%= ENV["HOME"] %>}}))
=> {"key"=>"<%= ENV[\"HOME\"] %>"}

Psych is correctly round-tripping the data, so I don't think there's any incorrect escaping happening here.

#3 Updated by Benoit Daloze about 1 year ago

It should as well round-trip data across versions, which it seems it does in this case.

Yet, I find it worrying quoting is changed in a minor version, should not the YAML spec be clear about this?

#4 Updated by Zachary Scott about 1 year ago

It seems Richard fixed this in Rails master: https://github.com/rails/rails/commit/d0926d3

Should this be closed then? There is a workaround solution available

#5 Updated by Richard Schneeman about 1 year ago

Do we know what changed in psych to cause this? Was this the result of a bug before that was fixed? It is fixed on that Rails commit, but it looks like this is not a stable interface if it can break at any time. I expect to_yaml to know more about writing valid YAML than me :)

#6 Updated by Aaron Patterson about 1 year ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r44531.
Richard, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • ext/psych/lib/psych/visitors/yaml_tree.rb: dumping strings with
    quotes should not have changed. [Bug #9300]

  • ext/psych/lib/psych.rb: fixed missing require.

  • test/psych/test_string.rb: test

#7 Updated by Aaron Patterson about 1 year ago

On Sat, Dec 28, 2013 at 01:35:02AM +0900, schneems (Richard Schneeman) wrote:

Issue #9300 has been updated by schneems (Richard Schneeman).

Do we know what changed in psych to cause this? Was this the result of a bug before that was fixed? It is fixed on that Rails commit, but it looks like this is not a stable interface if it can break at any time. I expect to_yaml to know more about writing valid YAML than me :)

The bug started in r42850. The output is valid YAML and does represent
the same data as it used to. It may look different, but the data is the
same. Though, I see this is trying to write valid ERb via YAML dump.
;-)

The output shouldn't have changed for this, and I fixed it in r44531.
I'll make a request for a backport.

Thanks for reporting this!
--
Aaron Patterson
http://tenderlovemaking.com/

#8 Updated by Usaku NAKAMURA about 1 year ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN to 1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: REQUIRED

#9 Updated by Yui NARUSE about 1 year ago

  • Backport changed from 1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: REQUIRED to 1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: DONE

r45079.

Also available in: Atom PDF