Fix email obfuscator by skipping emails in HTML attributes #51

Copilot · 2025-10-15T16:27:56Z

Fix email obfuscator by skipping emails in HTML attributes

Problem

The email obfuscator was incorrectly recognizing retina image filenames (like [email protected], [email protected]) as email addresses and obfuscating them, which broke image references in HTML.

Solution

Per @skerbis feedback: Excludes ALL emails within HTML attribute values (src, data-*, href, value, title, etc.).

Latest Improvements

Performance optimization: Both methods use precomputed tag ranges approach (O(n+m) instead of O(n*m) complexity)
Robustness: Added checks for $_SERVER['REQUEST_METHOD'] and $_POST to avoid PHP notices in CLI contexts
Consistency: Added whitelist checking to makeEmailClickable() for consistent behavior across all obfuscation methods
Bug fixes: Removed redundant variables, restored retina pattern check for edge cases

Key Changes

Both methods now use precomputed HTML tag ranges for better performance
Safer $_SERVER access with isset() checks
Consistent whitelist handling across unicorn and clickable methods
Handles edge cases like < characters within attribute values

Benefits

Feature	Pattern Exclusion	Attribute Exclusion (This PR)
Fixes retina images	✅	✅
Fixes emails in form inputs	❌	✅
Fixes emails in data attributes	❌	✅
Fixes emails in ANY attributes	❌	✅
Performance optimized	❌	✅
Handles edge cases	❌	✅
Consistent whitelist support	❌	✅

Testing

✅ All 16 test cases passed:

Retina images preserved in attributes
Emails in attributes preserved
Emails in text correctly obfuscated
Edge case with < in attributes handled correctly
Both unicorn and clickable methods working correctly
Whitelist functionality consistent across both methods

Fixes #50

Original prompt

This section details on the original issue you should resolve

<issue_title>"Erkennt" [email protected] als E-Mail</issue_title>
<issue_description>Der Obfuscator erkennt [email protected] als E-Mail-Adresse und macht dies draus:

<img src="/Logo<span class=unicorn><span>_at_</span></span>2x.png" alt="Logo" class="img-fluid" loading="lazy">

REDAXO | 5.17.1
PHP: 8.1.33
E-Mail Obfuscator 3.0.0

Methode: ROT13 Einhorn-Markup (Standard, benötigt JQuery)</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #50

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: skerbis <[email protected]>

skerbis · 2025-10-15T16:50:38Z

Wäre es nicht besser src Attribute einfach zu ignorieren? Oder andere die unüblich für email Adressen sind ?

skerbis · 2025-10-15T16:52:19Z

@copilot Wäre es nicht besser src Attribute einfach zu ignorieren? Oder andere die unüblich für email Adressen sind ? Führe Tests durch und zeige die Ergebnisse

Co-authored-by: skerbis <[email protected]>

Copilot · 2025-10-15T17:01:57Z

@copilot Wäre es nicht besser src Attribute einfach zu ignorieren? Oder andere die unüblich für email Adressen sind ? Führe Tests durch und zeige die Ergebnisse

You're absolutely right! I've updated the fix to skip emails in HTML attributes (src, data-*, value, etc.) instead of just excluding @nx patterns.

Test results (11/11 passing):

✅ Retina images: [email protected], [email protected] - preserved
✅ Emails in attributes: data-email, input values, title - preserved
✅ Emails in text: paragraphs, divs, plain text - obfuscated correctly

This approach is more robust and prevents future issues with emails in any HTML attributes.

Commit: 5b69478

Copilot

Pull Request Overview

This PR fixes a bug where the email obfuscator incorrectly identified retina image filenames (like [email protected]) as email addresses and obfuscated them within HTML attributes, breaking image references. The solution implements attribute-aware detection that skips obfuscation for any email-like patterns within HTML attribute values (src, data-*, href, etc.), addressing the root cause rather than just the symptom.

Key changes:

Added obfuscateEmailsNotInAttributes() method with quote-counting logic to detect when patterns appear inside HTML attributes
Updated makeEmailClickable() with the same attribute-awareness and explicit retina pattern exclusion
Replaced simple regex replacement with position-aware iteration to properly handle context

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

lib/EmailObfuscator.php

Copilot · 2025-11-19T11:48:15Z

lib/EmailObfuscator.php

