Change default Freemarker template file extension to .ftlh to enable HTML escaping

[jamesbarnett91]

Hi

When using the Spring Boot Freemarker Starter, content within the standard Freemarker interpolation blocks ${...} is not escaped by default.

The Freemarker docs say:

FreeMarker automatically escapes all values printed with ${...} if it's properly configured (that's the responsibility of the programmers). The recommended practice is using ftlh file extension to activate HTML auto-escaping, and ftlx file extension to activate XML auto-escaping.

I think most users would expect the Spring Boot Starter to do this 'proper configuration', since it does a lot of other sensible auto configuration, and I can't see why you would ever want to globally disable HTML escaping except in very rare circumstances.

Additionally, the recommended practice of using the .ftlh file extension to enable HTML auto-escaping does not work out-of-the-box with the Spring Boot starter as it seems to only be configured to scan for .ftl files.

The safest way to enable escaping seems to be to set spring.freemarker.settings.output_format=HTMLOutputFormat within the application.properties to force Freemarker to treat all templates as HTML regardless of their extension (so escaping will be done even if the h is forgotten on the file extension).

Or alternatively the spring.freemarker.suffix can also be set to .ftlh along with the enabling the spring.freemarker.settings.recognize_standard_file_extension

I would like to suggest that the default behaviour be changed to enable escaping by setting the Freemarker output format to HTML. However I appreciate the potential regression impact of making this change, so understand if this is not possible.
At the least, I would suggest that some mention of the need to do this configuration should be included in the documentation. Perhaps a list of recommended configuration options for each Templating engine or something?

Currently everything else about the default configuration is sensible and works well, so users are unlikely to examine any other configuration options, making it very easy to miss the need to configure the escaping and likely leading to XSS vulnerabilities.

For comparison, both the Thymeleaf and Mustache templating starters for Spring Boot escape html content by out-of-the-box when using the standard interpolation syntax.

Tested with spring-boot-2.0.4.RELEASE and spring-boot-starter-freemarker:2.0.4.RELEASE

Thanks

[bclozel]

Thanks for the report @jamesbarnett91 !

FreeMarker users should indeed apply those defaults for better security. Spring Boot auto-configures FreeMarker for Web and non-Web use cases (rendering email templates or text files, etc).

As you've described here, changing the default for spring.freemarker.suffix will break existing applications, since people will have to rename their template files. On the other hand, setting spring.freemarker.settings.output_format=HTMLOutputFormat by default will change the format even if you're not writing HTML templates; this is also a breaking change.

The team discussed this and we've decided to first tackle #15156 and then decide what we should do here.

[jamesbarnett91]

Thanks for the update @bclozel , I understand the need to maintain backwards compatibility.

Perhaps the Spring Initializr could be updated to apply the Freemarker output_format=HTMLOutputFormat property automatically? Then at least new projects will be safe by default.

This could only be applied if the Web dependency is selected to avoid setting the default format for non HTML templating applications.

If this sounds sensible I can raise an issue on the Spring Initializr repo?
Thanks

[snicoll]

@jamesbarnett91 this issue is not closed and we're going to address this, we just don't know how yet. I wouldn't update Spring Intializr as it must remain a convenience.

[bclozel]

Given the conclusion reached in #15156, we should make sure that Freemarker provides a safe configuration for web templates.

In the case of Freemarker, the prefix/suffix data is only used in the view resolver, so changing
those won't impact the non-web use case. We should then change the default suffix from .ftl
to .ftlh and configure Freemarker to associate the proper output format by setting the
recognize_standard_file_extensions Freemarker option to true by default. See Freemarker reference docs.

This is a breaking change, but we've made similar changes in the past, see #8997.

[jamesbarnett91]

Thanks for the fix @bclozel 👍

[janbodnar]

This is a breaking change which leads to the infamous 'whitelabel error' and is quite confusing. There is not a single line about this in the docs: https://docs.spring.io/spring-boot/docs/2.2.2.RELEASE/reference/htmlsingle/#boot-features-spring-mvc-template-engines
Only after a couple of hours I spotted the ftlh extension there which made me curious and finally brought me here.

In my opinion, breaking changes should be only reserved for major updates and should have red notes in the docs.

[wilkinsona]

Thanks for the feedback and sorry for the inconvenience. We occasionally make breaking changes in minor releases when we believe that it will not make upgrading too difficult. We communicate those changes in the release notes rather than in the reference documentation. The latter is intended to describe how things are for a particular release whereas the former are intended to describe how things have changed since the previous release.

[duttonw]

Please add it to the spring boot 2.2 release notes as it also caught my group offguard i.e. why is nothing showing up.. https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.2-Release-Notes
or set back to old way by adding spring.freemarker.suffix: ".ftl" but best to be on ".ftlh" for automatic html escaping etc.

[philwebb]

@duttonw It's covered in the release notes under the Freemarker templates configuration section.

[duttonw]

to make my templates work again i found https://freemarker.apache.org/docs/dgui_misc_autoescaping.html

I removed strings <#escape x as x?xhtml> ... </#escape>
and swapped <#noescape> ... </#noescape> to <#noautoesc> ... </#noautoesc>
and stripped ?xhtml or ?html off variables

there might be other things needing to be done, but this should be the gist of it if they are global wrapping their templates instead of piece meal escaping.