Skip to content

[SITES-40889] [Core Components] Add support in Fragment component for CFVT#3015

Open
alexandru-stancioiu wants to merge 11 commits intomainfrom
sites_40889
Open

[SITES-40889] [Core Components] Add support in Fragment component for CFVT#3015
alexandru-stancioiu wants to merge 11 commits intomainfrom
sites_40889

Conversation

@alexandru-stancioiu
Copy link
Contributor

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Minor: New Feature?
Major: Breaking Change?
Tests Added + Pass? Yes
Documentation Provided Yes (code comments and or markdown)
Any Dependency Changes?
License Apache License, Version 2.0

@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

String templateId = request.getParameter("templateId");
String variation = request.getParameter("variation");
response.setContentType("text/html;charset=UTF-8");
response.getWriter().write(buildVcfHtml(fragmentId, templateId, variation));

Check warning

Code scanning / CodeQL

Cross-site scripting Medium test

Cross-site scripting vulnerability due to a
user-provided value
.
Cross-site scripting vulnerability due to a
user-provided value
.
Cross-site scripting vulnerability due to a
user-provided value
.

Copilot Autofix

AI 2 days ago

In general, to fix cross-site scripting in servlets, all user-controlled data that is written into HTML should be encoded using a well-established, context-appropriate escaping library (for example, OWASP Java Encoder or Apache Commons Text) rather than handcrafted string replacement. This ensures correct and complete escaping for the specific HTML context and makes static analysis tools recognize the data as properly sanitized.

For this file, the best fix is to replace the custom escapeHtml implementation with one that delegates to a standard encoding routine from a known library, without changing the public behavior of the servlet. We should also keep the method signature so the rest of the code remains unchanged. A practical choice, without modifying other imports, is to add an import for org.apache.commons.text.StringEscapeUtils and implement escapeHtml by calling StringEscapeUtils.escapeHtml4(input). That encoder correctly escapes characters necessary for safe inclusion in HTML, including both single and double quotes, and is widely recognized. The changes are limited to:

  • Adding an import at the top of MockVCFServlet.java for org.apache.commons.text.StringEscapeUtils.
  • Replacing the body of escapeHtml (lines 131–138) with a call to StringEscapeUtils.escapeHtml4(input) while preserving the null check and method signature.

No other methods or call sites need to change, since appendRow and buildVcfHtml already use escapeHtml consistently.


Suggested changeset 2
testing/it/it.core/src/main/java/com/adobe/cq/wcm/core/components/it/support/MockVCFServlet.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/testing/it/it.core/src/main/java/com/adobe/cq/wcm/core/components/it/support/MockVCFServlet.java b/testing/it/it.core/src/main/java/com/adobe/cq/wcm/core/components/it/support/MockVCFServlet.java
--- a/testing/it/it.core/src/main/java/com/adobe/cq/wcm/core/components/it/support/MockVCFServlet.java
+++ b/testing/it/it.core/src/main/java/com/adobe/cq/wcm/core/components/it/support/MockVCFServlet.java
@@ -24,6 +24,7 @@
 import org.apache.sling.api.servlets.SlingAllMethodsServlet;
 import org.apache.sling.servlets.annotations.SlingServletPaths;
 import org.osgi.service.component.annotations.Component;
+import org.apache.commons.text.StringEscapeUtils;
 
 /**
  * Local-development mock for the Content Fragment Visualization API.
@@ -132,10 +133,7 @@
         if (input == null) {
             return "";
         }
-        return input.replace("&", "&amp;")
-                     .replace("<", "&lt;")
-                     .replace(">", "&gt;")
-                     .replace("\"", "&quot;");
+        return StringEscapeUtils.escapeHtml4(input);
     }
 
     private static String escapeJson(String input) {
EOF
@@ -24,6 +24,7 @@
import org.apache.sling.api.servlets.SlingAllMethodsServlet;
import org.apache.sling.servlets.annotations.SlingServletPaths;
import org.osgi.service.component.annotations.Component;
import org.apache.commons.text.StringEscapeUtils;

/**
* Local-development mock for the Content Fragment Visualization API.
@@ -132,10 +133,7 @@
if (input == null) {
return "";
}
return input.replace("&", "&amp;")
.replace("<", "&lt;")
.replace(">", "&gt;")
.replace("\"", "&quot;");
return StringEscapeUtils.escapeHtml4(input);
}

private static String escapeJson(String input) {
testing/it/it.core/pom.xml
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/testing/it/it.core/pom.xml b/testing/it/it.core/pom.xml
--- a/testing/it/it.core/pom.xml
+++ b/testing/it/it.core/pom.xml
@@ -202,6 +202,11 @@
             <artifactId>org.apache.sling.servlets.annotations</artifactId>
             <version>1.2.6</version>
         </dependency>
-    </dependencies>
+        <dependency>
+        <groupId>org.apache.commons</groupId>
+        <artifactId>commons-text</artifactId>
+        <version>1.15.0</version>
+    </dependency>
+</dependencies>
 
 </project>
EOF
@@ -202,6 +202,11 @@
<artifactId>org.apache.sling.servlets.annotations</artifactId>
<version>1.2.6</version>
</dependency>
</dependencies>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-text</artifactId>
<version>1.15.0</version>
</dependency>
</dependencies>

</project>
This fix introduces these dependencies
Package Version Security advisories
org.apache.commons:commons-text (maven) 1.15.0 None
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Contributor

Choose a reason for hiding this comment

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

@alexandru-stancioiu , this may need to be fixed because it's likely getting deployed to AEM CS instances.

Copy link
Member

Choose a reason for hiding this comment

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

This is a mock servlet used for ITs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, since it is a mock servlet used for ITs, it won't get deployed to AEMaaCS

@LSantha
Copy link
Contributor

LSantha commented Mar 11, 2026

@alexandru-stancioiu , it would be nice to add an example if possible for this cool new feature in the examples subproject so that it would show up here https://www.aemcomponents.dev/content/core-components-examples/library/core-content/content-fragment.html . Then people can see it already shortly after the new release.
@vladbailescu , wdyt?

…ialog

The dynamically-populated Coral Select for VCF templates has no
server-side items, so Coral loses the stored JCR value on dialog load.
Read the stored vcfTemplate from the component resource via an async
fetch and use it as fallback when populating the dropdown.

Added Karma/Jasmine tests for VCF template retention scenarios.

Made-with: Cursor
@alexandru-stancioiu alexandru-stancioiu force-pushed the sites_40889 branch 3 times, most recently from 13cfd81 to 86df6e9 Compare March 13, 2026 06:55
Alexandru Marian Stancioiu and others added 2 commits March 13, 2026 08:55
Add a VCF display mode example to the Content Fragment examples page,
showing the component configured with displayMode=vcf and a template.
The visual preview requires AEM as a Cloud Service.

Made-with: Cursor
@sonarqubecloud
Copy link

@alexandru-stancioiu
Copy link
Contributor Author

Unfortunately we cannot make the visual preview on the aemcomponents.dev website work because we cannot serve any template client-side, it works only on AEMaaCS.
MockVCFServlet doesn't help either because it handles author-side dialog requests only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants