snoopdave commented on code in PR #228:
URL: https://github.com/apache/roller/pull/228#discussion_r4406000129
##########
app/src/main/java/org/apache/roller/weblogger/util/Utilities.java:
##########
@@ -183,10 +183,20 @@ public static String removeHTML(String str, boolean
addSpace) {
start = endTag + 1;
beginTag = str.indexOf('<', start);
}
- // if no endTag found, get rest of str and break
+ // if no endTag found, drop the rest if this '<' starts a tag,
+ // which a browser would still parse as markup; otherwise keep the
+ // '<' as text and look at the next one
else {
- ret.append(str.substring(beginTag));
- break;
+ if (startsTag(str, beginTag)) {
Review Comment:
🤖Claude: fixed by narrowing the fallback. An unclosed `<` now drops the rest
only when it opens a comment, declaration or end tag (`<!`, `<?`, `</`), or a
tag name followed by whitespace or `/` with an `=` later on. So `x<y`,
`List<String`, `C<C++ fans` and `a<b and b<c` are kept, and `text <img src=x`
is still dropped. The CHANGES.md entry now says which text is kept. (Local
change, in the next push.)
##########
app/src/test/java/org/apache/roller/weblogger/util/UtilitiesTest.java:
##########
@@ -99,6 +99,29 @@ public void testRemoveHTML() {
assertEquals(expect, result);
}
+ @Test
+ public void testRemoveHTMLUnclosedTag() {
+ assertEquals("about", Utilities.removeHTML("about <img src=x", false));
+ assertEquals("about", Utilities.removeHTML("about <p>", false).trim());
+ assertEquals("a", Utilities.removeHTML("a </p", false));
+ assertEquals("a", Utilities.removeHTML("a <!-- note", false));
+ assertEquals("", Utilities.removeHTML("<img src=x", false));
+ }
+
+ @Test
+ public void testRemoveHTMLKeepsLessThanText() {
+ assertEquals("a < b", Utilities.removeHTML("a < b", false));
+ assertEquals("1 <2", Utilities.removeHTML("1 <2", false));
+ assertEquals("x <", Utilities.removeHTML("x <", false));
+ }
Review Comment:
🤖Claude: fixed by adding `testRemoveHTMLDefaultAddsSpaceOnlyForTags`, which
uses the default `removeHTML(str)` overload, and by adding `x<y`,
`List<String`, `C<C++ fans` and `a<b and b<c` to the `addSpace=false` cases.
The overload change is now explicit: a `<` kept as text no longer gets a space
added before it (`removeHTML("x<y")` returns `x<y`; it used to return `x <y`).
Real tags still become a space (`a<br>b` gives `a b`). (Local change, in the
next push.)
--
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]
