Side-by-side diff breaks on carriage returns

Issue #902 resolved
reima created an issue

Problem

When a diff contains a carriage return character (\r), the side-by-side diff page is missing the actual diff section and a JavaScript error is logged ("Uncaught SyntaxError: Unexpected token ILLEGAL" on Chrome, "SyntaxError: unterminated string literal" on Firefox) on the line starting with var orig1 =.

The reason is that carriage returns are pasted verbatim into the JavaScript code, where they are interpreted as line breaks inside a string literal.

Quick solution

A quick fix would be to add the entry

'\r': '\\r',

to the html_escape_table in FilesController.diff_2way.

Preferred solution

However, a more robust solution would be to escape everything outside the ASCII printable characters range (32 to 126) using unicode escape sequences.

For reference, the Apache Commons Lang library provides a method escapeEcmaScript, which uses an equivalent to the following translation table (see 1 and 2):

html_escape_table = {
    "'": "\\'",
    "\"": "\\\"",
    "\\": "\\\\",
    "/": "\\/",
    "\b": "\\b",
    "\n": "\\n",
    "\t": "\\t",
    "\f": "\\f",
    "\r": "\\r",
}

It additionally replaces characters outside the ASCII printable characters range.

Alternate solution

As an alternative suggestion, one could also circumvent the problem by not embedding the diff contents at all. Instead, the file contents at the two revisions could be loaded via additional AJAX requests. A "proof of concept" patch for this solution is attached (it doesn't handle binary files for example).

Comments (1)

  1. Marcin Kuzminski repo owner

    This is now fixed and merged into stable branch, we load files via ajax for two way compare.

  2. Log in to comment