Skip to content

Remove safe_yaml.#2397

Merged
bbenezech merged 2 commits into
railsadminteam:masterfrom
bf4:remove_safe_yaml
Jun 7, 2016
Merged

Remove safe_yaml.#2397
bbenezech merged 2 commits into
railsadminteam:masterfrom
bf4:remove_safe_yaml

Conversation

@bf4

@bf4 bf4 commented Sep 4, 2015

Copy link
Copy Markdown
Contributor

I don't think it's really this gem's job, but besides that, I was getting a failure
when starting zeus, so I just made it optional.

The below stacktrace is from a fork of v0.6.7 with a commit on top of it:

https://github.com/swipesense/rails_admin/commits/0.6.7-eager_loading

Here's the stacktrace:

$ zeus rake

     ~/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/psych/parser.rb:33:in `<class:Parser>': superclass mismatch for class Mark (TypeError)
from ~/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/psych/parser.rb:32:in `<module:Psych>'
from ~/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/psych/parser.rb:1:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/psych.rb:7:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/safe_yaml-1.0.4/lib/safe_yaml/psych_handler.rb:1:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/safe_yaml-1.0.4/lib/safe_yaml/load.rb:130:in `<module:SafeYAML>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/safe_yaml-1.0.4/lib/safe_yaml/load.rb:26:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/safe_yaml-1.0.4/lib/safe_yaml.rb:1:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/bundler/gems/rails_admin-975bb5075824/lib/rails_admin/engine.rb:10:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/bundler/gems/rails_admin-975bb5075824/lib/rails_admin.rb:1:in `<top (required)>'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:76:in `require'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:76:in `block (2 levels) in require'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:72:in `each'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:72:in `block in require'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:61:in `each'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:61:in `require'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler.rb:134:in `require'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus/rails.rb:97:in `default_bundle'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:200:in `run_action'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:74:in `block (2 levels) in boot_steps'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus/load_tracking.rb:7:in `features_loaded_by'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:73:in `block in boot_steps'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:56:in `catch'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:56:in `boot_steps'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:48:in `go'
from -e:1:in `<main>'

@bf4 bf4 force-pushed the remove_safe_yaml branch from d0b0411 to e78eaf3 Compare October 21, 2015 16:28
@bf4

bf4 commented Oct 21, 2015

Copy link
Copy Markdown
Contributor Author

Rebased against current master

Comment thread lib/rails_admin/engine.rb Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An error should be raised here, not warning. We shouldn't let users choose unsafe state.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will change

B mobile phone

On Nov 3, 2015, at 3:08 AM, Mitsuhiro Shibuya notifications@github.com wrote:

In lib/rails_admin/engine.rb:

module RailsAdmin

  • Backwards-compatible RailsAdmin::SafeYAML when SafeYAML isn't available

  • begin
  • require 'safe_yaml/load'
  • rescue LoadError
  • if YAML.respond_to?(:safe_load)
  •  SafeYAML = Module.new do
    
  •    def self.load(yaml)
    
  •      YAML.safe_load(yaml)
    
  •    end
    
  •  end
    
  • else
  •  warn "Safe-loading of YAML is not available. Please install 'safe_yaml' or install Psych 2.0+"
    
    An error should be raised here, not warning. We shouldn't let users choose unsafe state.


Reply to this email directly or view it on GitHub.

@bf4 bf4 force-pushed the remove_safe_yaml branch from e78eaf3 to 16ffd04 Compare November 3, 2015 18:40
Comment thread lib/rails_admin/engine.rb Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guard clause raises same exception but with better message when there's no fallback

@bbenezech

Copy link
Copy Markdown
Collaborator

@bf4 can we merge this?

@bf4 bf4 force-pushed the remove_safe_yaml branch 2 times, most recently from 07c9f0b to 830e99b Compare June 6, 2016 17:08
bf4 and others added 2 commits June 6, 2016 12:08
I don't think it's really this gem's job, but besides that, I was getting a failure
when starting zeus, so I just made it optional.

This one a fork of v0.6.7 with a commit on top of it:

https://github.com/swipesense/rails_admin/commits/0.6.7-eager_loading

Here's the stacktrace:

$ zeus rake

     ~/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/psych/parser.rb:33:in `<class:Parser>': superclass mismatch for class Mark (TypeError)
