resolves #3254 fix crash when resolving ID from text and a candidate contains an unresolved xref (PR #3255)

- prevent nested calls to resolve_id from populating reftexts table
- only call resolve_id if fragment matches requirements (contains space or uppercase character)
- add additional tests to cover scenarios

4 files changed, 102 insertions, 8 deletions

diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index e4f5afb2..0c72c614 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -17,6 +17,7 @@ For a detailed view of what has changed, refer to the {uri-repo}/commits/master[
 
 Bug Fixes::
 
+  * fix crash when resolving ID from text and at least one candidate contains an unresolved xref (#3254)
   * fix compatibility with Rouge 2.0
 
 Improvements::
diff --git a/lib/asciidoctor/document.rb b/lib/asciidoctor/document.rb
index e8891bca..708e9dc5 100644
--- a/lib/asciidoctor/document.rb
+++ b/lib/asciidoctor/document.rb
@@ -331,8 +331,7 @@ class Document < AbstractBlock
       options[:standalone] = options[:header_footer] if (options.key? :header_footer) && !(options.key? :standalone)
     end
 
-    @parsed = false
-    @header = @header_attributes = nil
+    @parsed = @reftexts = @header = @header_attributes = nil
     @counters = {}
     @attributes_modified = ::Set.new
     @docinfo_processor_extensions = {}
@@ -615,16 +614,25 @@ class Document < AbstractBlock
     end
   end
 
-  # Public: Scan all registered references and return the ID of the reference that matches the specified reference text.
-  #
-  # If multiple references in the document have the same reference text, the first match in document order is used.
+  # Public: Scan registered references and return the ID of the first reference that matches the specified reference text.
   #
   # text - The String reference text to compare to the converted reference text of each registered reference.
   #
   # Returns the String ID of the first reference with matching reference text or nothing if no reference is found.
   def resolve_id text
-    ((@reftexts ||= @parsed ? {}.tap {|accum| @catalog[:refs].each {|id, ref| accum[ref.xreftext] = id } } : nil) ||
-      {}.tap {|accum| @catalog[:refs].find {|id, ref| ref.xreftext == text ? accum[text] = id : nil } })[text]
+    if @reftexts
+      @reftexts[text]
+    elsif @parsed
+      # @reftexts is set eagerly to prevent nested lazy init
+      (@reftexts = {}).tap {|accum| @catalog[:refs].each {|id, ref| accum[ref.xreftext] ||= id } }[text]
+    else
+      # @reftexts is set eagerly to prevent nested lazy init
+      resolved_id = nil
+      # NOTE short-circuit early since we're throwing away this table
+      (@reftexts = {}).tap {|accum| @catalog[:refs].each {|id, ref| (xreftext = ref.xreftext) == text ? (break (resolved_id = id)) : (accum[xreftext] ||= id) } }
+      @reftexts = nil
+      resolved_id
+    end
   end
 
   def footnotes?
diff --git a/lib/asciidoctor/substitutors.rb b/lib/asciidoctor/substitutors.rb
index 49e6c088..3799e237 100644
--- a/lib/asciidoctor/substitutors.rb
+++ b/lib/asciidoctor/substitutors.rb
@@ -807,7 +807,7 @@ module Substitutors
           refid, target = fragment, %(##{fragment})
         # handles: Node Title or Reference Text
         # do reverse lookup on fragment if not a known ID and resembles reftext (contains a space or uppercase char)
-        elsif (refid = doc.resolve_id fragment) && ((fragment.include? ' ') || fragment.downcase != fragment)
+        elsif ((fragment.include? ' ') || fragment.downcase != fragment) && (refid = doc.resolve_id fragment)
           fragment, target = refid, %(##{refid})
         else
           refid, target = fragment, %(##{fragment})
diff --git a/test/links_test.rb b/test/links_test.rb
index 139479d8..2418df11 100644
--- a/test/links_test.rb
+++ b/test/links_test.rb
@@ -874,6 +874,91 @@ context 'Links' do
     assert_xpath '//a[@href="#_section_b"][text()="Section B"]', output, 1
   end
 
+  test 'should not fail to resolve broken xref in title of block with ID' do
+    input = <<~'EOS'
+    [#p1]
+    .<<DNE>>
+    paragraph text
+    EOS
+
+    output = convert_string_to_embedded input
+    assert_xpath '//*[@class="title"]/a[@href="#DNE"][text()="[DNE]"]', output, 1
+  end
+
+  test 'should resolve forward xref in title of block with ID' do
+    input = <<~'EOS'
+    [#p1]
+    .<<conclusion>>
+    paragraph text
+
+    [#conclusion]
+    == Conclusion
+    EOS
+
+    output = convert_string_to_embedded input
+    assert_xpath '//*[@class="title"]/a[@href="#conclusion"][text()="Conclusion"]', output, 1
+  end
+
+  test 'should not fail to resolve broken xref in section title' do
+    input = <<~'EOS'
+    [#s1]
+    == <<DNE>>
+
+    == <<s1>>
+    EOS
+
+    # NOTE this output is nonsensical, but we still need to verify the scenario
+    output = convert_string_to_embedded input
+    assert_xpath '//a[@href="#DNE"][text()="[DNE]"]', output, 2
+  end
+
+  test 'should not resolve forward xref evaluated during parsing' do
+    input = <<~'EOS'
+    [#s1]
+    == <<forward>>
+
+    == <<s1>>
+
+    [#forward]
+    == Forward
+    EOS
+
+    output = convert_string_to_embedded input
+    assert_xpath '//a[@href="#forward"][text()="Forward"]', output, 0
+  end
+
+  test 'should not resolve forward natural xref evaluated during parsing' do
+    input = <<~'EOS'
+    :idprefix:
+
+    [#s1]
+    == <<Forward>>
+
+    == <<s1>>
+
+    == Forward
+    EOS
+
+    output = convert_string_to_embedded input
+    assert_xpath '//a[@href="#forward"][text()="Forward"]', output, 0
+  end
+
+  test 'should resolve first matching natural xref' do
+    input = <<~'EOS'
+    see <<Section Title>>
+
+    [#s1]
+    == Section Title
+
+    [#s2]
+    == Section Title
+    EOS
+
+    output = convert_string_to_embedded input
+    assert_xpath '//a[@href="#s1"]', output, 1
+    assert_xpath '//a[@href="#s1"][text()="Section Title"]', output, 1
+  end
+
   test 'anchor creates reference' do
     doc = document_from_string '[[tigers]]Tigers roam here.'
     ref = doc.catalog[:refs]['tigers']