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.

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

Add generic support for text which contains XML control characters #72

Closed
AlexMekkering opened this issue Apr 4, 2015 · 4 comments
Closed

Comments

@AlexMekkering
Copy link
Member

In Fresnel Forms, the user can enter any text he or she likes, including XML control characters. A probable candidate for this is the delimiter for a list of property values which in some cases may contain an &, which isn't allowed in XML text elements which are used for page definitions in FresnelForms.
For the XML output to be well-formed at all times, these characters have to be encoded which is currently not implemented.

The use of standard Java DOM's for generating well-formed XML doesn't seem to be very scalable, for it will create large objects to build the XML tree just for generating output, but a short survey has lead me to the Apache Commons Lang library (http://commons.apache.org/proper/commons-lang/).
This library, which is very polular amongst Java developers, contains a StringEscapeUtils class with several static methods to escape strings for some target syntaxes including XML.

Generic use of this functionality for all article content will automatically escape all character(sequences) which violate the XML standard. In addition, our own code, which currently explicitly escapes all violating character sequences can be converted to be a more concise, readable and maintainable, for it too will be escaped automatically as needed.

@LloydRutledge
Copy link
Contributor

Well seen, Alex. Thanks! But do we already support this via the poorly scaling Java DOM? Or do we currently not support at all?

@AlexMekkering
Copy link
Member Author

@LloydRutledge, dit werd nog niet ondersteund in FresnelForms. Alles werd 'String-based' doorgestuurd naar de XML output.

@jheijning also,
Gebruik van StringEscapeUtils is geimplementeerd in r515 en r516, 't was gelukkig niet zoveel werk en alles laadt weer als vanouds in MediaWiki (met de juiste paginavulling natuurlijk). De output is alleen iets groter geworden omdat ook quotes en dubbelquotes escaped worden, maar dit is wat mij betreft zelfs iets zuiverder 😉.

In r515:

  • de statische methode StringEscapeUtils.escapeXml10 is gebruikt voor zowel de title als de content van een Article zodat het per definitie een well-formed XML bestand wordt.
  • Alle expliciete &lt; in de code zijn vervangen door <
  • Alle expliciete &gt; in de code zijn vervangen door >
  • Alle double quotes in de teksten in de code zijn vervangen door single quotes omdat deze binnen Java strings niet escaped hoeven te worden (Java Strings beginnen en eindigen met double quotes).

in r516:

  • De delimiter bepaling is vereenvoudigd. We hoeven niet meer te checken op &nbsp; om er &amp;nbsp; van te maken, dat gebeurt nu automatisch door StringEscapeUtils.

@jheijning, wil jij het reviewen?

@AlexMekkering
Copy link
Member Author

In r525 is een kleine, maar niet onbelangrijke wijziging doorgevoerd.
Hoewel de gegenereerde XML namelijk per definitie well-formed was, was de inhoud van een pagina zelf dat nog niet. Met name de labels, welke door de gebruiker worden opgegeven, worden een keer extra ge-escaped om zaken die veel op XHTML lijken, goed als tekst op de pagina te laten verschijnen.
Wanneer de gebruiker bijvoorbeeld <div> als label invult wordt dit nu ook als zodanig weergegeven terwijl dat eerder voor nesting problemen op de pagina zorgde.
De delimiter heb ik onaangetast gelaten omdat juist hierbij stuurcodes (zoals &nbsp;) van belang kunnen zijn.

Daarnaast zijn een aantal kleine wijzigingen in de unit tests aangebracht.

@jheijning, zou je deze wijzigingen willen meenemen in de review?

@jbachh
Copy link
Contributor

jbachh commented Apr 8, 2015

Ziet er goed uit! Maakt het allemaal een heel stuk leesbaarder ook. Wat mij betreft sluiten.

@jbachh jbachh closed this as completed Apr 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants