yaml_column_permitted_classes warning never actually gets output

[ajvondrak]

Thank you for your contribution!

Due to limited volunteers, issues that do not follow these instructions will be
closed without comment.

Check the following boxes:

• This is not a usage question, this is a bug report
• This bug can be reproduced with the script I provide below
• This bug can be reproduced in the latest release of the paper_trail gem

Due to limited volunteers, we cannot answer usage questions. Please ask such
questions on StackOverflow.

Bug reports must use the following template:

# frozen_string_literal: true

# Use this template to report PaperTrail bugs.
# Please include only the minimum code necessary to reproduce your issue.
require "bundler/inline"

# STEP ONE: What versions are you using?
gemfile(true) do
  ruby "3.2.2"
  source "https://rubygems.org"
  gem "activerecord", "7.1.3"
  gem "minitest", "5.22.0"
  gem "paper_trail", "15.1.0", require: false
  gem "sqlite3", "1.7.2"
end

require "active_record"
require "minitest/autorun"
require "logger"

# Please use sqlite for your bug reports, if possible.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = nil
ActiveRecord::Schema.define do
  # STEP TWO: Define your tables here.
  create_table :users, force: true do |t|
    t.text :first_name, null: false
    t.timestamps null: false
  end

  create_table :versions do |t|
    t.string :item_type, null: false
    t.integer :item_id, null: false
    t.string :event, null: false
    t.string :whodunnit
    t.text :object, limit: 1_073_741_823
    t.text :object_changes, limit: 1_073_741_823
    t.datetime :created_at
  end
  add_index :versions, %i[item_type item_id]
end
ActiveRecord::Base.logger = Logger.new(STDOUT)
require "paper_trail"

# STEP FOUR: Define your AR models here.
class User < ActiveRecord::Base
  has_paper_trail
end

# STEP FIVE: Please write a test that demonstrates your issue.
class BugTest < ActiveSupport::TestCase
  def test_paper_trail_logs_psych_disallowed_class
    user = User.create(first_name: 'bug') # tacitly changes created_at + updated_at
    refute_includes ActiveRecord.yaml_column_permitted_classes, Time
    assert_output(nil, PaperTrail::Version::E_YAML_PERMITTED_CLASSES % "Tried to load unspecified class: Time\n") do
      user.versions.last.changeset
    end
  end
end

# STEP SIX: Run this script using `ruby my_bug_report.rb`

$ ruby repro.rb
Fetching gem metadata from https://rubygems.org/...........
Resolving dependencies...
Using concurrent-ruby 1.2.3
Using mutex_m 0.2.0
Using timeout 0.4.1
Using bundler 2.4.10
Using rack 3.0.9
Using sqlite3 1.7.2 (x86_64-darwin)
Using base64 0.2.0
Using bigdecimal 3.1.6
Using connection_pool 2.4.1
Using i18n 1.14.1
Using tzinfo 2.0.6
Using request_store 1.5.1
Using ruby2_keywords 0.0.5
Using minitest 5.22.0
Using drb 2.2.0
Using activesupport 7.1.3
Using activemodel 7.1.3
Using activerecord 7.1.3
Using paper_trail 15.1.0
-- create_table(:users, {:force=>true})
   -> 0.0125s
-- create_table(:versions)
   -> 0.0002s
-- add_index(:versions, [:item_type, :item_id])
   -> 0.0002s
Run options: --seed 3621

# Running:

D, [2024-02-05T14:52:59.103328 #15132] DEBUG -- :   TRANSACTION (0.0ms)  begin transaction
D, [2024-02-05T14:52:59.103546 #15132] DEBUG -- :   User Create (0.3ms)  INSERT INTO "users" ("first_name", "created_at", "updated_at") VALUES (?, ?, ?) RETURNING "id"  [["first_name", "bug"], ["created_at", "2024-02-05 22:52:59.102705"], ["updated_at", "2024-02-05 22:52:59.102705"]]
D, [2024-02-05T14:52:59.111426 #15132] DEBUG -- :   PaperTrail::Version Create (0.1ms)  INSERT INTO "versions" ("item_type", "item_id", "event", "object_changes", "created_at") VALUES (?, ?, ?, ?, ?) RETURNING "id"  [["item_type", "User"], ["item_id", 1], ["event", "create"], ["object_changes", "---\nid:\n-\n- 1\nfirst_name:\n-\n- bug\ncreated_at:\n-\n- &1 2024-02-05 22:52:59.102705000 Z\nupdated_at:\n-\n- *1\n"], ["created_at", "2024-02-05 22:52:59.102705"]]
D, [2024-02-05T14:52:59.117680 #15132] DEBUG -- :   TRANSACTION (0.0ms)  commit transaction
D, [2024-02-05T14:52:59.120752 #15132] DEBUG -- :   PaperTrail::Version Load (0.3ms)  SELECT "versions".* FROM "versions" WHERE "versions"."item_id" = ? AND "versions"."item_type" = ? ORDER BY "versions"."created_at" DESC, "versions"."id" DESC LIMIT ?  [["item_id", 1], ["item_type", "User"], ["LIMIT", 1]]
D, [2024-02-05T14:52:59.122141 #15132] DEBUG -- :   User Load (0.0ms)  SELECT "users".* FROM "users" WHERE "users"."id" = ? LIMIT ?  [["id", 1], ["LIMIT", 1]]
F

Finished in 0.072451s, 13.8024 runs/s, 41.4073 assertions/s.

  1) Failure:
BugTest#test_paper_trail_logs_psych_disallowed_class [repro.rb:55]:
In stderr.
--- expected
+++ actual
@@ -1,2 +1 @@
-"PaperTrail encountered a Psych::DisallowedClass error during deserialization of YAML column, indicating that yaml_column_permitted_classes has not been configured correctly. Tried to load unspecified class: Time
-"
+""


1 runs, 3 assertions, 1 failures, 0 errors, 0 skips

This issue is because the error raised is an instance of Psych::DisallowedClass, but the check around the warning is for e.instance_of?(::Psych::Exception):

paper_trail/lib/paper_trail/version_concern.rb

Lines 358 to 360 in 47dbc22

	if defined?(::Psych::Exception) && e.instance_of?(::Psych::Exception)
	::Kernel.warn format(E_YAML_PERMITTED_CLASSES, e)
	end

instance_of? will only respond true when the object is an instance of the exact class; what we really want is is_a? which will also respond true for any subclasses: https://stackoverflow.com/a/3893305

#1397 only tests a stub that raises a Psych::Exception directly, hence why this was never caught. If I update my local installation of the paper_trail gem to use is_a? instead of instance_of?, the bug report script above passes.

[ajvondrak]

Looks like this would be fixed by #1451.

[github-actions]

This issue has been automatically marked as stale due to inactivity.
The resources of our volunteers are limited.
Bug reports must provide a script that reproduces the bug, using our template. Feature suggestions must include a promise to build the feature yourself.
Thank you for all your contributions.