From 1d14e2582412e5a982c895638c26278281b01cff Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 26 Aug 2021 13:33:45 +0200 Subject: [PATCH] Fix Psych 4 compatibility and add caching for YAML.unsafe_load_file Psych 4 now load in safe mode by default, which means many features such as aliases are disabled by default. This makes it complicated for Bootsnap to cache, and pretty much impossible to precompile. So until we figure out a better solution, we're better to entirely disable that cache for Psych 4+. However it can stay active for `YAML.unsafe_load_file`. --- .github/workflows/ci.yaml | 17 ++++++ CHANGELOG.md | 2 + Gemfile | 4 ++ lib/bootsnap/compile_cache/yaml.rb | 30 ++++++++- test/compile_cache/yaml_test.rb | 97 +++++++++++++++++++----------- 5 files changed, 114 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c4512830..e2ed8879 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -36,6 +36,23 @@ jobs: - run: bundle install - run: bundle exec rake + psych4: + strategy: + matrix: + os: [ubuntu] + ruby: ['3.0'] + runs-on: ${{ matrix.os }}-latest + env: + PSYCH_4: "1" + steps: + - uses: actions/checkout@v2 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + - run: bundle install + - run: bundle exec rake + + minimal: strategy: matrix: diff --git a/CHANGELOG.md b/CHANGELOG.md index c4150d9c..c8cbd1b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +* Improve support for Pysch 4. + # 1.7.7 * Fix `require_relative` in evaled code on latest ruby 3.1.0-dev. (#366) diff --git a/Gemfile b/Gemfile index 2c349539..6aee029c 100644 --- a/Gemfile +++ b/Gemfile @@ -4,6 +4,10 @@ source 'https://rubygems.org' # Specify your gem's dependencies in bootsnap.gemspec gemspec +if ENV["PSYCH_4"] + gem "psych", ">= 4" +end + group :development do gem 'rubocop' gem 'byebug', platform: :ruby diff --git a/lib/bootsnap/compile_cache/yaml.rb b/lib/bootsnap/compile_cache/yaml.rb index ddd4b167..f44dd32a 100644 --- a/lib/bootsnap/compile_cache/yaml.rb +++ b/lib/bootsnap/compile_cache/yaml.rb @@ -23,7 +23,7 @@ def storage_to_output(data, kwargs) end def input_to_output(data, kwargs) - ::YAML.load(data, **(kwargs || {})) + ::YAML.unsafe_load(data, **(kwargs || {})) end def strict_load(payload, *args) @@ -52,6 +52,13 @@ def init! require('msgpack') require('date') + if Patch.method_defined?(:unsafe_load_file) && !::YAML.respond_to?(:unsafe_load_file) + Patch.send(:remove_method, :unsafe_load_file) + end + if Patch.method_defined?(:load_file) && ::YAML::VERSION >= '4' + Patch.send(:remove_method, :load_file) + end + # MessagePack serializes symbols as strings by default. # We want them to roundtrip cleanly, so we use a custom factory. # see: https://github.com/msgpack/msgpack-ruby/pull/122 @@ -126,6 +133,27 @@ def load_file(path, *args) end ruby2_keywords :load_file if respond_to?(:ruby2_keywords, true) + + def unsafe_load_file(path, *args) + return super if args.size > 1 + if kwargs = args.first + return super unless kwargs.is_a?(Hash) + return super unless (kwargs.keys - ::Bootsnap::CompileCache::YAML.supported_options).empty? + end + + begin + ::Bootsnap::CompileCache::Native.fetch( + Bootsnap::CompileCache::YAML.cache_dir, + File.realpath(path), + ::Bootsnap::CompileCache::YAML, + kwargs, + ) + rescue Errno::EACCES + ::Bootsnap::CompileCache.permission_error(path) + end + end + + ruby2_keywords :unsafe_load_file if respond_to?(:ruby2_keywords, true) end end end diff --git a/test/compile_cache/yaml_test.rb b/test/compile_cache/yaml_test.rb index a81ff50d..9518a859 100644 --- a/test/compile_cache/yaml_test.rb +++ b/test/compile_cache/yaml_test.rb @@ -6,17 +6,21 @@ class CompileCacheYAMLTest < Minitest::Test module FakeYaml Fallback = Class.new(StandardError) - extend self - singleton_class.prepend(Bootsnap::CompileCache::YAML::Patch) + class << self + def load_file(path, symbolize_names: false, freeze: false, fallback: nil) + raise Fallback + end - def load_file(path, symbolize_names: false, freeze: false, fallback: nil) - raise Fallback + def unsafe_load_file(path, symbolize_names: false, freeze: false, fallback: nil) + raise Fallback + end end end def setup super Bootsnap::CompileCache::YAML.init! + FakeYaml.singleton_class.prepend(Bootsnap::CompileCache::YAML::Patch) end def test_yaml_strict_load @@ -44,49 +48,72 @@ def test_yaml_tags assert_equal "YAML tags are not supported: !ruby/object", error.message end - def test_load_file - Help.set_file('a.yml', "---\nfoo: bar", 100) - assert_equal({'foo' => 'bar'}, FakeYaml.load_file('a.yml')) - end + if YAML::VERSION >= '4' + def test_load_psych_4 + # Until we figure out a proper strategy, only `YAML.unsafe_load_file` + # is cached with Psych >= 4 + Help.set_file('a.yml', "foo: &foo\n bar: 42\nplop:\n <<: *foo", 100) + assert_raises FakeYaml::Fallback do + FakeYaml.load_file('a.yml') + end + end + else + def test_load_file + Help.set_file('a.yml', "---\nfoo: bar", 100) + assert_equal({'foo' => 'bar'}, FakeYaml.load_file('a.yml')) + end - def test_load_file_symbolize_names - Help.set_file('a.yml', "---\nfoo: bar", 100) - FakeYaml.load_file('a.yml') + def test_load_file_aliases + Help.set_file('a.yml', "foo: &foo\n bar: 42\nplop:\n <<: *foo", 100) + assert_equal({"foo" => { "bar" => 42 }, "plop" => { "bar" => 42} }, FakeYaml.load_file('a.yml')) + end - if ::Bootsnap::CompileCache::YAML.supported_options.include?(:symbolize_names) - 2.times do - assert_equal({foo: 'bar'}, FakeYaml.load_file('a.yml', symbolize_names: true)) - end - else - assert_raises(FakeYaml::Fallback) do # would call super - FakeYaml.load_file('a.yml', symbolize_names: true) + def test_load_file_symbolize_names + Help.set_file('a.yml', "---\nfoo: bar", 100) + FakeYaml.load_file('a.yml') + + if ::Bootsnap::CompileCache::YAML.supported_options.include?(:symbolize_names) + 2.times do + assert_equal({foo: 'bar'}, FakeYaml.load_file('a.yml', symbolize_names: true)) + end + else + assert_raises(FakeYaml::Fallback) do # would call super + FakeYaml.load_file('a.yml', symbolize_names: true) + end end end - end - def test_load_file_freeze - Help.set_file('a.yml', "---\nfoo", 100) - FakeYaml.load_file('a.yml') + def test_load_file_freeze + Help.set_file('a.yml', "---\nfoo", 100) + FakeYaml.load_file('a.yml') - if ::Bootsnap::CompileCache::YAML.supported_options.include?(:freeze) - 2.times do - string = FakeYaml.load_file('a.yml', freeze: true) - assert_equal("foo", string) - assert_predicate(string, :frozen?) + if ::Bootsnap::CompileCache::YAML.supported_options.include?(:freeze) + 2.times do + string = FakeYaml.load_file('a.yml', freeze: true) + assert_equal("foo", string) + assert_predicate(string, :frozen?) + end + else + assert_raises(FakeYaml::Fallback) do # would call super + FakeYaml.load_file('a.yml', freeze: true) + end end - else + end + + def test_load_file_unknown_option + Help.set_file('a.yml', "---\nfoo", 100) + FakeYaml.load_file('a.yml') + assert_raises(FakeYaml::Fallback) do # would call super - FakeYaml.load_file('a.yml', freeze: true) + FakeYaml.load_file('a.yml', fallback: true) end end end - def test_load_file_unknown_option - Help.set_file('a.yml', "---\nfoo", 100) - FakeYaml.load_file('a.yml') - - assert_raises(FakeYaml::Fallback) do # would call super - FakeYaml.load_file('a.yml', fallback: true) + if YAML.respond_to?(:unsafe_load_file) + def test_unsafe_load_file + Help.set_file('a.yml', "foo: &foo\n bar: 42\nplop:\n <<: *foo", 100) + assert_equal({"foo" => { "bar" => 42 }, "plop" => { "bar" => 42} }, FakeYaml.unsafe_load_file('a.yml')) end end end