Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On destroy, Integer returned instead of object #365

Open
andreierdoss opened this issue Nov 16, 2022 · 11 comments
Open

On destroy, Integer returned instead of object #365

andreierdoss opened this issue Nov 16, 2022 · 11 comments

Comments

@andreierdoss
Copy link

andreierdoss commented Nov 16, 2022

I migrated from Ruby 2.6.6 to 3.1.2 and merit gem 3.0.3 to 4.03 and now, on destroy action, inside the block, the parameter passed in (vote) is an integer (always the same number - 181), not a Vote object. The other actions work as expected.

score 2, on: 'votes#destroy', to: :voteable_user do |vote|
         vote.direction == 'down' && vote.voteable_type == 'Question'
end
@tute
Copy link
Member

tute commented Nov 16, 2022 via email

@andreierdoss
Copy link
Author

andreierdoss commented Nov 17, 2022

  def destroy
    authorize! vote_action(vote_params[:direction]), @voteable
    @vote = Vote.find_by(voteable_id: @voteable.id,
                      voteable_type: @voteable.class.name,
                      user_id: current_user.id,
                      direction: Vote.directions[vote_params[:direction]])
    @vote.destroy
    @voteable.reload
  end

@tute
Copy link
Member

tute commented Nov 17, 2022

Thanks! And can you post the logs for the action? Particularly I want to see what target_data holds. TY.

@andreierdoss
Copy link
Author

Here are the logs on DELETE of vote

