gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.52k stars 374 forks source link

RPC format relies on eval for large payloads and long strings #9578

Open niloc132 opened 6 years ago

niloc132 commented 6 years ago

GWT version:2.8.2 Browser (with version):any Operating System:any


Description

From https://github.com/gwtproject/gwt/issues/8197#issuecomment-349342472, it turns out that RPC's server serialization stream writer still assumes that the browser needs concatenated strings and arrays.

However, the strings were concatenated to support hosted mode (which apparently at the time couldn't support larger than 64kb strings), but that appears to have been fixed, and arrays were split to support a bug in IE6/7 where long arrays couldn't be eval'd, but again, we don't support IE6/7 any more.

So, we don't really need it. Additionally, this is an issue since it violates CSP, as eval isn't particularly safe, and some sites would like to forbid its use to further protect their data and their users.

I'll try removing these hacks and confirm that eval is no longer called (except of course in browsers that do not support JSON.parse), and confirm that running dev mode can handle giant strings correctly as well. This should remove the ability of the server to produce streams older than version 8 (since ServerSerializationStreamWriter.writeHeader is the only code that should be writing out a version for the client). Will leave in support on the client for reading older streams, as we've done in the past.

heartdisease commented 5 years ago

Any updates on this? We are using GWT at work and would love to finally get rid of the unsafe-eval rule.

niloc132 commented 5 years ago

I do have this patch somewhere, but can't recall where it stalled, probably in making sure that other browsers tested well.

Instead, most of my time in this area has been spent on a feature-complete replacement for RPC, based on APT instead of Generators. This is at https://github.com/vertispan/gwt-rpc/, and while it isn't ever going to get to 100%, it is probably at about 70% now, and should be able to reach 80-90% (minus just a few features like reflection/violator pattern to modify private or final fields directly, so bean-like getters and setters or custom field serializers will be required).

I'll see if I can find the patch, but this work was not needed for the new RPC at all, which doesn't mess with json strings at all at this time.

heartdisease commented 5 years ago

So that'd mean that without this patch we'd be kind of stuck with unsafe-eval unless we upgrade to GWT 3?

niloc132 commented 5 years ago

No - while the linked project is intended to be compatible with GWT 3, it is already compatible with GWT 2 (and that isn't expected to change).

Zim123 commented 5 years ago

Hello.

I'm not sure if this is the correct place for my question, but it was the only place more relative to what I'm struggling with. And to not open a new issue with relatively the same problem.

So, my problem is related to CSP, specifically 'unsafe-inline' rule, because now, in my situation compiled application violates it. Whether I'm using 'xsiframe' or 'direct_install' linker, the situation is related to the fact I'm using deferred split modules, which are injecting via inline js script to iframe, and in this way violate the 'unsave-inline' rule. I could use sso linker, but it's not a case, because it does not support several modules.

Can anyone suggest maybe some linker or approach, to fix this issue, please?

Regards.

tbroyer commented 5 years ago

@Zim123 Let's discuss on #8197 or, better, https://groups.google.com/forum/#!forum/google-web-toolkit

bcampolo commented 1 year ago

We are hitting this issue (9578) as well. We recently had a security scan find the use of unsafe-eval in our GWT-based application. Upon removing unsafe-eval and setting the gwt.rpc.version to 8, everything worked except for requests with large responses > 32k, which executed an eval instead of safeEval. It appears that ServerSerializationStreamWriter.LengthConstrainedArray is used to conditionally change the response when the size is greater than 32k which resets the rpc version back to 7 and uses a special "PRELUDE" concatenator and flags the stream buffer as 'javascript'. There are comments in this thread and in the code which indicates this is only needed for IE 6/7 both of which are no longer supported. We quickly built GWT locally to test with larger size constraint and this fixed the issue we were seeing. Do you know if a fix for this issue would be included in the next release?

Section of code that I referenced regarding the length check: https://github.com/gwtproject/gwt/blob/main/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamWriter.java#L71

niloc132 commented 1 year ago

Yes, I think we could probably remove that workaround entirely - are you interested in putting together a patch for it?

bcampolo commented 1 year ago

I appreciate your willingness to accept a patch. Unfortunately my company is not able to prioritize the work to create a patch at this time. I will monitor this issue and let you know if that changes.

hijackit commented 4 months ago

how do you set gwt.rpc.version to version 8?

niloc132 commented 4 months ago

https://github.com/gwtproject/gwt/pull/9961 provides a workaround for this issue, by letting gwt.rpc.maxPayloadChunkSize be set to a large number to prevent payloads from being split. The gwt.rpc.version value is already supported in past GWT versions, but will not alone be enough to stop using eval.

These are both Java System Properties, so how you set them will depend on how you start your server. If you can pass JVM arguments (such as through JAVA_OPTS etc), you could pass them as -Dgwt.rpc.maxPayloadChunkSize=2147483647 -Dgwt.rpc.version=8, but you could also call System.getProperty("gwt.rpc.version", "8") etc on startup, before any calls have been made.

niloc132 commented 4 months ago

Sorry, this isn't fixed, but the workaround is improved.