Skip to content

Fix Response encoding not stored in the ext context session #5543#5544

Merged
BalusC merged 6 commits into4.0from
issue_5543
Feb 1, 2025
Merged

Fix Response encoding not stored in the ext context session #5543#5544
BalusC merged 6 commits into4.0from
issue_5543

Conversation

@BalusC
Copy link
Copy Markdown
Contributor

@BalusC BalusC commented Jan 18, 2025

@BalusC BalusC requested review from arjantijms and mnriem January 18, 2025 14:02
@BalusC BalusC added this to the 4.0.10 milestone Jan 18, 2025
// Save encoding in UIViewRoot for faster consult when Util#getResponseEncoding() is invoked again elsewhere.
context.getViewRoot().getAttributes().put(FACELETS_ENCODING_KEY, encoding);

// Save encoding in Session for consult in subsequent postback request as per spec section "2.5.2.2. Determining the Character Encoding".
Copy link
Copy Markdown
Contributor

@mnriem mnriem Jan 21, 2025

Choose a reason for hiding this comment

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

Why is this stored in the session? Please review this with Servlet 6.0 specification, section 5.6. Internationalization in mind.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As stated in that comment, this is mandated by the JSF 2.3 specification chapter 2.5.2.2 Determining the Character Encoding, see:

"[P1-start-encoding]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 servlet or portlet response) in the session (if and only if a session
already exists) under a well known, implementation-dependent key."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As I stated before this needs to be reviewed with the specified Servlet 6.0 specification in mind. I do not object to setting it, but I do not see any logic that looks at the Servlet level. Note that JSF 2.3 is not supported and I assume you meant to refer to Faces specifications?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes, my fault, sorry, I meant Faces 4.0...

Anyhow, the spec has not changed there, see Jakarta Faces 4.0 specification.

As of that I don't see how this is related to the Servlet 6.0 specification, as the Faces specification already requires storing the character encoding in the session.

Copy link
Copy Markdown
Contributor Author

@BalusC BalusC Jan 24, 2025

Choose a reason for hiding this comment

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

Why is this stored in the session?

Because last used form encoding is not sent back via headers/payload of a postback request per-se.

Please review this with Servlet 6.0 specification, section 5.6. Internationalization in mind.

It's indeed used for setRequestEncoding() as mentioned in that spec to remedy any breakages.

If the client hasn’t set character encoding and the request data is encoded with a different encoding than the default as described above, breakage can occur. To remedy this situation, setRequestCharacterEncoding(String enc) is available on ServletContext

@arjantijms arjantijms changed the title Fix #5543 Fix Response encoding not stored in the ext context session #5543 Jan 22, 2025
@bmaxwell
Copy link
Copy Markdown

@arjantijms , is this one ok to merge? We have some users needing the fix. Thanks !

@BalusC BalusC merged commit 60f58f7 into 4.0 Feb 1, 2025
@BalusC BalusC deleted the issue_5543 branch February 1, 2025 13:14
@soul2zimate
Copy link
Copy Markdown
Contributor

Hi @BalusC @arjantijms, Is there any plan to release a new 4.0.10 soon?

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.

5 participants