Kickstart include not handling escaped Unicode characters properly

voidmain commented 1 year ago

Kickstart include not handling Unicode characters properly

Description

In a Kickstart file, if you include another file that contains escaped Unicode characters (i.e. \u2026), they get mangled and do not render properly.

Affects versions

1.40.x

Steps to reproduce

Steps to reproduce the behavior:

Create a Kickstart file and have it create a new theme (see JSON below)
In the theme, include a file for the messages properties file
In the messages, include an escaped character such as \u2026
Open the theme up in the admin UI and edit the messages. You'll see that it gets mangled to 2026

Expected behavior

The kickstart include should not modify the escapes.

Example JSON

    {
      "method": "POST",
      "url": "/api/theme/a499b51c-b7d6-4ceb-87a9-bf27a01014d3",
      "body": {
        "theme": {
          "name": "Example theme",
          "defaultMessages": "@{theme/messages.properties}",
          "stylesheet": "@{theme/stylesheet.css}",
          "templates": {
            "accountEdit": "@{theme/account/edit.ftl}",
            "accountIndex": "@{theme/account/index.ftl}",
            "accountTwoFactorDisable": "@{theme/account/two-factor/disable.ftl}",
            "accountTwoFactorEnable": "@{theme/account/two-factor/enable.ftl}",
            "accountTwoFactorIndex": "@{theme/account/two-factor/index.ftl}",
            "emailComplete": "@{theme/email/verification-complete.ftl}",
            "emailSent": "@{theme/email/verification-resent.ftl}",
            "emailVerificationRequired": "@{theme/email/verification-required.ftl}",
            "emailVerify": "@{theme/email/verification.ftl}",
            "helpers": "@{theme/_helpers.ftl}",
            "index": "@{theme/index.ftl}",
            "oauth2Authorize": "@{theme/oauth2/authorize.ftl}",
            "oauth2AuthorizedNotRegistered": "@{theme/oauth2/authorized-not-registered.ftl}",
            "oauth2ChildRegistrationNotAllowed": "@{theme/oauth2/child-registration-not-allowed.ftl}",
            "oauth2ChildRegistrationNotAllowedComplete": "@{theme/oauth2/child-registration-not-allowed-complete.ftl}",
            "oauth2CompleteRegistration": "@{theme/oauth2/complete-registration.ftl}",
            "oauth2Device": "@{theme/oauth2/device.ftl}",
            "oauth2DeviceComplete": "@{theme/oauth2/device-complete.ftl}",
            "oauth2Error": "@{theme/oauth2/error.ftl}",
            "oauth2Logout": "@{theme/oauth2/logout.ftl}",
            "oauth2Passwordless": "@{theme/oauth2/passwordless.ftl}",
            "oauth2Register": "@{theme/oauth2/register.ftl}",
            "oauth2StartIdPLink": "@{theme/oauth2/start-idp-link.ftl}",
            "oauth2TwoFactor": "@{theme/oauth2/two-factor.ftl}",
            "oauth2TwoFactorMethods": "@{theme/oauth2/two-factor-methods.ftl}",
            "oauth2Wait": "@{theme/oauth2/wait.ftl}",
            "passwordChange": "@{theme/password/change.ftl}",
            "passwordComplete": "@{theme/password/complete.ftl}",
            "passwordForgot": "@{theme/password/forgot.ftl}",
            "passwordSent": "@{theme/password/sent.ftl}",
            "registrationComplete": "@{theme/registration/verification-complete.ftl}",
            "registrationSent": "@{theme/registration/verification-resent.ftl}",
            "registrationVerificationRequired": "@{theme/registration/verification-required.ftl}",
            "registrationVerify": "@{theme/registration/verification.ftl}",
            "samlv2Logout": "@{theme/samlv2/logout.ftl}",
            "unauthorized": "@{theme/unauthorized.ftl}"
          }
        }
      }
    }

Doc tasks

[ ] Expand doc for Kickstart escape sequences
[ ] Check for Kickstart examples that show escape sequence processing that was updated

robotdan commented 1 year ago

Not sure if I know how to solve this, we may need some additional rules around includes.

In the following example CSS include:

/* A literal unicode character */
.example {
  content: '\u2026';
}

/* A comment with unicode literal: \u2026 */

/* A comment with an escape that looks like unicode: \\u2026 */

Currently this renders as

/* A literal unicode character */
.example {
  content: '2026';
}

/* A comment with unicode literal: 2026 */

/* A comment with an escape that looks like unicode: u2026 */"

Not sure if we need to treat escapes differently within a comment, but if we do that we'd need to know the file type to know what a comment means.

We should discuss further.

voidmain commented 1 year ago

I think we should treat everything as raw text with no escaping. For your example above, the result in the database should be:

/* A literal unicode character */
.example {
  content: '\u2026';
}

/* A comment with unicode literal: \u2026 */

/* A comment with an escape that looks like unicode: \\u2026 */"

When this is spit out in the FTL (or when we update the themes to spit out a CSS file directly), then it would retain those same escapes and the browser will be happy.

This should hold true for email templates, message templates, and included JSON as well (depending on how JSON is included).

spwitt commented 1 year ago

Internal

https://github.com/FusionAuth/fusionauth-app/pull/188

spwitt commented 1 year ago

The PR was abandoned due to concerns with breaking existing Kickstart files. If/when this issue is revisited, it probably makes sense to add a version to the Kickstart spec. Without a specific version called out in the Kickstart file, it should retain the current behavior. One of the biggest changes to the spec should be a new escape character (% may be a good option). The Kickstart files are parsed as JSON, which requires \\ in the file to begin an escape sequence. This can be difficult to keep straight while parsing.

If unicode or other escaped characters are needed in an HTML include, the current version supports a workaround by either:

Entering the actual character instead of the escape sequence
Use the HTML escape sequence (e.g. … instead of \u2026)

FusionAuth / fusionauth-issues