frankframework / frank-doc

Frank!Doc
https://frankdoc.frankframework.org
Apache License 2.0
4 stars 5 forks source link

Detect suspicious HTML in documentation #115

Closed gvanbrakel closed 2 years ago

gvanbrakel commented 2 years ago

Brackets '<' and '>' are used in documentation to HTML format the text, like in ' ... if the value is <code>true</code> this means ... '. In some cases an author tried to use these brackets for some other purpose, e.g. to describe a variable. See for example WebServiceListener.setAddress:

@IbisDoc({ "The address to listen to, e.g the part <address> in https://mydomain.com/ibis4something/services/</address>, where mydomain.com and ibis4something refer to 'your ibis'","" })

The author clearly meant to display the brackets 'as is', but they are currently interpreted as HTML, see #3165. In most case this would yield some illegal XML (not in this example however). It would be good to signal that suspicious XML in the logging as a warning, so the documentation can be changed.

mhdirkse commented 2 years ago

Do you want a warning each time a < and a > appear in the text of JavaDoc comments?

mhdirkse commented 2 years ago

I discussed on the phone with @gvanbrakel. We will make a list of HTML tags like <tr>, <td> and <code>. When these appear literally in JavaDoc comments, they are meant to be HTML. All other HTML tags are suspicious. Probably they should be escaped to render them correctly.

mhdirkse commented 2 years ago

I applied this Frank!Doc code to F!F commit 0e1ebd36561f68a6f6526a0f190831e0bd5f4506. Here are the suspicious HTML warnings I got:

19:04:28,047 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.scheduler.JobDef] has a description with suspicious HTML tags: [<p>]
19:04:28,047 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.receivers.Receiver] has a description with suspicious HTML tags: [<p>]
19:04:28,047 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.errormessageformatters.XslErrorMessageFormatter] has a description with suspicious HTML tags: [<p>]
19:04:28,047 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.bis.BisJmsListener] has a description with suspicious HTML tags: [<p>]
19:04:28,048 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.jms.JmsListener] has a description with suspicious HTML tags: [<p>]
19:04:28,048 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.jms.PushingJmsListener] has a description with suspicious HTML tags: [<p>]
19:04:28,048 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.esb.EsbJmsListener] has a description with suspicious HTML tags: [<p>]
19:04:28,048 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.fxf.FxfListener] has a description with suspicious HTML tags: [<p>]
19:04:28,048 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.ifsa.jms.PushingIfsaProviderListener] has a description with suspicious HTML tags: [<p>]
19:04:28,048 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.ifsa.jms.IfsaFacade] has a description with suspicious HTML tags: [<p>]
19:04:28,048 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.mqtt.MqttListener] has a description with suspicious HTML tags: [<p>]
19:04:28,048 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.http.RestListener] has a description with suspicious HTML tags: [<p>]
19:04:28,049 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.jms.PullingJmsListener] has a description with suspicious HTML tags: [<p>]
19:04:28,049 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.filesystem.MailListener] has a description with suspicious HTML tags: [<p>]
19:04:28,049 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.receivers.JavaListener] has a description with suspicious HTML tags: [<p>]
19:04:28,049 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.akamai.NetStorageSender] has a description with suspicious HTML tags: [<p>]
19:04:28,049 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.http.HttpSenderBase] has a description with suspicious HTML tags: [<p>]
19:04:28,049 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.bis.BisJmsSender] has a description with suspicious HTML tags: [<p>]
19:04:28,049 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.cmis.CmisSender] has a description with suspicious HTML tags: [<p>]
19:04:28,049 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.ibm.IMSSender] has a description with suspicious HTML tags: [<p>]
19:04:28,050 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.ibm.MQSender] has a description with suspicious HTML tags: [<p>]
19:04:28,050 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.ifsa.IfsaSimulatorJmsSender] has a description with suspicious HTML tags: [<p>]
19:04:28,050 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.mqtt.MqttSender] has a description with suspicious HTML tags: [<p>]
19:04:28,050 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.http.HttpSender] has a description with suspicious HTML tags: [<p>]
19:04:28,050 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.jdbc.FixedQuerySender] has a description with suspicious HTML tags: [<p>]
19:04:28,050 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.jms.XmlJmsBrowserSender] has a description with suspicious HTML tags: [<p>]
19:04:28,051 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.senders.AmazonS3Sender] has a description with suspicious HTML tags: [<p>]
19:04:28,051 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.senders.FileSender] has a description with suspicious HTML tags: [<p>]
19:04:28,051 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.util.FileHandler] has a description with suspicious HTML tags: [<p>]
19:04:28,051 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.senders.MailSender] has a description with suspicious HTML tags: [<p>]
19:04:28,051 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.senders.ReloadSender] has a description with suspicious HTML tags: [<p>]
19:04:28,051 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.senders.SambaSenderOld] has a description with suspicious HTML tags: [<p>]
19:04:28,051 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.senders.ShadowSender] has a description with suspicious HTML tags: [<p>]
19:04:28,051 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.esb.EsbJmsTransactionalStorage] has a description with suspicious HTML tags: [<p>]
19:04:28,052 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.api.ApiWsdlXmlValidator] has a description with suspicious HTML tags: [<p>]
19:04:28,052 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.soap.SoapValidator] has a description with suspicious HTML tags: [<A>, <p>]
19:04:28,052 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.pipes.AbstractPipe] has a description with suspicious HTML tags: [<p>]
19:04:28,052 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.api.ApiSoapWrapperPipe] has a description with suspicious HTML tags: [<p>]
19:04:28,052 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.bis.BisWrapperPipe] has a description with suspicious HTML tags: [<p>]
19:04:28,053 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.esb.EsbSoapWrapperPipe] has a description with suspicious HTML tags: [<p>]
19:04:28,053 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.api.ApiStreamPipe] has a description with suspicious HTML tags: [<p>]
19:04:28,053 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.rekenbox.Adios2XmlPipe] has a description with suspicious HTML tags: [<p>]
19:04:28,053 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.rekenbox.LabelFormat] has a description with suspicious HTML tags: [<p>]
19:04:28,053 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.rekenbox.RekenBoxCaller] has a description with suspicious HTML tags: [<p>]
19:04:28,053 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.extensions.tibco.GetTibcoQueues] has a description with suspicious HTML tags: [<p>]
19:04:28,053 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.ldap.LdapChallengePipe] has a description with suspicious HTML tags: [<p>]
19:04:28,054 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.pipes.EscapePipe] has a description with suspicious HTML tags: [<p>]
19:04:28,054 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.pipes.FilePipe] has a description with suspicious HTML tags: [<p>]
19:04:28,054 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.pipes.FilenameSwitch] has a description with suspicious HTML tags: [<p>]
19:04:28,054 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.pipes.IsUserInRolePipe] has a description with suspicious HTML tags: [<p>]
19:04:28,054 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.pipes.LarvaPipe] has a description with suspicious HTML tags: [<p>]
19:04:28,054 WARN  FrankDocModel.checkSuspiciousHtml():860[] FrankElement [nl.nn.adapterframework.pipes.PGPPipe] has a description with suspicious HTML tags: [<p>]
19:04:28,054 WARN  FrankDocModel.checkSuspiciousHtml():864[] The following suspicious HTML tags appear in all FrankElements combined: [<p>, <A>]
mhdirkse commented 2 years ago

I only check descriptions of classes, not attributes and not config child setters. Should I add these?

gvanbrakel commented 2 years ago

<p> is a valid HTML tag for class javadoc '` is a valid HTML tag for all javadoc

please also check descriptions of attributes and config child setters.

mhdirkse commented 2 years ago

@gvanbrakel, when I used <p> in my JavaDoc comments then @nielsm5 protested. JavaDoc comments should only have HTML tags that can be closed. Instead of <p>, we should use <br/><br/>. Do you mind if I leave <p> as suspicious HTML?

nielsm5 commented 2 years ago

The use of <p> is fine, as long as you close the tag!

mhdirkse commented 2 years ago

The code I have now does not check whether tags are closed. Do you want me to add that check? I would prefer not to add that to keep the scope of this issue limited.

mhdirkse commented 2 years ago

In my PR, I have added <p> as an allowed HTML tag. I also check config children and attributes now.