+		while (preg_match($pattern, $ret, $matches, PREG_OFFSET_CAPTURE, $offset)) {
+			$fullMatch = $matches[0][0];
+			$pos = $matches[0][1];
+			$email = $matches[2][0] . '@' . $matches[3][0];
+
+			// Skip retina image patterns like @2x.png, @3x.jpg, etc.
+			if (preg_match('/^[^@]+@\d+x\./i', $email)) {
+				$offset = $pos + strlen($fullMatch);
+				continue;
+			}
+
+			// Check if we're inside an HTML attribute value
+			$before = substr($ret, 0, $pos);
+			$lastTagStart = strrpos($before, '<');
+			$lastTagEnd = strrpos($before, '>');
+
+			$shouldMakeClickable = true;
+
+			// If we found a < after the last >, we're potentially inside a tag
+			if ($lastTagStart !== false && ($lastTagEnd === false || $lastTagStart > $lastTagEnd)) {
+				// We're inside a tag, check if we're inside quotes (attribute value)
+				$tagContent = substr($before, $lastTagStart);
+
+				// Count quotes - if odd, we're inside an attribute value
+				$doubleQuotes = substr_count($tagContent, '"');
+				$singleQuotes = substr_count($tagContent, "'");
+
+				if (($doubleQuotes % 2) == 1 || ($singleQuotes % 2) == 1) {
+					$shouldMakeClickable = false;
+				}
+			}
+
+			if ($shouldMakeClickable) {
+				// Make clickable
+				$replacement = $matches[1][0] . "<a href=\"mailto:$email\">$email</a>";
+				$ret = substr_replace($ret, $replacement, $pos, strlen($fullMatch));
+				$offset = $pos + strlen($replacement);
+			} else {
+				// Skip this match
+				$offset = $pos + strlen($fullMatch);
+			}
+		}


[nitpick] Similar performance concern as in the obfuscateEmailsNotInAttributes method: repeated string operations for each match can be inefficient with many emails in large documents. The repeated substr, strrpos, and substr_count calls scale as O(n*m).

Consider the same optimization suggestion: a single-pass parsing approach that identifies both tag boundaries and email patterns simultaneously.

@copilot apply changes based on this feedback

Applied performance optimization to both methods. The makeEmailClickable() now uses the same precomputed tag ranges approach as obfuscateEmailsNotInAttributes(), reducing complexity from O(n*m) to O(n+m). Also added isset() checks for $_SERVER['REQUEST_METHOD'] to prevent PHP notices in CLI contexts. All tests passing.

Commit: 8584636

Copilot · 2025-11-19T11:48:15Z

lib/EmailObfuscator.php

+			$before = substr($ret, 0, $pos);
+			$lastTagStart = strrpos($before, '<');
+			$lastTagEnd = strrpos($before, '>');


Same issue as lines 171-173: the logic doesn't account for < characters within attribute values. In <div data-compare="a < b">[email protected]</div>, the strrpos($before, '<') would incorrectly find the < inside the attribute value, causing the email in the text content to be incorrectly skipped.

Consider using a more robust approach to identify actual HTML tag boundaries.

Copilot · 2025-11-19T11:48:15Z

