Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
cpitclaudel opened this issue Jul 22, 2023 · 6 comments

Comments

@cpitclaudel
Copy link

cpitclaudel commented Jul 22, 2023

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
Copy link
Collaborator

dgutov commented Jul 23, 2023

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
Copy link
Author

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
Copy link
Collaborator

dgutov commented Jul 29, 2023

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
Copy link
Author

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
Copy link

UwUnyaa commented Aug 13, 2023

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
Copy link
Collaborator

dgutov commented Aug 16, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants