Skip to content
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.

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

Store response encoding during initial request. #5542

Closed

Conversation

TomasHofman
Copy link
Contributor

@TomasHofman TomasHofman commented Jan 14, 2025

This is a proposal to resolve #5543

Version 4.0.6 introduces a change (fault?) in behavior, which breaks character encoding detection under some circumstances. I think the change was introduced in #5385.

Originally, the FaceletViewHandlingStrategy had this line (sessionMap.put(CHARACTER_ENCODING_KEY, encoding);), which stored the response encoding into the external context session. The stored encoding could then be taken into account during next request in the ViewHandler:

charEnc = (String) extContext.getSessionMap().get(CHARACTER_ENCODING_KEY);

Storing the response encoding is I think in line with spec section "2.5.2.2. Determining the Character Encoding" (https://jakarta.ee/specifications/faces/4.0/jakarta-faces-4.0#determining-the-character-encoding), which mentions:

At the end of the render-response phase, the ViewHandler must store the response character encoding used by the underlying response object (e.g., the Jakarta Servlet or Portlet response) in the session (if and only if a session already exists) under a well known, implementation-dependent key.

The mentioned PR added the storing of the encoding to ViewHandler:

https://github.com/eclipse-ee4j/mojarra/pull/5385/files#diff-6903369dd9e9cf64f15a5126c3d098577e81cb1dc05c760c6586d827966268beR235

but that IMO is not the right place in the lifecycle, or at least it doesn't have the same effect. In my case it's not executed as the encoding at that point is null.

The reproducer for this is a page containing a simple form:

        <h:form>
		<h:inputText value="#{testBean.text}"/>
		<h:commandButton value="click here" />
		<h:outputText value="#{testBean.text}"/>
	</h:form>

When you enter an accented character into the input and press the button, the output field should display the character. With the version 4.0.6+ the encoding gets mangled, character is not displayed correctly.

This reverts to behavior from 4.0.4. The calculated response session
encoding is stored in the session so it can be taken into account at
next request.

See spec section "2.5.2.2. Determining the Character Encoding"

https://jakarta.ee/specifications/faces/4.0/jakarta-faces-4.0#determining-the-character-encoding
@TomasHofman TomasHofman changed the base branch from master to 4.0 January 14, 2025 20:05
@TomasHofman
Copy link
Contributor Author

This is a reproducer project: https://github.com/jbaesner/mojarra-response-encoding-issue/tree/main

I reproduced on Wildfly 35 (mojarra 4.0.8).

@schluedo
Copy link

schluedo commented Jan 15, 2025

https://www.w3schools.com/tags/ref_urlencode.asp :
"Your browser will encode input, according to the character-set used in your page."
-> I would like to suggest a small but useful change:
Store the encoding in the ViewState, so the encoding is an "attribute" of the rendered Page/View. This fits the JSF-state-on-server-paradigm perfectly and also matches the JSF-spec "store it in the session" (but once per view).
This also avoids concurrency issues if you have multiple tabs, with different encodings.

having a look at:
<f:view locale="xy" encoding="UTF-8"> ...
the encoding already looks like an attribute of the view but it does not behave like it.

@TomasHofman
Copy link
Contributor Author

@schluedo you suggestion sounds interesting but I would leave that to somebody with better knowledge of Mojarra internals. Also it's maybe more suitable for the "next" version rather than the "maintenance" 4.0, as it would again be a behavior change.

@schluedo
Copy link

OK!
I'm not familiar with the processes here...
Will YOU @TomasHofman start that? or what do I have to do for that?

@TomasHofman
Copy link
Contributor Author

Not me, I'm not a Mojarra developer, I just came to propose this PR to fix my problem :).

@BalusC
Copy link
Contributor

BalusC commented Jan 18, 2025

Superseded by #5544

@BalusC BalusC closed this Jan 18, 2025
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