Started DELETE "/votes/up/question/158" for ::1 at 2022-11-18 14:13:50 +0200
Processing by VotesController#destroy as JS
  Parameters: {"direction"=>"up", "voteable_type"=>"question", "voteable_id"=>"158"}
  User Load (0.2ms)  SELECT "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" ASC LIMIT $2  [["id", 1], ["LIMIT", 1]]
  Question Load (5.4ms)  SELECT "questions".* FROM "questions" WHERE "questions"."id" = $1 LIMIT $2  [["id", 158], ["LIMIT", 1]]
  ↳ app/controllers/votes_controller.rb:26:in `find_voteable'
  Shooter Load (0.3ms)  SELECT "shooters".* FROM "shooters" WHERE "shooters"."user_id" = $1 LIMIT $2  [["user_id", 1], ["LIMIT", 1]]
  ↳ app/models/ability.rb:7:in `initialize'
  Coach Load (0.3ms)  SELECT "coaches".* FROM "coaches" WHERE "coaches"."user_id" = $1 LIMIT $2  [["user_id", 1], ["LIMIT", 1]]
  ↳ app/models/ability.rb:8:in `initialize'
  Merit::Sash Load (0.2ms)  SELECT "sashes".* FROM "sashes" WHERE "sashes"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
  ↳ app/models/ability.rb:137:in `initialize'
  Merit::Score Load (0.3ms)  SELECT "merit_scores".* FROM "merit_scores" WHERE "merit_scores"."sash_id" = $1  [["sash_id", 1]]
  ↳ app/models/ability.rb:137:in `initialize'
   (0.4ms)  SELECT SUM("merit_score_points"."num_points") AS sum_num_points, "merit_score_points"."score_id" AS merit_score_points_score_id FROM "merit_score_points" WHERE "merit_score_points"."score_id" = $1 GROUP BY "merit_score_points"."score_id"  [["score_id", 1]]
  ↳ app/models/ability.rb:137:in `initialize'
  CACHE  (0.0ms)  SELECT SUM("merit_score_points"."num_points") AS sum_num_points, "merit_score_points"."score_id" AS merit_score_points_score_id FROM "merit_score_points" WHERE "merit_score_points"."score_id" = $1 GROUP BY "merit_score_points"."score_id"  [["score_id", 1]]
  ↳ app/models/ability.rb:138:in `initialize'
  CACHE  (0.0ms)  SELECT SUM("merit_score_points"."num_points") AS sum_num_points, "merit_score_points"."score_id" AS merit_score_points_score_id FROM "merit_score_points" WHERE "merit_score_points"."score_id" = $1 GROUP BY "merit_score_points"."score_id"  [["score_id", 1]]
  ↳ app/models/ability.rb:141:in `initialize'
  CACHE  (0.0ms)  SELECT SUM("merit_score_points"."num_points") AS sum_num_points, "merit_score_points"."score_id" AS merit_score_points_score_id FROM "merit_score_points" WHERE "merit_score_points"."score_id" = $1 GROUP BY "merit_score_points"."score_id"  [["score_id", 1]]
  ↳ app/models/ability.rb:142:in `initialize'
  User Load (0.6ms)  SELECT "users".* FROM "users" WHERE "users"."id" = $1 LIMIT $2  [["id", 2026], ["LIMIT", 1]]
  ↳ app/controllers/votes_controller.rb:11:in `destroy'
  Vote Load (0.4ms)  SELECT "votes".* FROM "votes" WHERE "votes"."voteable_id" = $1 AND "votes"."voteable_type" = $2 AND "votes"."user_id" = $3 AND "votes"."direction" = $4 LIMIT $5  [["voteable_id", 158], ["voteable_type", "Question"], ["user_id", 1], ["direction", 1], ["LIMIT", 1]]
  ↳ app/controllers/votes_controller.rb:12:in `destroy'
  TRANSACTION (0.2ms)  BEGIN
  ↳ app/controllers/votes_controller.rb:16:in `destroy'
  Vote Destroy (0.4ms)  DELETE FROM "votes" WHERE "votes"."id" = $1  [["id", 224]]
  ↳ app/controllers/votes_controller.rb:16:in `destroy'
  Question Load (0.4ms)  SELECT "questions".* FROM "questions" WHERE "questions"."id" = $1 LIMIT $2  [["id", 158], ["LIMIT", 1]]
  ↳ app/models/vote.rb:32:in `update_voteable_votes_counters'
  User Load (0.3ms)  SELECT "users".* FROM "users" WHERE "users"."id" = $1 LIMIT $2  [["id", 2026], ["LIMIT", 1]]
  ↳ app/models/vote.rb:32:in `update_voteable_votes_counters'
  Question Update (0.7ms)  UPDATE "questions" SET "up_votes" = $1, "votes_sum" = $2, "updated_at" = $3 WHERE "questions"."id" = $4  [["up_votes", 0], ["votes_sum", 0], ["updated_at", "2022-11-18 12:13:50.609765"], ["id", 158]]
  ↳ app/models/vote.rb:32:in `update_voteable_votes_counters'
  TRANSACTION (39.6ms)  COMMIT
  ↳ app/controllers/votes_controller.rb:16:in `destroy'
  Question Load (0.3ms)  SELECT "questions".* FROM "questions" WHERE "questions"."id" = $1 LIMIT $2  [["id", 158], ["LIMIT", 1]]
  ↳ app/controllers/votes_controller.rb:17:in `destroy'
  Rendering votes/destroy.js.haml
   (0.6ms)  SELECT "users"."name" FROM "users" INNER JOIN "votes" ON "users"."id" = "votes"."user_id" WHERE "votes"."voteable_id" = $1 AND "votes"."voteable_type" = $2  [["voteable_id", 158], ["voteable_type", "Question"]]
  ↳ app/helpers/votes_helper.rb:46:in `votes_options'
  Rendered votes/destroy.js.haml (Duration: 13.5ms | Allocations: 7940)
  TRANSACTION (0.2ms)  BEGIN
  Merit::Action Create (2.2ms)  INSERT INTO "merit_actions" ("user_id", "action_method", "target_model", "target_id", "target_data", "created_at", "updated_at") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id"  [["user_id", 1], ["action_method", "destroy"], ["target_model", "votes"], ["target_id", 224], ["target_data", "--- !ruby/object:Vote\nconcise_attributes:\n- !ruby/object:ActiveModel::Attribute::FromDatabase\n  name: id\n  value_before_type_cast: 224\n- !ruby/object:ActiveModel::Attribute::FromDatabase\n  name: voteable_id\n  value_before_type_cast: 158\n- !ruby/object:ActiveModel::Attribute::FromDatabase\n  name: voteable_type\n  value_before_type_cast: Question\n- !ruby/object:ActiveModel::Attribute::FromDatabase\n  name: direction\n  value_before_type_cast: 1\n- !ruby/object:ActiveModel::Attribute::FromDatabase\n  name: created_at\n  value_before_type_cast: 2022-11-18 12:13:37.250735000 Z\n- !ruby/object:ActiveModel::Attribute::FromDatabase\n  name: updated_at\n  value_before_type_cast: 2022-11-18 12:13:37.250735000 Z\n- !ruby/object:ActiveModel::Attribute::FromDatabase\n  name: user_id\n  value_before_type_cast: 1\nnew_record: false\nactive_record_yaml_version: 2\n"], ["created_at", "2022-11-18 12:13:50.683021"], ["updated_at", "2022-11-18 12:13:50.683021"]]
  TRANSACTION (0.6ms)  COMMIT
  Merit::Action Load (0.5ms)  SELECT "merit_actions".* FROM "merit_actions" WHERE "merit_actions"."processed" = $1 ORDER BY "merit_actions"."id" ASC LIMIT $2  [["processed", false], ["LIMIT", 1000]]
  TRANSACTION (0.2ms)  BEGIN
  Merit::Action Update (0.4ms)  UPDATE "merit_actions" SET "processed" = $1, "updated_at" = $2 WHERE "merit_actions"."id" = $3  [["processed", true], ["updated_at", "2022-11-18 12:13:50.711079"], ["id", 146]]
  TRANSACTION (0.6ms)  COMMIT
  Vote Load (0.5ms)  SELECT "votes".* FROM "votes" WHERE "votes"."id" = $1 LIMIT $2  [["id", 224], ["LIMIT", 1]]
[merit] no target found: Tried to load unspecified class: Vote. /Users/andrei/.frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/merit-4.0.3/lib/merit/base_target_finder.rb:12:in `find'
Completed 500  in 227ms (Views: 14.1ms | ActiveRecord: 55.5ms | Allocations: 122699)



