The wrong content type of "text/html" is set for execute "@all" Ajax responses
Steps to reproduce:
-
Clone the wrong-content-type-ajax-excecute-all-reproducer project:
git clone https://github.com/stiemannkj1/wrong-content-type-ajax-excecute-all-reproducer.git -
Build the project:
cd wrong-content-type-ajax-excecute-all-reproducer && mvn clean package -
Deploy the project to Tomcat:
cp target/*.war $TOMCAT_HOME/webapps/wrong-content-type-ajax-excecute-all-reproducer.war -
Navigate to the deployed webapp at http://localhost:8080/wrong-content-type-ajax-excecute-all-reproducer/.
-
Note that the External Context Calls: show that
setResponseContentType("text/html")was correctly called before the first call togetResponseOutputWriter(). -
Click the Execute (default) Ajax Request button.
-
Note that the External Context Calls: show that
setResponseContentType("text/xml")was correctly called before the first call togetResponseOutputWriter(). -
Click the Execute @all Ajax Request button.
If the bug still exists, the External Context Calls: will show that setResponseContentType("text/html") was incorrectly called before the first call to getResponseOutputWriter().
If the bug is fixed, the External Context Calls: will show that setResponseContentType("text/xml") was called immediately before the first call to getResponseOutputWriter().
Additional Information:
This issue does not affect Servlets but it does affect portlets which do not allow changing the content type after the first call to ExternalContext.getResponseOutputWriter() (Portlet Spec 3.0 Section 15.5.1 Content Type):
The
setContentTypemethod must be called before thegetWriterorgetPortletOutputStreammethods. Otherwise, the method will have no effect.
A similar issue (but worse since it affects all Ajax responses) exists in MyFaces: https://issues.apache.org/jira/browse/MYFACES-4214.
To work around this issue in portlets, avoid using execute="@all".
This issue has been fix in 2.3.x and 2.2.x. It still needs to be fixed in 2.4.x/master, but there is a code freeze right now. Once master can be changed, this PR should be merged: https://github.com/javaserverfaces/mojarra/pull/4359
Once master can be changed, this PR should be merged: #4359
I'm not sure if that branch will ever be unfrozen, or directly be set to read-only/archived mode.
As Mojarra will be transferred to Eclipse all development will continue there. At a minimum there won't be any more releases from the repos here, as the final release has just been done: https://maven.java.net/content/repositories/releases/org/glassfish/javax.faces/2.4.0/
@arjantijms, thanks. My comment wasn't clear, but I meant that the commits could be merged into whatever version of Mojarra is given to the Eclipse foundation.
@stiemannkj1 If I set the execute attribute to "@all" my code is not working. That's correct? "@all" should have the same behavior like "@form", but that's not happening and I'm getting this error "java.lang.IllegalArgumentException: Unrecognized Content Type". This change was released in 2.3.4 and 2.3.5 but not in 2.4.0. Could you clarify this?
@Lynx12, it's possible that I broke something with this change, could you provide more details about your app and the stack trace for the IllegalArgumentException?
@Lynx12, perhaps you need to make a change similar to the one the joinfaces folks made here: https://github.com/joinfaces/joinfaces-maven-jar-example/pull/158
@stiemannkj1 the fix was easy to apply, but not so quick to detect because the template is added as a dependency. Thanks for guiding me to the solution.
After commits connected with this issue there is problems with <p:commandButton /> (tested on Primeface 4.0, but I think it is conneted with all versions).
<p:commandButton update=".." /> generates Ajax request that is handled as a partial request, so inside method RenderKitImpl::createResponseWriter we get contentType="text/html" instead of contentType="application/xhtml+xml" as it used to be causing e.g. HtmlResponseWriter incorrectly handling <![CDATA[...]]> in <script> tags.
@jacekzwroclawia, I've opened #4388 to track this issue.
I've reverted the commits that fix this issue since they caused so many issues, but that will cause the original issue to reappear. So if anyone wants to fix it in the future, here are the commits that fix #4358:
- https://github.com/javaserverfaces/mojarra/commit/8a5a562e129a31f3655de86cfeb79c33d3eaa1f8
- https://github.com/javaserverfaces/mojarra/commit/ee3671db95986395e3a40a591c52522421b25bf2
You can apply this patch on top of those commits in order to avoid #4388 (although you should probably also add a regression tester for #4358 to be sure):
diff --git a/jsf-ri/src/main/java/com/sun/faces/renderkit/html_basic/HtmlResponseWriter.java b/jsf-ri/src/main/java/com/sun/faces/renderkit/html_basic/HtmlResponseWriter.java
index f5198d7004..5ec2f26d96 100644
--- a/jsf-ri/src/main/java/com/sun/faces/renderkit/html_basic/HtmlResponseWriter.java
+++ b/jsf-ri/src/main/java/com/sun/faces/renderkit/html_basic/HtmlResponseWriter.java
@@ -55,6 +55,7 @@ import com.sun.faces.RIConstants;
import com.sun.faces.config.WebConfiguration;
import com.sun.faces.config.WebConfiguration.BooleanWebContextInitParameter;
import com.sun.faces.io.FastStringWriter;
+import com.sun.faces.renderkit.RenderKitUtils;
import com.sun.faces.util.HtmlUtils;
import com.sun.faces.util.MessageUtils;
import java.util.Map;
@@ -134,7 +135,7 @@ public class HtmlResponseWriter extends ResponseWriter {
private boolean isPartial;
// flag to indicate if the content type is XHTML
- private boolean isXhtml;
+ private boolean isXml;
// HtmlResponseWriter to use when buffering is required
private Writer origWriter;
@@ -478,8 +479,7 @@ public class HtmlResponseWriter extends ResponseWriter {
dontEscape = false;
}
- isXhtml = getContentType().equals(
- RIConstants.XHTML_CONTENT_TYPE);
+ isXml = RenderKitUtils.isXml(getContentType());
if (isScriptOrStyle(name)
&& !scriptOrStyleSrc
@@ -489,7 +489,7 @@ public class HtmlResponseWriter extends ResponseWriter {
if (result != null) {
String trim = result.trim();
- if (isXhtml) {
+ if (isXml) {
if (isScript) {
Matcher
cdataStartSlashSlash =
@@ -548,7 +548,7 @@ public class HtmlResponseWriter extends ResponseWriter {
}
}
}
- if (isXhtml) {
+ if (isXml) {
if (!writingCdata) {
if (isScript) {
writer.write("\n//]]>\n");
@@ -1130,9 +1130,8 @@ public class HtmlResponseWriter extends ResponseWriter {
writer.write('>');
closeStart = false;
if (isScriptOrStyle() && !scriptOrStyleSrc) {
- isXhtml = getContentType().equals(
- RIConstants.XHTML_CONTENT_TYPE);
- if (isXhtml) {
+ isXml = RenderKitUtils.isXml(getContentType());
+ if (isXml) {
if (!writingCdata) {
if (isScript) {
writer.write("\n//<![CDATA[\n");
I don't have any problem with JSF 2.3, what version will this change be released?
@wsaca, the change that caused regressions was only introduced int 2.2.17 so it only exists in 2.2.17 and 2.2.18. The change was never applied to 2.3.x, so no issues exist there.
@stiemannkj1 so when there will be new release of 2.2 version with reverted commits?
@jacekzwroclawia, that's up to Oracle.
cc @ren-zhijun-oracle, @ruolli, @xinyuan-zhang, @edburns, @manfredriem
@stiemannkj1 Hi, Kyle,
I just noticed you ever submitted the fix for #4358 to branch : "MOJARRA_2_3X_ROLLING" in repository: "https://github.com/javaserverfaces/mojarra.git" . Would you help revert the fix from "MOJARRA_2_3X_ROLLING" as you did for "MOJARRA_2_2X_ROLLING" ?
Thanks. Ruolin
@ruolli, I just reverted the commits from MOJARRA_2_3X_ROLLING.
@stiemannkj1 Thanks a lot, Kyle.
Do I understand it correctly?
- 2.3+ will not be published from this repository/by Donor, but rather by Donee, and changes pushed to MOJARRA_2_3X_ROLLING/master - if not ported - will not be visible for Clients of Mojarra 2.3.9+
- #4359 wasn't merged to master and thus fix was also not donated
@pzygielo 1 ... Yes, 2.3+ would be released from "github.com/eclipse-ee4j/mojarra" , instead of from this repository : "github.com/javaserverfaces/mojarra" . Meanwhile, legacy releases, including 2.3, would be released from here. Generally speaking, they are separate repositories, consequently, porting is necessary for certain fix to be merged from one to the other.
2... Seemingly, there was no submission via #4359.
Thanks. Ruolin
Still not 100% clear for me...
Yes, 2.3+ would be released from "github.com/eclipse-ee4j/mojarra" , instead of from this repository : "github.com/javaserverfaces/mojarra" . Meanwhile, legacy releases, including 2.3, would be released from here.
Eclipse released 2.3.9, by 2.3+ I meant ANY future 2.3 release. Shall it be 2.3.9+?
Do you - by legacy releases including 2.3 mean (for 2.3) patched 2.3.8 like 2.3.8-1 or something like that?
@pzygielo
By saying 2.3+, I actually meant Mojarra 2.4 or Mojarra 3.0 in future. Those releases would be released from "github.com/eclipse-ee4j/mojarra" . And, the artifact would be renamed to jakarta.faces , instead of javax.faces .
As to 2.3.* , they are maintained in "github.com/javaserverfaces/mojarra" "MOJARRA_2_3X_ROLLING" . And, 2.3.9 , 2.3.10 ... would be released from here.
Thanks. Ruolin
Thanks for your clarifications, @ruolli (I'm aware of groupId/artifactId renames, thank you.)
@pzygielo @ruolli @arjantijms
As far as I understand it (please confirm Arjan)
Oracle Mojarra supported releases will be coming from the github.com/javaserverfaces/mojarra for 2.3 and earlier.
Eclipse Mojarra supported and branded releases will be coming from github.com/eclipse-ee4j/mojarra for 2.3 and later