mirror of
https://github.com/opf/openproject.git
synced 2026-06-14 03:30:14 +00:00
Reject semantic-shaped ids in HashSeparator#applicable?
The matcher regex now accepts both numeric and semantic identifier shapes (`\d+|PROJ-\d+`) on a single alternation branch shared by the bare WP handler and the prefixed-resource handler. Without this guard, `version#PROJ-1` reaches HashSeparator with `oid = "PROJ-1".to_i = 0`, which is `present?`, so it issues `Version.find_by(id: 0)` — a guaranteed miss against the wrong table. The same shape applies to every prefix HashSeparator handles (document, message, meeting, project, user, group, view), turning a single pasted comment into one wasted SELECT per prefix. Gating `applicable?` on `numeric_id?(matcher.identifier)` short-circuits the handler before any DB lookup; the regex still matches and the text falls through to literal output. Regression test asserts zero `FROM "versions"` SELECTs for `version#PROJ-1`.
This commit is contained in:
@@ -39,8 +39,11 @@ module OpenProject::TextFormatting::Matchers
|
||||
# Hash-separated object links
|
||||
# Condition: Separator is '#'
|
||||
# Condition: Prefix is present, checked to be one of the allowed values
|
||||
# Condition: Identifier is numeric — prefixed resources address rows by
|
||||
# primary key, so a semantic-shaped identifier (PROJ-1) would `to_i` to 0
|
||||
# and run a guaranteed-miss `find_by(id: 0)`.
|
||||
def applicable?
|
||||
matcher.sep == "#" && valid_prefix? && oid.present?
|
||||
matcher.sep == "#" && valid_prefix? && WorkPackage::SemanticIdentifier.numeric_id?(matcher.identifier)
|
||||
end
|
||||
|
||||
# Examples:
|
||||
|
||||
@@ -270,4 +270,31 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages
|
||||
"classic mode added unexpected WP SELECTs for semantic input:\n#{wp_selects.join("\n")}"
|
||||
end
|
||||
end
|
||||
|
||||
describe "prefixed resource refs with semantic-shaped identifiers",
|
||||
with_flag: { semantic_work_package_ids: true },
|
||||
with_settings: { work_packages_identifier: "semantic" } do
|
||||
# `version#PROJ-1`, `message#PROJ-1`, etc. share the regex with the WP
|
||||
# macro because `\d+|PROJ-\d+` is a single alternation. The prefixed
|
||||
# branches address tables keyed by numeric primary key, so a semantic
|
||||
# identifier paired with a prefix must short-circuit before the handler
|
||||
# would otherwise issue `find_by(id: 0)` (the round-trip of any
|
||||
# non-numeric string through `to_i`).
|
||||
include_context "with author signed in"
|
||||
let(:project) { create(:project, identifier: "MACROPROJ") }
|
||||
|
||||
it "does not query the prefixed resource table for `version#PROJ-1`" do
|
||||
project
|
||||
author
|
||||
|
||||
rendered = nil
|
||||
recorder = ActiveRecord::QueryRecorder.new { rendered = format_text("see version#PROJ-1 here") }
|
||||
version_selects = recorder.log.grep(/FROM "versions"/i)
|
||||
|
||||
expect(version_selects).to be_empty,
|
||||
"expected zero versions SELECTs for semantic-shaped input, got:\n" \
|
||||
"#{version_selects.join("\n")}"
|
||||
expect(rendered).to include("version#PROJ-1")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user