Skip to content

Normalize links in HTML subset formatting #190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
snoopdave merged 1 commit into from
Sep 17, 2026
+87 −10
Merged

Normalize links in HTML subset formatting #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions app/src/main/java/org/apache/roller/weblogger/util/Utilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ public class Utilities {
private static final Pattern CLOSING_A_TAG_PATTERN = Pattern.compile(
"</a>", Pattern.CASE_INSENSITIVE);
private static final Pattern OPENING_A_TAG_PATTERN = Pattern.compile(
"<a href=.*?>", Pattern.CASE_INSENSITIVE);
"<a\\s+href\\s*=.*?>", Pattern.CASE_INSENSITIVE | Pattern.DOTALL);
private static final Pattern A_HREF_PATTERN = Pattern.compile(
"&lt;a\\s+href\\s*=\\s*(?:\"([^\"]*)\"|'([^']*)'|([^\\s\"'=<>`]+))\\s*&gt;",
Comment thread
snoopdave marked this conversation as resolved.
Pattern.CASE_INSENSITIVE);
Comment thread
snoopdave marked this conversation as resolved.
private static final Pattern SUBSET_LINK_PATTERN = Pattern.compile(
"(?:https?://|mailto:)[^\\x00-\\x20\\x7f]+", Pattern.CASE_INSENSITIVE);
private static final Pattern QUOTE_PATTERN = Pattern.compile("&quot;",
Pattern.CASE_INSENSITIVE);

Expand Down Expand Up @@ -975,23 +980,39 @@ public static String transformToHTMLSubset(String s) {
s = replace(s, CLOSING_LI_TAG_PATTERN, "</li>");
s = replace(s, QUOTE_PATTERN, "\"");

// HTTP links
// Normalize supported links while retaining the surrounding text.
s = replace(s, CLOSING_A_TAG_PATTERN, "</a>");

@snoopdave snoopdave Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐞Claude Issue: Important: CLOSING_A_TAG_PATTERN rewrites &lt;/a&gt; unconditionally, but the opening tag is only rewritten when OPENING_A_TAG_PATTERN matches. Anchors it does not recognise therefore emit an unbalanced </a>:

<a>x</a>                              ->  &lt;a&gt;x</a>
<a class="c" href="https://x/">y</a>  ->  &lt;a class="c" href="https://x/"&gt;y</a>

This is pre-existing, but the loop is being rewritten here and already has an "unsupported anchor" output (<a>). Emitting that for unmatched opening tags as well would keep the markup balanced for a couple of lines of change.

@snoopdave snoopdave Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖Claude: fixed by broadening OPENING_A_TAG_PATTERN to &lt;a(?:\s[\s\S]*?)?&gt; so it matches every escaped opening anchor, not just ones starting <a href=. Each one now goes through the same reconstruction, so an anchor whose href is missing or unusable becomes <a> and the markup stays balanced:

<a>x</a>                              ->  <a>x</a>
<a name="anchor">x</a>                ->  <a>x</a>
<a class="c" href="https://x/">y</a>  ->  <a href="https://x/">y</a>

The pattern requires whitespace or the closing delimiter after a, so sibling tags are unaffected — <abbr> still passes through escaped. Covered by new test testHtmlSubsetBalancesUnsupportedAnchors.

Matcher m = OPENING_A_TAG_PATTERN.matcher(s);
StringBuilder result = new StringBuilder(s.length());
int end = 0;
while (m.find()) {
int start = m.start();
int end = m.end();
String link = s.substring(start, end);
link = "<" + link.substring(4, link.length() - 4) + ">";
s = s.substring(0, start) + link + s.substring(end, s.length());
m = OPENING_A_TAG_PATTERN.matcher(s);
result.append(restoreSubsetEntities(s.substring(end, m.start())));
Matcher hrefMatcher = A_HREF_PATTERN.matcher(m.group());
String link = "<a>";
if (hrefMatcher.matches()) {
String href = hrefMatcher.group(1);
if (href == null) {
href = hrefMatcher.group(2);
}
if (href == null) {
href = hrefMatcher.group(3);
}
href = StringEscapeUtils.unescapeHtml4(restoreSubsetEntities(href));
if (SUBSET_LINK_PATTERN.matcher(href).matches()) {
link = "<a href=\"" + StringEscapeUtils.escapeHtml4(href) + "\">";
}
}
result.append(link);
end = m.end();
}
result.append(restoreSubsetEntities(s.substring(end)));
return result.toString();
}

