Skip to content

Commit

Permalink
Add missing class and allow extending serializable classes on logs
Browse files Browse the repository at this point in the history
`Spree::LogEntry` loads details from a YAML string that can potentially
contain any serialized Ruby object if users have overridden some part of
the codebase.

New Psych (YAML parser) version packed with Ruby 3.1 requires that, by
default, users are explicit about which type of classes are allowed to
be loaded (see ruby/psych@cb50aa8 and ruby/psych@1764942).

On 008168c, we updated our code to allow instances of
`ActiveMerchant::Billing::Response`. However, as [noted in a
comment](solidusio@008168c#r73483078),
it seems we're missing, at least, instances of
`ActiveSupport::TimeWithZone`.

We need to add all classes used internally and leave a door open for
users to add their own classes (via a new `log_entry_permitted_classes`
preference).
  • Loading branch information
waiting-for-dev committed May 19, 2022
1 parent 093fad1 commit 1f7442f
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 1 deletion.
45 changes: 44 additions & 1 deletion core/app/models/spree/log_entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,53 @@

module Spree
class LogEntry < Spree::Base
# Classes used in core that can be present in serialized details
#
# Users can add their own classes in
# `Spree::Config#log_entry_permitted_classes`.
#
# @see Spree::AppConfiguration#log_entry_permitted_classes
CORE_PERMITTED_CLASSES = [
ActiveMerchant::Billing::Response,
ActiveSupport::TimeWithZone,
Time,
ActiveSupport::TimeZone
].freeze

# Raised when a disallowed class is tried to be loaded
class DisallowedClass < RuntimeError
attr_reader :psych_exception

def initialize(psych_exception:)
@psych_exception = psych_exception
super(default_message)
end

private

def default_message
<<~MSG
#{psych_exception.message}
You can specify custom classes to be loaded in config/initializers/spree.rb. E.g:
Spree.config do |config|
config.log_entry_permitted_classes = ['MyClass']
end
MSG
end
end

def self.permitted_classes
CORE_PERMITTED_CLASSES + Spree::Config.log_entry_permitted_classes.map(&:constantize)
end

belongs_to :source, polymorphic: true, optional: true

def parsed_details
@details ||= YAML.safe_load(details, permitted_classes: [ActiveMerchant::Billing::Response])
@details ||= YAML.safe_load(details, permitted_classes: self.class.permitted_classes)
rescue Psych::DisallowedClass => e
raise DisallowedClass.new(psych_exception: e)
end
end
end
9 changes: 9 additions & 0 deletions core/lib/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,15 @@ class AppConfiguration < Preferences::Configuration
# @return [String] URL of logo used on frontend (default: +'logo/solidus.svg'+)
preference :logo, :string, default: 'logo/solidus.svg'

# @!attribute [rw] log_entry_permitted_classes
# @return [Array<String>] An array of extra classes that are allowed to be
# loaded from a serialized YAML as details in {Spree::LogEntry}
# (defaults to a non-frozen empty array, so that extensions can add
# their own classes).
# @example
# config.log_entry_permitted_classes = ['Date']
preference :log_entry_permitted_classes, :array, default: []

# @!attribute [rw] mails_from
# @return [String] Email address used as +From:+ field in transactional emails.
preference :mails_from, :string, default: '[email protected]'
Expand Down
37 changes: 37 additions & 0 deletions core/spec/models/spree/log_entry_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe Spree::LogEntry, type: :model do
describe '#parsed_details' do
it 'can parse ActiveMerchant::Billing::Response instances' do
response = ActiveMerchant::Billing::Response.new('success', 'message')

log_entry = described_class.new(details: response.to_yaml)

expect { log_entry.parsed_details }.not_to raise_error
end

it 'can parse ActiveSupport::TimeWithZone instances' do
time = Time.zone.now

log_entry = described_class.new(details: time.to_yaml)

expect { log_entry.parsed_details }.not_to raise_error
end

it 'can parse user specified classes instances' do
stub_spree_preferences(log_entry_permitted_classes: ['Date'])

log_entry = described_class.new(details: Date.today)

expect { log_entry.parsed_details }.not_to raise_error
end

it 'raises a meaningful exception when a disallowed class is found' do
log_entry = described_class.new(details: Date.today)

expect { log_entry.parsed_details }.to raise_error(described_class::DisallowedClass, /log_entry_permitted_classes/)
end
end
end

0 comments on commit 1f7442f

Please sign in to comment.