NoMethodError - undefined method `direction' for 181:Integer
         vote.direction == 'down' && vote.voteable_type == 'Question'
             ^^^^^^^^^^:
  app/models/merit/point_rules.rb:24:in `block in initialize'

@tute
Copy link
Member

tute commented Nov 21, 2022

There we go! I cut the relevant snippet:

[merit] no target found: Tried to load unspecified class: Vote. /Users/andrei/.frum/versions/3.1.2/lib/ruby/gems/3.1.0/gems/merit-4.0.3/lib/merit/base_target_finder.rb:12:in find'`

I've never seen this one before, found the following (links to the CVE and a potential workraound): https://stackoverflow.com/questions/72970170/upgrading-to-rails-6-1-6-1-causes-psychdisallowedclass-tried-to-load-unspecif

Can you retry adding config.active_record.yaml_column_permitted_classes = [Symbol, Vote] to your config/application.rb file?

@tute
Copy link
Member

tute commented Nov 21, 2022

That would be one workaround, but the solution I think would be to refactor out the YAML.load call within merit.

@andreierdoss
Copy link
Author

andreierdoss commented Nov 22, 2022

I made the change as requested and then ran a test:

 web git:(master) ✗ rspec spec/features/votes_spec.rb

An error occurred while loading ./spec/features/votes_spec.rb.
Failure/Error: require File.expand_path('../../config/environment', __FILE__)

NameError:
  uninitialized constant Web::Application::Vote
# ./config/application.rb:21:in `<class:Application>'
# ./config/application.rb:11:in `<module:Web>'
# ./config/application.rb:9:in `<top (required)>'
# ./config/environment.rb:2:in `require_relative'
# ./config/environment.rb:2:in `<top (required)>'
# ./spec/rails_helper.rb:12:in `<top (required)>'
# ./spec/features/votes_spec.rb:1:in `<top (required)>'
No examples found.


Finished in 0.00019 seconds (files took 5.04 seconds to load)
0 examples, 0 failures, 1 error occurred outside of examples

@tute
Copy link
Member

tute commented Nov 23, 2022

Two questions:

  1. Did yaml_column_permitted_classes already appear in the file? If so, you might need to edit the previous one rather than add the line as I suggested
  2. Is Vote an ActiveRecord model?

@andreierdoss
Copy link
Author

  1. yaml_column_permitted_classes does appear in the file, but I just edited it to include Vote. Like this:
config.active_record.yaml_column_permitted_classes = [Symbol, Hash, Array, ActiveSupport::HashWithIndifferentAccess,  Vote]
  1. Yes, Vote is an ActiveRecord model

@tute
Copy link
Member

tute commented Nov 24, 2022

Can't reference models yet in config/application.rb. Anyhow it's not the real solution; we'll need to change how merit works for destroy to avoid that CVE and restriction. This is a bug, thanks for reporting.

@tute
Copy link
Member

tute commented Jul 6, 2024

Just skipped three related unit tests related to this bug: 08e65bd#diff-a2eb130f0d1ee0b07aa5562e4ae246cdaaa12ebcb16355e301c05b30b24828c8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants