mooz / js2-mode

Improved JavaScript editing mode for GNU Emacs
GNU General Public License v3.0
1.33k stars 186 forks source link

js2-mode doesn't recognize LS and PS line endings, leading to incorrect syntax highlighting #597

Open cpitclaudel opened 1 year ago

cpitclaudel commented 1 year ago

Javascript allows LS and PS as line endings, but js2-mode doesn't recognize them and hence produces incorrect highlighting:

Screenshot from 2023-07-22 18-46-47

(This is a JS-mode copy of the bug at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=64792 )

dgutov commented 1 year ago

Hi Clement!

This is probably harder to fix in js-mode, which relies on what font-lock considers a newline.

Here's a quick patch you can try, though there are probably more places that will need updating:

diff --git a/js2-mode.el b/js2-mode.el
index 11ebb37..d53c399 100644
--- a/js2-mode.el
+++ b/js2-mode.el
@@ -5585,16 +5585,17 @@ Increments `js2-ts-lineno' if the return value is a newline char.
 Updates `js2-ts-cursor' to the point after the returned char.
 Returns `js2-EOF_CHAR' if we hit the end of the buffer.
 Also updates `js2-ts-hit-eof' and `js2-ts-line-start' as needed."
+  (defvar js2-eol-chars)
   (let (c)
     ;; check for end of buffer
     (if (>= js2-ts-cursor (point-max))
         (setq js2-ts-hit-eof t
               js2-ts-cursor (1+ js2-ts-cursor)
-              c js2-EOF_CHAR)  ; return value
+              c js2-EOF_CHAR)           ; return value
       ;; otherwise read next char
       (setq c (char-before (cl-incf js2-ts-cursor)))
       ;; if we read a newline, update counters
-      (if (= c ?\n)
+      (if (memq c js2-eol-chars)
           (setq js2-ts-line-start js2-ts-cursor
                 js2-ts-lineno (1+ js2-ts-lineno)))
       ;; TODO:  skip over format characters
@@ -5670,7 +5671,7 @@ See http://es5.github.io/#x7.6"
      ;; TODO:  change this nil to check for Unicode space character
      nil)))

-(defconst js2-eol-chars (list js2-EOF_CHAR ?\n ?\r))
+(defconst js2-eol-chars (list js2-EOF_CHAR ?\n ?\r ?\u2028 ?\u2029))

 (defun js2-skip-line ()
   "Skip to end of line."

These details aside, do you expect to be using this distinction? Is there JS code in the wild relying on this?

cpitclaudel commented 1 year ago

Thanks!

These details aside, do you expect to be using this distinction? Is there JS code in the wild relying on this?

No, I don't expect to write any such code myself — it's more of a security thing.

dgutov commented 1 year ago

All right.

Then should we go for a more obvious warning via font-lock rather than parsing the code in a different way? This could be done in both js-mode and js2-mode fairly easily: just highlight every such line from the encountered character until eol with the warning face or something.

cpitclaudel commented 1 year ago

Then should we go for a more obvious warning via font-lock rather than parsing the code in a different way?

That would be a reasonably good workaround, I think.

UwUnyaa commented 1 year ago

Considering this class of attacks exists in a bunch of languages, maybe it would be a good idea to report this sort of thing upstream so it can be handled in a consistent way across major modes?

dgutov commented 1 year ago

maybe it would be a good idea to report this sort of thing upstream so it can be handled in a consistent way across major modes?

That could be a good addition to the original report, yes.