fix potential issue when highlighting keyword #5526

nobodyiam · 2026-01-03T02:30:54Z

What's the purpose of this PR

fix potential issue when highlighting keyword

Follow this checklist to help us incorporate your contribution quickly and easily:

Read the Contributing Guide before making this pull request.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit tests to verify the code.
Run mvn clean test to make sure this pull request doesn't break anything.
Run mvn spotless:apply to format your code.
Update the CHANGES log.

Summary by CodeRabbit

Bug Fixes
- Improved search-result highlighting to safely handle special characters, null/empty inputs and malformed patterns.
- Now consistently escapes HTML in both matched and non-matched text to prevent injection and ensure stable display.
- Added robust error handling so highlighting gracefully falls back to plain, escaped text on failure.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-03T02:31:04Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Refactors highlightKeyword in GlobalSearchValueController.js to escape all output, introduce an escapeHtml helper, use a split-based highlighting approach with regex construction guarded by try/catch, and ensure safe fallback to escaped fulltext on errors (no raw HTML injection).

Changes

Cohort / File(s)	Summary
Global Search Highlighting `apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js`	Reworked `highlightKeyword(fulltext, keyword)` to normalize null input, escape HTML for all segments, safely construct/guard regex usage with try/catch, replace regex replace with split-based highlighting that wraps only matched segments in a highlight span, and return escaped fulltext on errors. Added internal `escapeHtml(text)` helper to escape `&`, `<`, `>`, `"` and `'`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I nibble code with careful paws,
Escaping crumbs and fixing flaws.
Keywords glow but never bite,
Safe and snug in HTML light.
🌿✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing a potential issue with keyword highlighting logic to prevent HTML injection vulnerabilities.

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR addresses potential security and stability issues in the keyword highlighting functionality by adding HTML escaping, regex special character escaping, and error handling.

Key changes:

Refactored highlightKeyword function to escape HTML entities and prevent XSS vulnerabilities
Added escapeHtml helper function to sanitize user input and matched text
Added buildRegex helper function with proper regex escaping and error handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-03T02:36:34Z

apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js

+        while ((match = regex.exec(fulltext)) !== null) {
+            hasMatch = true;
+            highlighted += escapeHtml(fulltext.substring(lastIndex, match.index));
+            highlighted += '<span class="highlight" style="background: yellow;padding: 1px 4px;">' + escapeHtml(match[0]) + '</span>';
+            lastIndex = match.index + match[0].length;
+        }


The regex execution loop may lead to an infinite loop if the regex matches an empty string. When a regex matches an empty string, match.index + match[0].length equals match.index, causing lastIndex to not advance. Consider adding a check to increment lastIndex by 1 when an empty match occurs to prevent the infinite loop.

The review suggestion is technically correct for general regular expression loops, but it is not necessary in this specific context.

Input Validation: The function highlightKeyword explicitly checks if (!keyword || keyword.length === 0) at line 265. This ensures the keyword is never an empty string.

Regex Construction: The buildRegex function (lines 307-317) escapes all special regex characters in the keyword. This means the generated RegExp will only match the literal characters of the keyword.

No Zero-Length Matches: Since the keyword is guaranteed to be non-empty and is treated as a literal string, any match found by regex.exec(fulltext) will have a length greater than 0 (match[0].length > 0).

Loop Termination: Because match[0].length is always positive, lastIndex will always increment, guaranteeing that the while loop will eventually terminate.

Therefore, while the infinite loop risk exists for complex or user-supplied regex patterns that can match empty strings (e.g., a*), it is impossible with the logic currently implemented in this file.

Copilot · 2026-01-03T02:36:34Z

apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js

+        let lastIndex = 0;
+        let highlighted = '';
+        let hasMatch = false;
+        let match;
+        while ((match = regex.exec(fulltext)) !== null) {
+            hasMatch = true;
+            highlighted += escapeHtml(fulltext.substring(lastIndex, match.index));
+            highlighted += '<span class="highlight" style="background: yellow;padding: 1px 4px;">' + escapeHtml(match[0]) + '</span>';
+            lastIndex = match.index + match[0].length;
+        }
+        highlighted += escapeHtml(fulltext.substring(lastIndex));
+        return hasMatch ? highlighted : safeFulltext;


The manual regex execution with while loop and state management adds complexity. Since JavaScript's String.replace() method with a function callback handles global replacements safely and automatically, consider simplifying this implementation by using fulltext.replace(regex, function(match) { return '<span class="highlight" style="background: yellow;padding: 1px 4px;">' + escapeHtml(match) + '</span>'; }). This would be more maintainable and avoid potential infinite loop issues.

apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (1)

283-300: Consider using an object map for cleaner character mapping.

The switch statement works correctly but could be simplified using an object literal for more concise code.

🔎 Optional refactor using object map

 function escapeHtml(text) {
-    return (text || '').replace(/[&<>"']/g, function (char) {
-        switch (char) {
-            case '&':
-                return '&amp;';
-            case '<':
-                return '&lt;';
-            case '>':
-                return '&gt;';
-            case '"':
-                return '&quot;';
-            case "'":
-                return '&#39;';
-            default:
-                return char;
-        }
-    });
+    const escapeMap = {
+        '&': '&amp;',
+        '<': '&lt;',
+        '>': '&gt;',
+        '"': '&quot;',
+        "'": '&#39;'
+    };
+    return (text || '').replace(/[&<>"']/g, function (char) {
+        return escapeMap[char] || char;
+    });
 }

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 19e03cd and 794350f.

📒 Files selected for processing (1)

apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: build (8)

🔇 Additional comments (1)

apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (1)

263-281: Excellent security-focused refactor!

The implementation correctly addresses XSS/HTML injection risks by:

Escaping all output (both matched and non-matched segments)

Using a split-based approach that preserves match boundaries while ensuring safe output

Adding robust error handling with try-catch for regex construction

The split-map pattern is the right choice here because alternatives like String.replace() cannot safely escape all text while preserving match positions.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jan 3, 2026

nobodyiam requested a review from Copilot January 3, 2026 02:34

Copilot started reviewing on behalf of nobodyiam January 3, 2026 02:34 View session

Copilot AI reviewed Jan 3, 2026

View reviewed changes

fix potential issue when highlighting keyword

794350f

nobodyiam force-pushed the fix-potential-issue branch from 19e03cd to 794350f Compare January 3, 2026 03:06

hound bot reviewed Jan 3, 2026

View reviewed changes

coderabbitai bot reviewed Jan 3, 2026

View reviewed changes

nobodyiam requested a review from Copilot January 3, 2026 03:16

Copilot started reviewing on behalf of nobodyiam January 3, 2026 03:17 View session

Copilot AI reviewed Jan 3, 2026

View reviewed changes

AnalogyC0de mentioned this pull request Jan 4, 2026

CVE Application: Stored HTML Injection via Search Highlighting Mechanism in Apollo Config Server AnalogyC0de/public_exp#1

Open

nobodyiam merged commit 55e09fe into apolloconfig:master Jan 11, 2026
15 checks passed

nobodyiam deleted the fix-potential-issue branch January 11, 2026 03:15

github-actions bot locked and limited conversation to collaborators Jan 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix potential issue when highlighting keyword #5526

fix potential issue when highlighting keyword #5526

nobodyiam commented Jan 3, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 3, 2026 •

edited

Loading

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

nobodyiam Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

fix potential issue when highlighting keyword #5526

fix potential issue when highlighting keyword #5526

Conversation

nobodyiam commented Jan 3, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's the purpose of this PR

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

nobodyiam Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

nobodyiam commented Jan 3, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 3, 2026 •

edited

Loading