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.

Sign up for GitHub

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

Jump to bottom

Make rpc serialization chunk size configurable #9961

Merged
merged 1 commit into from
May 31, 2024
Merged

Make rpc serialization chunk size configurable #9961

merged 1 commit into from
May 31, 2024
+4 −4

Conversation

codemasterover9000
Copy link
Contributor

Make the RPC chunk size configurable through a system property gwt.rpc.maxPayloadChunkSize. This allows implementors to circumvent the RPC protocol version to fallback to version 7 on large payloads. Which in the current client implementation uses unsafe javascript eval, preventing proper operation on sites with CSP's restricting unsafe-eval.

Fixes #9578

@codemasterover9000 codemasterover9000 mentioned this pull request May 24, 2024
Open
@niloc132 niloc132 changed the title Make rpc serialization chunk size configurable (#9578) Make rpc serialization chunk size configurable May 24, 2024
niloc132
niloc132 approved these changes May 27, 2024
View reviewed changes
user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamWriter.java
private static final String POSTLUDE = "])";
private static final String PRELUDE = "].concat([";

private final StringBuffer buffer;
private final int maximumArrayLength = Integer.getInteger("gwt.rpc.maxPayloadChunkSize", 1 << 15);
Copy link
Member

Choose a reason for hiding this comment

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

Keep this static? It should only need to be read once (and used consistently afterwards)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes true, that was my initial change but then I looked at the gtw.rpc.version property which does seem runtime configurable. I can change it back to static if you want.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, that makes sense. System.getProperty is a blocking call on older JVMs, but at least on Java 11 this is replaced by a ConcurrentHashMap.

I suspect the reason that GWT historically looked up the value on each call is to allow the SerializationException to be thrown when a call was made, rather than when the class was initialized, since the range of accepted values could change release to release.

I think I'm fine leaving this max size as a runtime lookup, as you have it written.

Will leave the PR open a bit longer for any other thoughts on the matter.

@niloc132 niloc132 mentioned this pull request May 31, 2024
Open
@niloc132 niloc132 merged commit f7ffe6d into gwtproject:main May 31, 2024
3 checks passed
@niloc132 niloc132 added this to the 2.12 milestone May 31, 2024
@niloc132
Copy link
Member

Looks like tests weren't run on this PR, or even compiled? I'll submit a followup to correct the compile error...

niloc132 added a commit to niloc132/gwt that referenced this pull request May 31, 2024
@niloc132
Restore constant field for max array length to fix tests
a1d21a1 
Follow-up gwtproject#9961
@niloc132 niloc132 mentioned this pull request May 31, 2024
Merged
@niloc132
Copy link
Member

@codemasterover9000 PTAL #9963

@codemasterover9000
Copy link
Contributor Author

Yeah sorry I assumed that the tests would run automatically after commit. I only tested the compile of the single file from intellij. As I can not get GWT te compile anymore using ant after updating the project. I should have mentioned that though... sorry about that.

niloc132 added a commit that referenced this pull request Jun 1, 2024
@niloc132
Restore constant field for max array length to fix tests (#9963)
f47c121 
Follow-up #9961
@lgemeinhardt lgemeinhardt mentioned this pull request Aug 28, 2024
Merged
niloc132 pushed a commit that referenced this pull request Sep 5, 2024
@lgemeinhardt
Make MAX_STRING_NODE_LENGTH configurable (#9995)
be05c1e 
Make `ServerSerializationStreamWriter.MAX_STRING_NODE_LENGTH`
configurable, to better work around #9578 and allow real JSON in all
cases.
This PR is a follow-up to #9961 (making the rpc serialization chunk size
configurable).

Workaround #9578 
Followup #9961
@codeart1st codeart1st mentioned this pull request Nov 11, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niloc132 niloc132 niloc132 approved these changes

Assignees
No one assigned
Labels
Category-RPC
Projects
None yet
Milestone
2.12
Development

Successfully merging this pull request may close these issues.

RPC format relies on eval for large payloads and long strings
2 participants
@codemasterover9000 @niloc132