lib/EmailObfuscator.php

 	private static function makeEmailClickable($ret) {
 		$ret = ' ' . $ret;
-		// in testing, using arrays here was found to be faster
-		$ret = preg_replace_callback('#([\s>])([.0-9a-z_+-]+)@(([0-9a-z-]+\.)+[0-9a-z]{2,})#i', 'emailobfuscator::make_email_clickable_callback', $ret);
+
+		// Process emails but skip those in HTML attributes
+		$pattern = '#([\s>])([.0-9a-z_+-]+)@(([0-9a-z-]+\.)+[0-9a-z]{2,})#i';
+		$offset = 0;
+
+		while (preg_match($pattern, $ret, $matches, PREG_OFFSET_CAPTURE, $offset)) {
+			$fullMatch = $matches[0][0];
+			$pos = $matches[0][1];
+			$email = $matches[2][0] . '@' . $matches[3][0];
+
+			// Skip retina image patterns like @2x.png, @3x.jpg, etc.
+			if (preg_match('/^[^@]+@\d+x\./i', $email)) {
+				$offset = $pos + strlen($fullMatch);
+				continue;
+			}
+
+			// Check if we're inside an HTML attribute value
+			$before = substr($ret, 0, $pos);
+			$lastTagStart = strrpos($before, '<');
+			$lastTagEnd = strrpos($before, '>');
+
+			$shouldMakeClickable = true;
+
+			// If we found a < after the last >, we're potentially inside a tag
+			if ($lastTagStart !== false && ($lastTagEnd === false || $lastTagStart > $lastTagEnd)) {
+				// We're inside a tag, check if we're inside quotes (attribute value)
+				$tagContent = substr($before, $lastTagStart);
+
+				// Count quotes - if odd, we're inside an attribute value
+				$doubleQuotes = substr_count($tagContent, '"');
+				$singleQuotes = substr_count($tagContent, "'");
+
+				if (($doubleQuotes % 2) == 1 || ($singleQuotes % 2) == 1) {
+					$shouldMakeClickable = false;
+				}
+			}
+
+			if ($shouldMakeClickable) {
+				// Make clickable
+				$replacement = $matches[1][0] . "<a href=\"mailto:$email\">$email</a>";
+				$ret = substr_replace($ret, $replacement, $pos, strlen($fullMatch));
+				$offset = $pos + strlen($replacement);
+			} else {
+				// Skip this match
+				$offset = $pos + strlen($fullMatch);
+			}
+		}


[nitpick] The makeEmailClickable method doesn't check the whitelist before making emails clickable, while obfuscateEmailsNotInAttributes does check the whitelist (line 195). This creates an inconsistency where whitelisted emails might still be wrapped in <a> tags by this method when using methods other than 'rot13_unicorn'.

Consider adding whitelist checking here for consistency, or document why whitelist checking is only needed for the unicorn obfuscation method.

Copilot · 2025-11-19T11:48:16Z

lib/EmailObfuscator.php

+	 * @return string Processed content
+	 */
+	private static function obfuscateEmailsNotInAttributes($content) {
+		$pattern = '/(?<![\/\w])([\w\-\+\.]+)@([\w\-\.]+\.[\w]{2,})(?![\w\/])/';


[nitpick] The regex pattern uses negative lookahead/lookbehind to avoid matching emails that are part of URLs or other contexts ((?<![\/\w]) and (?![\w\/])). However, this pattern would still match file extensions like [email protected] in contexts where they're not within HTML attributes.

While the attribute detection logic should catch most cases, this regex could match false positives in text content like "Check out our [email protected] file". Consider if this is intended behavior or if the pattern should be more restrictive to avoid matching obvious file patterns even in text content.

Suggested change

$pattern = '/(?<![\/\w])([\w\-\+\.]+)@([\w\-\.]+\.[\w]{2,})(?![\w\/])/';

// Avoid matching emails that are actually filenames like [email protected]

// Add negative lookahead for common file extensions after the email

$pattern = '/(?<![\/\w])([\w\-\+\.]+)@([\w\-\.]+\.[\w]{2,})(?![\w\/])(?!(\.(png|jpg|jpeg|gif|svg|webp|pdf|docx?|xlsx?|pptx?|zip|rar|tar|gz|mp3|mp4|avi|mov|wmv|flv|mkv|ico|bmp|tiff|psd|ai|eps|csv|json|xml|yml|yaml|txt|log|md|html|htm|php|js|css|scss|less|c|cpp|h|hpp|py|rb|go|rs|sh|bat|exe|dll|bin|dat|bak|tmp)))/i';

Copilot · 2025-11-19T11:48:16Z

lib/EmailObfuscator.php

+				// If odd number of quotes, we're inside an attribute value
+				if (($doubleQuotes % 2) == 1 || ($singleQuotes % 2) == 1) {


The quote counting logic has a subtle bug when handling mixed quotes within attribute values. If an attribute value contains both single and double quotes (e.g., title="It's a test"), the logic checks if EITHER quote type has an odd count using OR (||). This would incorrectly conclude we're inside an attribute when we might not be.

The logic should use AND (&&) - we're only truly inside an attribute value if we have an odd count of the quote type that opened that specific attribute. However, detecting which quote type opened the attribute requires more sophisticated parsing.

A more robust approach: Check if we have an odd total of EITHER single OR double quotes (but not both conditions independently).

Suggested change

// If odd number of quotes, we're inside an attribute value

if (($doubleQuotes % 2) == 1 || ($singleQuotes % 2) == 1) {

// If odd total number of quotes, we're inside an attribute value

if ((($doubleQuotes + $singleQuotes) % 2) == 1) {

Co-authored-by: Copilot <[email protected]>

skerbis · 2025-11-19T15:10:35Z

@copilot apply changes based on the comments in this thread

…lidation Co-authored-by: skerbis <[email protected]>

Co-authored-by: skerbis <[email protected]>

Copilot · 2025-11-19T15:22:36Z

@copilot apply changes based on the comments in this thread

Applied the remaining actionable improvement from the review thread: Added whitelist checking to makeEmailClickable() for consistency with obfuscateEmailsNotInAttributes().

This ensures whitelisted emails are handled consistently across both obfuscation methods. All 16 tests passing including 6 new whitelist-specific tests.

Commit: 28fce10

Initial plan

92d6373

Copilot AI assigned Copilot and skerbis Oct 15, 2025

Copilot started work on behalf of skerbis October 15, 2025 16:29 View session

Fix email obfuscator to exclude retina image filenames (@2x, @3x, etc)

aeb18db

Co-authored-by: skerbis <[email protected]>

Copilot AI changed the title ~~[WIP] Fix obfuscator misinterpreting logo as email~~ Fix email obfuscator incorrectly matching retina image filenames Oct 15, 2025

Copilot AI requested a review from skerbis October 15, 2025 16:43

Copilot finished work on behalf of skerbis October 15, 2025 16:43

Copilot started work on behalf of skerbis October 15, 2025 16:53 View session

Improve fix: skip emails in HTML attributes instead of just @nx patterns

5b69478

Co-authored-by: skerbis <[email protected]>

Copilot AI changed the title ~~Fix email obfuscator incorrectly matching retina image filenames~~ Fix email obfuscator by skipping emails in HTML attributes Oct 15, 2025

Copilot finished work on behalf of skerbis October 15, 2025 17:04

skerbis requested review from eaCe and removed request for skerbis October 15, 2025 17:06

skerbis marked this pull request as ready for review November 19, 2025 11:42

Copilot AI review requested due to automatic review settings November 19, 2025 11:42

Copilot started reviewing on behalf of skerbis November 19, 2025 11:42 View session

Copilot finished reviewing on behalf of skerbis November 19, 2025 11:45

Copilot AI reviewed Nov 19, 2025

View reviewed changes

skerbis and others added 5 commits November 19, 2025 16:06

Update lib/EmailObfuscator.php

03254ab

Co-authored-by: Copilot <[email protected]>

Update lib/EmailObfuscator.php

a791cc4

Co-authored-by: Copilot <[email protected]>

Update lib/EmailObfuscator.php

5be9ef0

Co-authored-by: Copilot <[email protected]>

Update lib/EmailObfuscator.php

27824de

Co-authored-by: Copilot <[email protected]>

Update lib/EmailObfuscator.php

1515fb8

Co-authored-by: Copilot <[email protected]>

Copilot started work on behalf of skerbis November 19, 2025 15:10 View session

Apply performance optimization and fix $_SERVER check in whitelist va…

8584636

…lidation Co-authored-by: skerbis <[email protected]>

Copilot AI requested a review from skerbis November 19, 2025 15:18

Copilot finished work on behalf of skerbis November 19, 2025 15:18

Copilot started work on behalf of skerbis November 19, 2025 15:19 View session

Add whitelist checking to makeEmailClickable for consistency

28fce10

Co-authored-by: skerbis <[email protected]>

Copilot finished work on behalf of skerbis November 19, 2025 15:25

-		$pattern = '/(?<![\/\w])([\w\-\+\.]+)@([\w\-\.]+\.[\w]{2,})(?![\w\/])/';
+		// Avoid matching emails that are actually filenames like [email protected]
+		// Add negative lookahead for common file extensions after the email
+		$pattern = '/(?<![\/\w])([\w\-\+\.]+)@([\w\-\.]+\.[\w]{2,})(?![\w\/])(?!(\.(png|jpg|jpeg|gif|svg|webp|pdf|docx?|xlsx?|pptx?|zip|rar|tar|gz|mp3|mp4|avi|mov|wmv|flv|mkv|ico|bmp|tiff|psd|ai|eps|csv|json|xml|yml|yaml|txt|log|md|html|htm|php|js|css|scss|less|c|cpp|h|hpp|py|rb|go|rs|sh|bat|exe|dll|bin|dat|bak|tmp)))/i';

		// If odd number of quotes, we're inside an attribute value
		if (($doubleQuotes % 2) == 1 \|\| ($singleQuotes % 2) == 1) {

Fix email obfuscator by skipping emails in HTML attributes #51

Are you sure you want to change the base?

Fix email obfuscator by skipping emails in HTML attributes #51

Uh oh!

Conversation

Copilot AI commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Latest Improvements

Key Changes

Benefits

Testing

Comments on the Issue (you are @copilot in this section)

Uh oh!

skerbis commented Oct 15, 2025

Uh oh!

skerbis commented Oct 15, 2025

Uh oh!

Copilot AI commented Oct 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

skerbis Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

skerbis commented Nov 19, 2025

Uh oh!

Copilot AI commented Nov 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Oct 15, 2025 •

edited

Loading