snoopdave commented on code in PR #190:
URL: https://github.com/apache/roller/pull/190#discussion_r4040981809
##########
app/src/main/java/org/apache/roller/weblogger/util/Utilities.java:
##########
@@ -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(
+
"<a\\s+href\\s*=\\s*(?:\"([^\"]*)\"|'([^']*)'|([^\\s\"'=<>`]+))\\s*>",
Review Comment:
🤖Claude: fixed by allowing `=` in unquoted href values. `A_HREF_PATTERN` now
runs against the tag's interior (the text between the escaped `<a` and
`>`) with `find()` instead of against the whole tag with `matches()`, so the
escaped `>` terminator is no longer inside the search space and cannot be
swallowed by a greedy unquoted value. The class is now `[^\s"'<>`]+`,
terminating on whitespace or end of attributes. `<a href=https://ex.com/?a=1>`
and `<a href=https://ex.com/?a=1&b=2>` both render as links again; added
assertions for both in `testHtmlSubsetLinkFormats`.
##########
app/src/main/java/org/apache/roller/weblogger/util/Utilities.java:
##########
@@ -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(
+
"<a\\s+href\\s*=\\s*(?:\"([^\"]*)\"|'([^']*)'|([^\\s\"'=<>`]+))\\s*>",
+ Pattern.CASE_INSENSITIVE);
Review Comment:
🤖Claude: fixed by matching the href as an attribute rather than requiring
the whole tag to be exactly `<a href=X>`. `A_HREF_PATTERN` is now
`(?:^|\s)href\s*=\s*(...)` applied with `find()` to the tag's attribute text,
so attributes before and after the href are tolerated and discarded. All three
cases render as links again, and `href` no longer has to come first:
```
<a href="https://example.com/"; target="_blank">x</a> -> <a
href="https://example.com/";>x</a>
<a href='https://example.com/' title='site'>x</a> -> <a
href="https://example.com/";>x</a>
<a class="external" href="https://example.com/";>x</a> -> <a
href="https://example.com/";>x</a>
```
This stays safe because the output anchor is still reconstructed from the
validated href and never echoes the input tag — `<a href="https://example.com/";
onclick="void(0)">` yields `<a href="https://example.com/";>` with the handler
dropped, and `javascript:`/`data:` hrefs still degrade to `<a>`. New test
`testHtmlSubsetLinkKeepsHrefAmongOtherAttributes` covers these; the `title`
case in `testHtmlSubsetLinkSubset` was re-pointed at an `ftp://` href so it
still asserts scheme rejection rather than attribute rejection.
##########
app/src/main/java/org/apache/roller/weblogger/util/Utilities.java:
##########
@@ -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>");
Review Comment:
🤖Claude: fixed by broadening `OPENING_A_TAG_PATTERN` to
`<a(?:\s[\s\S]*?)?>` 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`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
