Hash the URI as part of the hyperlink ID

src/buffer/out/textBuffer.cpp

@@ -2279,32 +2279,35 @@ std::wstring TextBuffer::GetHyperlinkUriFromId(uint16_t id) const
 // - The user-defined id
 // Return value:
 // - The internal hyperlink ID
-uint16_t TextBuffer::GetHyperlinkId(std::wstring_view params)
+uint16_t TextBuffer::GetHyperlinkId(std::wstring_view uri, std::wstring_view id)
 {
-    uint16_t id = 0;
-    if (params.empty())
+    uint16_t numericId = 0;
+    if (id.empty())
     {
         // no custom id specified, return our internal count
-        id = _currentHyperlinkId;
+        numericId = _currentHyperlinkId;
         ++_currentHyperlinkId;
     }
     else
     {
         // assign _currentHyperlinkId if the custom id does not already exist
-        const auto result = _hyperlinkCustomIdMap.emplace(params, _currentHyperlinkId);
+        std::wstring newId{ id };
+        // hash the URL and add it to the custom ID - GH#7698
+        newId += L"%" + std::to_wstring(std::hash<std::wstring_view>{}(uri));
+        const auto result = _hyperlinkCustomIdMap.emplace(newId, _currentHyperlinkId);
         if (result.second)
         {
             // the custom id did not already exist
             ++_currentHyperlinkId;
         }
-        id = (*(result.first)).second;
+        numericId = (*(result.first)).second;
     }
     // _currentHyperlinkId could overflow, make sure its not 0
     if (_currentHyperlinkId == 0)
     {
         ++_currentHyperlinkId;
     }
-    return id;
+    return numericId;
 }
 
 // Method Description:

src/buffer/out/textBuffer.hpp

@@ -143,7 +143,7 @@ class TextBuffer final
 
     void AddHyperlinkToMap(std::wstring_view uri, uint16_t id);
     std::wstring GetHyperlinkUriFromId(uint16_t id) const;
-    uint16_t GetHyperlinkId(std::wstring_view params);
+    uint16_t GetHyperlinkId(std::wstring_view uri, std::wstring_view id);
     void RemoveHyperlinkFromMap(uint16_t id);
     std::wstring GetCustomIdFromId(uint16_t id) const;
     void CopyHyperlinkMaps(const TextBuffer& OtherBuffer);

src/cascadia/TerminalCore/TerminalApi.cpp

@@ -574,7 +574,7 @@ CATCH_LOG_RETURN_FALSE()
 bool Terminal::AddHyperlink(std::wstring_view uri, std::wstring_view params) noexcept
 {
     auto attr = _buffer->GetCurrentAttributes();
-    const auto id = _buffer->GetHyperlinkId(params);
+    const auto id = _buffer->GetHyperlinkId(uri, params);
     attr.SetHyperlinkId(id);
     _buffer->SetCurrentAttributes(attr);
     _buffer->AddHyperlinkToMap(uri, id);

src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp

@@ -37,6 +37,7 @@ namespace TerminalCoreUnitTests
 
         TEST_METHOD(AddHyperlink);
         TEST_METHOD(AddHyperlinkCustomId);
+        TEST_METHOD(AddHyperlinkCustomIdDifferentUri);
     };
 };
 
@@ -296,15 +297,45 @@ void TerminalCoreUnitTests::TerminalApiTest::AddHyperlinkCustomId()
     stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c");
     VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
     VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
-    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
 
     // Send any other text
     stateMachine.ProcessString(L"Hello World");
     VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
     VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
-    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
 
     // Process the closing osc 8 sequences
     stateMachine.ProcessString(L"\x1b]8;;\x9c");
     VERIFY_IS_FALSE(tbi.GetCurrentAttributes().IsHyperlink());
 }
+
+void TerminalCoreUnitTests::TerminalApiTest::AddHyperlinkCustomIdDifferentUri()
+{
+    // This is a nearly literal copy-paste of ScreenBufferTests::TestAddHyperlinkCustomId, adapted for the Terminal
+
+    Terminal term;
+    DummyRenderTarget emptyRT;
+    term.Create({ 100, 100 }, 0, emptyRT);
+
+    auto& tbi = *(term._buffer);
+    auto& stateMachine = *(term._stateMachine);
+
+    // Process the opening osc 8 sequence
+    stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c");
+    VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
+
+    const auto oldAttributes{ tbi.GetCurrentAttributes() };
+
+    // Send any other text
+    stateMachine.ProcessString(L"\x1b]8;id=myId;other.url\x9c");
+    VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"other.url");
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"other.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
+
+    // This second URL should not change the URL of the original ID!
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(oldAttributes.GetHyperlinkId()), L"test.url");
+    VERIFY_ARE_NOT_EQUAL(oldAttributes.GetHyperlinkId(), tbi.GetCurrentAttributes().GetHyperlinkId());
+}

src/host/getset.cpp

@@ -1567,7 +1567,7 @@ void DoSrvAddHyperlink(SCREEN_INFORMATION& screenInfo,
                        const std::wstring_view params)
 {
     auto attr = screenInfo.GetAttributes();
-    const auto id = screenInfo.GetTextBuffer().GetHyperlinkId(params);
+    const auto id = screenInfo.GetTextBuffer().GetHyperlinkId(uri, params);
     attr.SetHyperlinkId(id);
     screenInfo.GetTextBuffer().SetCurrentAttributes(attr);
     screenInfo.GetTextBuffer().AddHyperlinkToMap(uri, id);

src/host/ut_host/ScreenBufferTests.cpp

@@ -213,6 +213,7 @@ class ScreenBufferTests
 
     TEST_METHOD(TestAddHyperlink);
     TEST_METHOD(TestAddHyperlinkCustomId);
+    TEST_METHOD(TestAddHyperlinkCustomIdDifferentUri);
 
     TEST_METHOD(UpdateVirtualBottomWhenCursorMovesBelowIt);
 
@@ -5956,19 +5957,46 @@ void ScreenBufferTests::TestAddHyperlinkCustomId()
     stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c");
     VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
     VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
-    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
 
     // Send any other text
     stateMachine.ProcessString(L"Hello World");
     VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
     VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
-    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
 
     // Process the closing osc 8 sequences
     stateMachine.ProcessString(L"\x1b]8;;\x9c");
     VERIFY_IS_FALSE(tbi.GetCurrentAttributes().IsHyperlink());
 }
 
+void ScreenBufferTests::TestAddHyperlinkCustomIdDifferentUri()
+{
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& tbi = si.GetTextBuffer();
+    auto& stateMachine = si.GetStateMachine();
+
+    // Process the opening osc 8 sequence with a custom id
+    stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c");
+    VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
+
+    const auto oldAttributes{ tbi.GetCurrentAttributes() };
+
+    // Send any other text
+    stateMachine.ProcessString(L"\x1b]8;id=myId;other.url\x9c");
+    VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"other.url");
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"other.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
+
+    // This second URL should not change the URL of the original ID!
+    VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(oldAttributes.GetHyperlinkId()), L"test.url");
+    VERIFY_ARE_NOT_EQUAL(oldAttributes.GetHyperlinkId(), tbi.GetCurrentAttributes().GetHyperlinkId());
+}
+
 void ScreenBufferTests::UpdateVirtualBottomWhenCursorMovesBelowIt()
 {
     auto& g = ServiceLocator::LocateGlobals();

src/host/ut_host/TextBufferTests.cpp

@@ -2464,30 +2464,33 @@ void TextBufferTests::HyperlinkTrim()
 
     // Set a hyperlink id in the first row and add a hyperlink to our map
     const COORD pos{ 70, 0 };
-    const auto id = _buffer->GetHyperlinkId(customId);
+    const auto id = _buffer->GetHyperlinkId(url, customId);
     TextAttribute newAttr{ 0x7f };
     newAttr.SetHyperlinkId(id);
     _buffer->GetRowByOffset(pos.Y).GetAttrRow().SetAttrToEnd(pos.X, newAttr);
     _buffer->AddHyperlinkToMap(url, id);
 
     // Set a different hyperlink id somewhere else in the buffer
     const COORD otherPos{ 70, 5 };
-    const auto otherId = _buffer->GetHyperlinkId(otherCustomId);
+    const auto otherId = _buffer->GetHyperlinkId(otherUrl, otherCustomId);
     newAttr.SetHyperlinkId(otherId);
     _buffer->GetRowByOffset(otherPos.Y).GetAttrRow().SetAttrToEnd(otherPos.X, newAttr);
     _buffer->AddHyperlinkToMap(otherUrl, otherId);
 
     // Increment the circular buffer
     _buffer->IncrementCircularBuffer();
 
+    const auto finalCustomId = fmt::format(L"{}%{}", customId, std::hash<std::wstring_view>{}(url));
+    const auto finalOtherCustomId = fmt::format(L"{}%{}", otherCustomId, std::hash<std::wstring_view>{}(otherUrl));
+
     // The hyperlink reference that was only in the first row should be deleted from the map
     VERIFY_ARE_EQUAL(_buffer->_hyperlinkMap.find(id), _buffer->_hyperlinkMap.end());
     // Since there was a custom id, that should be deleted as well
-    VERIFY_ARE_EQUAL(_buffer->_hyperlinkCustomIdMap.find(customId), _buffer->_hyperlinkCustomIdMap.end());
+    VERIFY_ARE_EQUAL(_buffer->_hyperlinkCustomIdMap.find(finalCustomId), _buffer->_hyperlinkCustomIdMap.end());
 
     // The other hyperlink reference should not be deleted
     VERIFY_ARE_EQUAL(_buffer->_hyperlinkMap[otherId], otherUrl);
-    VERIFY_ARE_EQUAL(_buffer->_hyperlinkCustomIdMap[otherCustomId], otherId);
+    VERIFY_ARE_EQUAL(_buffer->_hyperlinkCustomIdMap[finalOtherCustomId], otherId);
 }
 
 // This tests that when we increment the circular buffer, non-obsolete hyperlink references
@@ -2505,7 +2508,7 @@ void TextBufferTests::NoHyperlinkTrim()
 
     // Set a hyperlink id in the first row and add a hyperlink to our map
     const COORD pos{ 70, 0 };
-    const auto id = _buffer->GetHyperlinkId(customId);
+    const auto id = _buffer->GetHyperlinkId(url, customId);
     TextAttribute newAttr{ 0x7f };
     newAttr.SetHyperlinkId(id);
     _buffer->GetRowByOffset(pos.Y).GetAttrRow().SetAttrToEnd(pos.X, newAttr);
@@ -2518,7 +2521,9 @@ void TextBufferTests::NoHyperlinkTrim()
     // Increment the circular buffer
     _buffer->IncrementCircularBuffer();
 
+    const auto finalCustomId = fmt::format(L"{}%{}", customId, std::hash<std::wstring_view>{}(url));
+
     // The hyperlink reference should not be deleted from the map since it is still present in the buffer
     VERIFY_ARE_EQUAL(_buffer->GetHyperlinkUriFromId(id), url);
-    VERIFY_ARE_EQUAL(_buffer->_hyperlinkCustomIdMap[customId], id);
+    VERIFY_ARE_EQUAL(_buffer->_hyperlinkCustomIdMap[finalCustomId], id);
 }