from ~/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/psych/parser.rb:32:in `<module:Psych>'
from ~/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/psych/parser.rb:1:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/psych.rb:7:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/safe_yaml-1.0.4/lib/safe_yaml/psych_handler.rb:1:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/safe_yaml-1.0.4/lib/safe_yaml/load.rb:130:in `<module:SafeYAML>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/safe_yaml-1.0.4/lib/safe_yaml/load.rb:26:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/safe_yaml-1.0.4/lib/safe_yaml.rb:1:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/bundler/gems/rails_admin-975bb5075824/lib/rails_admin/engine.rb:10:in `<top (required)>'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `block in require'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
from ~/projects/rails_server/bundle/ruby/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:274:in `require'
from ~/projects/rails_server/bundle/ruby/2.2.0/bundler/gems/rails_admin-975bb5075824/lib/rails_admin.rb:1:in `<top (required)>'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:76:in `require'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:76:in `block (2 levels) in require'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:72:in `each'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:72:in `block in require'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:61:in `each'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler/runtime.rb:61:in `require'
from ~/.rvm/gems/ruby-2.2.2/gems/bundler-1.10.6/lib/bundler.rb:134:in `require'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus/rails.rb:97:in `default_bundle'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:200:in `run_action'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:74:in `block (2 levels) in boot_steps'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus/load_tracking.rb:7:in `features_loaded_by'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:73:in `block in boot_steps'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:56:in `catch'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:56:in `boot_steps'
from ~/.rvm/gems/ruby-2.2.2/gems/zeus-0.15.4/lib/zeus.rb:48:in `go'
from -e:1:in `<main>'
Failure running today June 6, 2016 at 10:47 am CST

```
1) RailsAdmin::Adapters::ActiveRecord #build_statement supports datetime type query
   Failure/Error: expect(abstract_model.send(:filter_scope, scope,  'datetime_field' => {'1' => {v: ['', 'February 01, 2012 00:00', 'March 01, 2012 00:00'], o: 'between'}}).where_values).to eq(scope.where(['(field_tests.datetime_field BETWEEN ? AND ?)', Time.local(2012, 2, 1), Time.local(2012, 3, 1).end_of_day]).where_values)

       expected: ["(field_tests.datetime_field BETWEEN '2012-02-01 06:00:00.000000' AND '2012-03-02 05:59:59.999999')"]
            got: ["(field_tests.datetime_field BETWEEN '2012-01-31 06:00:00.000000' AND '2012-03-01 05:59:59.999999')"]

2) RailsAdmin::AbstractModel filters on datetimes with :en locale lists elements within outbound limits
   Failure/Error: expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['', 'January 03, 2012 00:00', ''], o: 'between'}}}).count).to eq(2)

        expected: 2
             got: 3
```
@bf4 bf4 force-pushed the remove_safe_yaml branch from 830e99b to 5c14d48 Compare June 6, 2016 17:08
@bf4

bf4 commented Jun 7, 2016

Copy link
Copy Markdown
Contributor Author

@bbenezech I think it can be merged, and is actually better since you last asked the question.

Comment thread lib/rails_admin.rb
require 'safe_yaml/load'
def self.yaml_load(yaml)
SafeYAML.load(yaml)
end

@bf4 bf4 Jun 7, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

define self.yaml_load to use SafeYAML.load in non-monkey-patching mode, when available

@bbenezech bbenezech merged commit 9819122 into railsadminteam:master Jun 7, 2016
@bbenezech

Copy link
Copy Markdown
Collaborator

Thanks a lot. It's good to see safe_yaml go. (with psych 2 at least)

@bf4

bf4 commented Jun 8, 2016

Copy link
Copy Markdown
Contributor Author

Awesome!

@bf4 bf4 deleted the remove_safe_yaml branch June 8, 2016 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants