BUG SAMI tags in comments aren't sanitised

[andrewandante]

See http://api.silverstripe.org/4/SilverStripe/Forms/DropdownField.html

The phpdoc for the DropdownField class contains the line Dropdown field, created from a <select> tag. an actual tag is rendered, throwing off the whole page. This can be worked around by using <select> instead - which means that either:

a) the content needs to be sanitised before rendered - see https://github.com/FriendsOfPHP/Sami/issues/261 b) we need to check all our classes for tags and update/remove them accordingly

[robbieaverill]

Perhaps the quick win is to remove the <> from the source code for now

[tractorcow]

Doh, I think so!

[andrewandante]

I've had a little play around with this today - one of the other things we can do is edit the twig templates to escape HTML - i.e

 {% if class.shortdesc -%}
   <p>{{ class.shortdesc|desc(class)|escape('html') }}</p>
 {%- endif %}

However, you end up with a big blob of text, rather than the very-useful examples.

So we can either remove < and > from docblocks, or add backticks - they render the same, but backticks read a little cleaner.

I'm curious how we managed to get around this with the previous implementation - it seems that it would turn on the HTML renderer if a line began with a tag? Something like that? Maybe we can try and re-implement something along those lines.

[tractorcow]

If we pass the docblock through a DOMDocument, and return that, wouldn't that guarantee correctly matched opening / closing tags, and prevent the escaping occurring?

[tractorcow]

All merged, just need to rebuild the docs now.

[chillu]

/cc @nathanbrauer

[chillu]

@tractorcow How do we get the rebuild done? I would've thought that's at least nightly?

[tractorcow]

Yes it's ment to happen every night but it may not be working.

[andrewandante]

Build is running, just had a look. Seems that the 3 branch is updated (see http://api.silverstripe.org/3/DropdownField.html) but 4 is not (see http://api.silverstripe.org/4/SilverStripe/Forms/DropdownField.html). Not sure what is going on there tbh.

You can see the job running here: https://logs.platform.silverstripe.com/streams/5955c89de4b09ee3b9815f57/messages?rangetype=absolute&fields=source%2Clog_type%2Cmessage&width=1920&from=2017-12-13T13%3A59%3A29.000Z&to=2017-12-13T14%3A03%3A02.000Z&q=log_type%3ASilverStripe_cron#fields=source%2Clog_type%2Cmessage if you have Graylog permissions.

[andrewandante]

Update - looks like the pull --all command isn't pulling, well, all. Will put in a PR.

[andrewandante]

https://github.com/silverstripe/api.silverstripe.org/pull/66 is ready

[chillu]

Thanks for looking into this, Andrew!

[dhensby]

66 is merged so I'm closing this

[chillu]

Note that this isn't deployed yet, I'm just following up with @robbieaverill :)

[robbieaverill]

Deploying to UAT now

[robbieaverill]

UAT good, will deploy to production a bit later today

[robbieaverill]

Deploying now - we'll leave the cron to pick up the run overnight =)