// escaped angle brackets
private static String restoreSubsetEntities(String s) {
s = s.replace("&amp;lt;", "&lt;");
s = s.replace("&amp;gt;", "&gt;");
s = s.replace("&amp;#", "&#");

return s;
}

Expand Down
56 changes: 56 additions & 0 deletions app/src/test/java/org/apache/roller/weblogger/util/UtilitiesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,70 @@
package org.apache.roller.weblogger.util;

import org.junit.jupiter.api.Test;
import org.apache.roller.weblogger.business.plugins.comment.HTMLSubsetPlugin;
import org.apache.roller.weblogger.pojos.WeblogEntryComment;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

/**
* Test utilities.
*/
public class UtilitiesTest {

@Test
public void testHtmlSubsetLinkFormats() {
assertEquals("<a href=\"https://example.com/\">site</a>",
htmlSubset("<a href='https://example.com/'>site</a>"));
assertEquals("<a href=\"HTTP://example.com/\">site</a>",
htmlSubset("<A HREF=HTTP://example.com/>site</A>"));
assertEquals("<a href=\"mailto:reader@example.com\">mail</a>",
htmlSubset("<a href=\"mailto:reader@example.com\">mail</a>"));
assertEquals("<a href=\"https://example.com/?a=1&amp;b=2\">query</a>",
htmlSubset("<a href=\"https://example.com/?a=1&b=2\">query</a>"));
assertEquals("<a href=\"https://example.com/\">site</a>",
htmlSubset("<a\n href = 'https://example.com/' >site</a>"));
assertEquals("<a href=\"https://example.com/~reader\">site</a>",
htmlSubset("<a href=\"https://example.com/&#126;reader\">site</a>"));
}

@Test
public void testHtmlSubsetLinkSubset() {
assertEquals("<a>file</a>", htmlSubset("<a href=\"ftp://example.com/file\">file</a>"));
assertEquals("<a>local</a>", htmlSubset("<a href=\"../page\">local</a>"));
assertEquals("<a>site</a>",
htmlSubset("<a href=\"https://example.com/\" title=\"site\">site</a>"));
assertEquals("<a>empty</a>", htmlSubset("<a href=\"\">empty</a>"));
}

@Test
public void testHtmlSubsetRetainsTextAndFormatting() {
assertNull(Utilities.transformToHTMLSubset(null));
assertEquals("", htmlSubset(""));
String text = "<p><b>Bold</b> and <i>italic</i><br>line</p>";
assertEquals("<p><b>Bold</b> and <i>italic</i><br />line</p>", htmlSubset(text));
assertEquals("&#65; &lt;example&gt;", htmlSubset("&#65; &lt;example&gt;"));
assertEquals("&lt;a href=\"https://example.com/\"&gt;site&lt;/a&gt;",
htmlSubset("&lt;a href=\"https://example.com/\"&gt;site&lt;/a&gt;"));
assertEquals("<a href=\"https://example.com/$1\">one</a> <a>two</a>",
htmlSubset("<a href=\"https://example.com/$1\">one</a> <a href=\"ftp://example.com/\">two</a>"));
}

@Test
public void testHtmlSubsetPluginUsesLinkFormatting() {
WeblogEntryComment comment = new WeblogEntryComment();
comment.setContentType("text/html");
HTMLSubsetPlugin plugin = new HTMLSubsetPlugin();
String text = "<a href=\"ftp://example.com/\">file</a>";
assertEquals("<a>file</a>", plugin.render(comment, text));
comment.setContentType("text/plain");
assertEquals(text, plugin.render(comment, text));
}

private String htmlSubset(String text) {
return Utilities.transformToHTMLSubset(Utilities.escapeHTML(text));
}

@Test
public void testExtractHTML() {
String test = "<a>keep me</a>";
Expand Down
Loading