mirror of
https://github.com/discourse/discourse.git
synced 2026-03-05 15:27:34 +08:00
The ONP diff algorithm could run for an extremely long time on pathological inputs (e.g. fully replaced or random content), causing request timeouts and high CPU usage. The previous safeguard (`p >= 1000`) did not reliably bound work. Replace it with an explicit comparison budget that caps the number of element comparisons in the inner `snake` loop. The budget scales linearly with input size (default factor: 200× combined length, ceiling: 2 000 000 comparisons). When exceeded, a new `DiffLimitExceeded` exception is raised. Handle the new exception gracefully at every call site: - PostsController#revisions / #latest_revision → 422 with message - StaffActionLogsController#diff → inline fallback message - PostRevisor#compute_diff_size → treat as Infinity (forces new version) - discourse-ai UpdateArtifact → fallback diff block Also adds a pathology benchmark script and comprehensive test coverage for all affected paths. Co-authored-by: Sam Saffron <sam.saffron@gmail.com>
131 lines
4.1 KiB
Ruby
131 lines
4.1 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
class Admin::StaffActionLogsController < Admin::StaffController
|
|
INDEX_LIMIT = 200
|
|
|
|
def index
|
|
filters = params.slice(*UserHistory.staff_filters + %i[page limit], :start_date, :end_date)
|
|
|
|
page = (params[:page] || 0).to_i
|
|
page_size = fetch_limit_from_params(default: INDEX_LIMIT, max: INDEX_LIMIT)
|
|
|
|
staff_action_logs = UserHistory.staff_action_records(current_user, filters)
|
|
|
|
count = staff_action_logs.count
|
|
staff_action_logs = staff_action_logs.offset(page * page_size).limit(page_size).to_a
|
|
|
|
load_more_params = params.permit(UserHistory.staff_filters)
|
|
load_more_params.merge!(page: page + 1, page_size: page_size)
|
|
|
|
render_json_dump(
|
|
staff_action_logs: serialize_data(staff_action_logs, UserHistorySerializer),
|
|
total_rows_staff_action_logs: count,
|
|
load_more_staff_action_logs: admin_staff_action_logs_path(load_more_params),
|
|
extras: {
|
|
user_history_actions: staff_available_actions,
|
|
},
|
|
)
|
|
end
|
|
|
|
def diff
|
|
@history = UserHistory.find(params[:id])
|
|
raise Discourse::NotFound unless guardian.can_see_staff_action_log?(@history)
|
|
raise Discourse::NotFound unless guardian.can_see_staff_action_log_content?(@history)
|
|
|
|
prev = @history.previous_value
|
|
cur = @history.new_value
|
|
|
|
prev = JSON.parse(prev) if prev
|
|
cur = JSON.parse(cur) if cur
|
|
|
|
output = +"<h2>#{CGI.escapeHTML(cur&.dig("name").to_s)}</h2><p></p>"
|
|
|
|
diff_fields = {}
|
|
diff_fields["name"] = { prev: prev&.dig("name").to_s, cur: cur&.dig("name").to_s }
|
|
|
|
case @history.action
|
|
when *%i[change_theme delete_theme].map { |i| UserHistory.actions[i] }
|
|
diff_theme(diff_fields, prev, cur)
|
|
when *%i[tag_group_create tag_group_destroy tag_group_change].map { |i| UserHistory.actions[i] }
|
|
diff_tag_group(diff_fields, prev, cur)
|
|
end
|
|
|
|
diff_fields.delete_if { |k, v| v[:cur] == v[:prev] }
|
|
|
|
diff_fields.each do |k, v|
|
|
output << "<h3>#{k}</h3><p></p>"
|
|
begin
|
|
diff = DiscourseDiff.new(v[:prev] || "", v[:cur] || "")
|
|
output << diff.side_by_side_markdown
|
|
rescue ONPDiff::DiffLimitExceeded
|
|
output << "<p>#{CGI.escapeHTML(I18n.t("errors.diff_too_complex"))}</p>"
|
|
end
|
|
end
|
|
|
|
render json: { side_by_side: output }
|
|
end
|
|
|
|
protected
|
|
|
|
def child_themes(theme)
|
|
return "" unless children = theme&.dig("child_themes")
|
|
|
|
children.map { |row| row["name"] }.join(" ").to_s
|
|
end
|
|
|
|
def load_diff(hash, key, val)
|
|
if f = val&.dig("theme_fields")
|
|
f.each do |row|
|
|
entry = hash[row["target"] + " " + row["name"]] ||= {}
|
|
entry[key] = row["value"]
|
|
end
|
|
end
|
|
end
|
|
|
|
private
|
|
|
|
def staff_available_actions
|
|
UserHistory.staff_actions.sort.map do |name|
|
|
{ id: name, action_id: UserHistory.actions[name] || UserHistory.actions[:custom_staff] }
|
|
end
|
|
end
|
|
|
|
def diff_theme(diff_fields, prev, cur)
|
|
%w[default user_selectable].each do |f|
|
|
diff_fields[f] = { prev: (!!prev&.dig(f)).to_s, cur: (!!cur&.dig(f)).to_s }
|
|
end
|
|
|
|
diff_fields["color scheme"] = {
|
|
prev: prev&.dig("color_scheme", "name").to_s,
|
|
cur: cur&.dig("color_scheme", "name").to_s,
|
|
}
|
|
|
|
diff_fields["included themes"] = { prev: child_themes(prev), cur: child_themes(cur) }
|
|
|
|
load_diff(diff_fields, :cur, cur)
|
|
load_diff(diff_fields, :prev, prev)
|
|
end
|
|
|
|
def diff_tag_group(diff_fields, prev, cur)
|
|
%w[one_per_topic permissions].each do |f|
|
|
diff_fields[f] = { prev: prev&.dig(f).to_s, cur: cur&.dig(f).to_s }
|
|
end
|
|
add_tag_diff(diff_fields, prev, cur, "tags", "tag_names")
|
|
add_tag_diff(diff_fields, prev, cur, "parent_tag", "parent_tag_name")
|
|
end
|
|
|
|
def add_tag_diff(diff_fields, prev, cur, new_key, old_key)
|
|
prev_key = prev&.key?(new_key) ? new_key : old_key
|
|
cur_key = cur&.key?(new_key) ? new_key : old_key
|
|
field_name = cur&.key?(new_key) || prev&.key?(new_key) ? new_key : old_key
|
|
diff_fields[field_name] = {
|
|
prev: extract_tag_names(prev&.dig(prev_key)),
|
|
cur: extract_tag_names(cur&.dig(cur_key)),
|
|
}
|
|
end
|
|
|
|
def extract_tag_names(tags)
|
|
return nil if tags.blank?
|
|
tags.map { |t| t.is_a?(Hash) ? t["name"] : t }.join("\n")
|
|
end
|
|
end
|