[cc gwt-steering; please reply to gwt-contrib]

Hi there,

There's a patch currently in review of which I don't quite know what to 
do: https://gwt-review.googlesource.com/12940

It proposes a change to c.g.g.safehtml.shared.UriUtils to allow 
whitelisting URL schemes in addition to the default whitelist (http, https, 
ftp, and mailto), with the (original) purpose of considering "tel:" URIs as 
"safe". Actually, it started out as simply whitelisting the "tel:" scheme, 
but then we had a "security review" that asked not to do it.

As I see it:

   - Now that GWT 2.8 has been released; GWT 2.x is moving to "maintenance 
   mode" (one could argue this is not the case yet, as we still don't have a 
   clear plan for GWT 3.x and work on it hasn't yet started).
   - Google has rewritten c.g.g.safehtml for use outside GWT (but is still 
   GWT-compatible) at https://github.com/google/safe-html-types Given that 
   Google will slowly be moving their projects from GWT to J2Cl, we should 
   expect them to invest in safe-html-types and deprecate (from their point of 
   view at least) c.g.g.safehtml.
   - As a corollary, there's little to no reason to port c.g.g.safehtml to 
   J2Cl. We'll probably rather end up deprecating it, and/or rewriting it as a 
   wrapper around safe-html-types to make it J2Cl-compatible (for a smoother 
   transition).
   - There are easy workarounds for whitelisting custom URL schemes today: 
   one could use Google's safe-html-types and "wrap" the sanitized value into 
   a UriUtils.fromTrustedString(), or use String#regionMatches to 
   case-insensitively compare the URL prefix with the custom scheme(s) and 
   fallback to UriUtils.sanitizeUri() / UriUtils.fromString(). Given that with 
   the proposed change, call sites would have to explicitly whitelist new 
   custom schemes (i.e. require changes to call sites), they could just as 
   well be changed to do that extra work (possibly through custom factories). 
   The patch author probably already has such code in place already (the patch 
   was actually initially proposed more than one year ago).

As such, I'm worried that we (and I include everyone, including the 
contributor, and the webappsec team at Google) may be investing into an API 
that's bound to be deprecated in the (near) future; and it doesn't seem 
worth the effort.
I'm also worried that this change may have an impact in the final code size 
(for everyone, not only those using the new API, which would obviously not 
be a problem), and possibly runtime performance (I doubt it, but who 
knows?). Making sure this isn't the case (or is acceptable) means spending 
even more time on the matter.

I'm a bit reluctant to giving a +2 (or even a +1) to the patch (after some 
more changes anyway) for the above reasons, actually thinking I may rather 
reject it entirely with a -2. I don't want to take such a decision 
unilaterally though.

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/2a5f6b57-2462-484c-bfab-aec09c2e2eea%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to