Hmm. I may not be the best to weigh in on this, as i never used event handlers for escaping. I always preferred the direct, explicit control of escaping via tools, and trusted myself (YMMV) not to forget escaping anywhere important.
I'll definitely agree that we shouldn't shade. I'm perfectly comfortable with an optional dependency, so long as it's clearly documented. And as for providing basic escaping methods, well, if you're up for the work and like that, go for it. Those who do the work, yada, yada. :) On Thu, Mar 7, 2019 at 4:08 AM Claude Brisson <cla...@renegat.net.invalid> wrote: > The StringEscapeUtils class is moving from commons-lang3 to commons-text. > > We are using it in several reference insertion event handlers (to escape > xml, javascript or html). > > For now the commons-lang3 version is still there, but deprecated. We > should address this in engine 2.1, but how? > > 1) just *add* the new dependency towards commons-text > 2) add it, but make it optional, since only people using those escaping > event handlers will need it > 3) move the event handlers to a new module velocity-engine-events > containing all event handlers > 4) use shading > 5) switch to using an internal escaping utility > > First, I'd like to rule out 1), 3) and 4): > - adding a 192k dependency for just a few basic escaping functions > looks overkill > - creating a new module just for this need is painful and awkward > - shading is evil, and StringEscapeUtils uses many internal classes > > The thing to note here is that those escaping reference insertion > handlers are basically *examples* of reference insertion handlers, > hardly usable in production, because how to escape a reference in a > Javascript/HTML/XML template depends on where this reference goes (you > don't have the same escaping needs in an attribute value and in plain > text for instance). > > To properly handle escaping (beyond explicitly escaping each reference > when needed, which is what most people do), we need to switch escaping > context, either manually in the template (as proposed in VELOCITY-705 > [1]), or automatically by parsing the generated content on-the-fly > (let's dream). But it's another debate. > > I'm usually not so fond of optional dependencies, since the main point > of using maven is to have it handle dependencies automatically. But > since those event handlers aren't really usable in production, I'd be ok > with it. Or, we can just provide them basic escaping standalone methods. > > Thoughts? > > Claude > > [1]: https://issues.apache.org/jira/browse/VELOCITY-705 > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org > For additional commands, e-mail: dev-h...@velocity.apache.org > >
