FWIW, I agree with tbroyer's assessment and the suggestion to use a helper sounds reasonable.
On Thu, Nov 3, 2016 at 10:40 AM, Thomas Broyer <t.bro...@gmail.com> wrote: > > > On Thursday, November 3, 2016 at 5:33:31 PM UTC+1, Colin Alworth wrote: >> >> With 3.0 on the horizon, we've promised consistency of a sort in 2.x, and >> without 3.0 actually in sight, 2.x is going to need to see active >> development. Encouraging a third generation of url tools is not a bad >> thing, but only switching over half-way leaves something to be desired. >> >> I'll play devil's advocate a bit - I'm not addressing the patch >> specifically (since I haven't fully read all of it or the discussion around >> it, and if it was a bad patch, I'm sure it would have been shot down on its >> own merits) but the thinking to use around what goes into the 2.x branch: >> * (point 1) We're not ready for maintenance mode, at least until we have >> timelines and completed APIs for GWT 3. If we are, we should be forbidding >> all merge requests to gwt-user that don't fix critical bugs. >> >> * (point 2 and 3) Can't argue with new tools arriving that solve the >> problem better, especially if they are going into 3 as a cross-platform way >> of solving the problem (gwt/java/j2cl). Obvious caveat here (even for the >> devil's advocate) that with the release of 3, breaking changes off of 2 are >> acceptable and expected. >> >> * (point 4) Without transitioning the existing GWT Safe* tools, >> SafeHtmlTemplates is stuck in the past, as is UiBinder, I18n Messages, and >> any HasSafeHtml widget (both in GWT and in the general ecosystem). This is >> a lot to leave behind in the name of "but the tool didn't belong in GWT in >> the first place". We've had our chance to properly deprecate it for the 2.8 >> release, and if our past timelines are any measure, it is going to be at >> least a year before we finally ship a release with SafeHtml, and all that >> use it, deprecated. And once we've reached that point, unless gwt-user >> depends on (or rebases, etc) safe-html-types or any other successor, all of >> the above downstream classes which use SafeHtml are left effectively broken >> (not unlike the unported Generators and Linkers we have today). >> >> Obviously ClientBundle/GssResource isn't actually deprecated, but we have >> officially said that new tools should not use the features that it takes >> advantage of. SafeHtml takes this a step further - GssResource got several >> upgrades within the 2.8 release (unlike other generators), despite it being >> deprecated, but with SafeHtml going out, it takes out features within many >> GWT Widgets themselves. There will be no officially supported way to >> correctly assign safe html content into an HTML widget. >> >> Now yes, a bit of hyperbole there - you can of course (as Thomas noted in >> his email) simply asString() the not-gwt SafeHtml and use it, or provide >> your own wrapper, but depending on your project size (and GWT users out >> there have some big ones) that could be hundreds or thousands of sites to >> fix in code. Yet another change would be needed for any widgets that the >> project has which implement HasSafeHtml (so it can still be used in >> UiBinder), UiBinder @UiFields or getters (which return SafeUri for use in >> its embedded SafeHtml handling), Messages (to wrap any incoming url). This >> ignores any use of Messages/Constants in a SafeHtml-using uibinder itself - >> I'm not seeing a clear path there without the use of default methods in >> I18n interfaces (which admittedly, would probably work, but isn't going to >> be pretty). >> > > You forgot one major point here: this patch is proposing a new API (new > overloads to existing methods), without actually changing the current API > (at least in terms of its contract). > So it's not about changing "every call sites for SafeHtml/UriUtils API", > it's about being able to take advantage of a new behavior by selectively > calling a new API. > If you want that behavior everywhere, then yes you'll have to update all > call sites; but the patch doesn't change that in any way (except what you > change your code *to*) > All those existing usage of SafeHtml you cited (SafeHtmlTemplates, > UiBinder, I18N and HasSafeHtml) would be unchanged and keeping their > current behavior (and only 2 of them deal with SafeUri: SafeHtmlTemplates > and UiBinder). And the only two cases where UriUtils is called by those > (vs. receiving a SafeUri value from the "user") are discouraged and > generate a warning (and would continue to only use the default whitelist > and reject "tel:" URIs; if you'd like to allow "tel:" URIs there, you'd > have to change to SafeUri anyway, and then the responsibility of converting > a String to a SafeUri falls on you, so no change to UriUtils or other > APIs/generators is actually *required*): > https://github.com/gwtproject/gwt/blob/3a41a8067767965ff72f4e49eb015f > 587fcfee16/user/src/com/google/gwt/uibinder/attributeparsers/ > SafeUriAttributeParser.java#L41 > https://github.com/gwtproject/gwt/blob/3a41a8067767965ff72f4e49eb015f > 587fcfee16/user/src/com/google/gwt/safehtml/rebind/ > SafeHtmlTemplatesImplMethodCreator.java#L183 (warning at: > https://github.com/gwtproject/gwt/blob/3a41a8067767965ff72f4e49eb015f > 587fcfee16/user/src/com/google/gwt/safehtml/rebind/ > SafeHtmlTemplatesImplMethodCreator.java#L364) > > After the security review, we're not in a situation of "I think GWT's > behavior should be changed (and this will affect everyone in many places)", > we're rather in a situation of "I'd like to add this new feature to GWT; > and this is something I could do in my own code" (delegating to the > existing behavior after I checked whether the new behavior I want would > apply). > > To put it in precise terms for this patch, it's a matter of changing > UriUtils.fromString(…) to either TelAwareUriUtils.fromString(…) > (application-specific factory that eventually delegate to > UriUtils.fromString) or UriUtils.fromString(…, > Collections.singleton("tel")) (new proposed API). That TelAwareUriUtils > would look like: > > public class TelAwareUriUtils { > public static SafeUri fromString(String uri) { > if (isTelUri(uri)) { > return UriUtils.fromTrustedString(uri); > } > return UriUtils.fromString(uri); > } > > // add similar isSafeUri and sanitizeUri methods > > private boolean isTelUri(String uri) { > return uri.regionMatches(true, 0, "tel:", 0, 4); // this is a > case-insensitive startsWith > } > } > > Speaking for myself again, I am not prepared to outright "-2" any >> contribution to gwt-user that isn't a fix for a critical bug. I don't think >> we're ready today to deprecate everything inside of gwt-user that isn't JRE >> emulation or jsinterop-based. Ideally of course aspects of GWT will be spun >> out as community projects and maintained separately, but until we pull it >> off (or even start), any new GWT user will assume that anything in gwt-user >> is the best basis for a GWT project. Actively abandoning most of gwt-user >> is not going to help the already difficult situation we're in there. >> > > +1, but we're not in that exact situation here. > > -- > 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 google-web-toolkit-contributors+unsubscr...@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/ > msgid/google-web-toolkit-contributors/cede6c7f-7428- > 42f0-bc42-8b1cb03cbecf%40googlegroups.com > <https://groups.google.com/d/msgid/google-web-toolkit-contributors/cede6c7f-7428-42f0-bc42-8b1cb03cbecf%40googlegroups.com?utm_medium=email&utm_source=footer> > . > > For more options, visit https://groups.google.com/d/optout. > -- 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 google-web-toolkit-contributors+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/google-web-toolkit-contributors/CAN%3DyUA0c2wjzwsZnS0hj3Z27VBpZDTCdf9z_tCjsXbyB05E_qw%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
