From feced339ea17d66deffbac8eebe911c068fb2fa5 Mon Sep 17 00:00:00 2001
From: Wilson Lin <code@wilsonl.in>
Date: Thu, 15 Apr 2021 00:18:16 +1000
Subject: [PATCH] Escape unintentional script closing tag in JS

---
 bench/minifiers.js |  2 +-
 src/proc/mod.rs    | 28 ++++++++++++++++++----------
 src/tests/mod.rs   |  7 +++++++
 src/unit/script.rs | 18 ++++++++++--------
 4 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/bench/minifiers.js b/bench/minifiers.js
index 32acc15..5faf7d1 100644
--- a/bench/minifiers.js
+++ b/bench/minifiers.js
@@ -43,7 +43,7 @@ class EsbuildAsync {
 
   async finalise (html) {
     const jsTransformResults = await Promise.all(this.promises);
-    return html.replace(/_____ESBUILD_ASYNC_PLACEHOLDER_([0-9]+)_____/g, (_, id) => jsTransformResults[id].code);
+    return html.replace(/_____ESBUILD_ASYNC_PLACEHOLDER_([0-9]+)_____/g, (_, id) => jsTransformResults[id].code.replace(/<\/script/g, "<\\/script"));
   }
 }
 
diff --git a/src/proc/mod.rs b/src/proc/mod.rs
index 5904fe1..f276ec5 100644
--- a/src/proc/mod.rs
+++ b/src/proc/mod.rs
@@ -3,21 +3,21 @@ use std::fmt::{Debug, Formatter};
 use std::ops::{Index, IndexMut};
 
 use aho_corasick::AhoCorasick;
-
-use crate::err::{Error, ErrorType, ProcessingResult, debug_repr};
-use crate::proc::MatchAction::*;
-use crate::proc::MatchMode::*;
-use crate::proc::range::ProcessorRange;
 use memchr::memchr;
-use crate::gen::codepoints::Lookup;
 
 #[cfg(feature = "js-esbuild")]
 use {
-    std::sync::{Arc, Mutex},
     crossbeam::sync::WaitGroup,
     esbuild_rs::TransformResult,
+    std::sync::{Arc, Mutex},
 };
 
+use crate::err::{debug_repr, Error, ErrorType, ProcessingResult};
+use crate::gen::codepoints::Lookup;
+use crate::proc::MatchAction::*;
+use crate::proc::MatchMode::*;
+use crate::proc::range::ProcessorRange;
+
 pub mod checkpoint;
 pub mod entity;
 pub mod range;
@@ -383,9 +383,17 @@ impl<'d> Processor<'d> {
         let mut write_next = results.get(0).map_or(self.write_next, |r| r.src.start);
         for (i, EsbuildSection { result, src }) in results.iter().enumerate() {
             // Resulting minified JS/CSS to write.
-            // TODO Verify.
-            // TODO Handle potential `</script>` in output code, which could be in string (e.g. orig. "</" + "script>"), comment, or expression (e.g. orig. `a < /script>/.exec(b)?.length`).
-            let min_code = result.code.as_str().trim();
+            // TODO Handle other forms:
+            // 1 < /script/.exec(a).length
+            // `  ${`  ${a</script/}  `}  `
+            // // </script>
+            // /* </script>
+            // Considerations:
+            // - Need to parse strings (e.g. "", '', ``) so syntax within strings aren't mistakenly interpreted as code.
+            // - Need to be able to parse regex literals to determine string delimiters aren't actually characters in the regex.
+            // - Determining whether a slash is division or regex requires a full-blown JS parser to handle all cases (this is a well-known JS parsing problem).
+            // - `/</script` or `/</ script` are not valid JS so don't need to be handled.
+            let min_code = result.code.as_str().trim().replace("</script", "<\\/script");
             let min_len = if min_code.len() < src.len() {
                 self.code[write_next..write_next + min_code.len()].copy_from_slice(min_code.as_bytes());
                 min_code.len()
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
index 3b5502f..9b9516e 100644
--- a/src/tests/mod.rs
+++ b/src/tests/mod.rs
@@ -450,6 +450,13 @@ fn test_js_minification() {
     "#, b"<script>let a=1;</script>");
 }
 
+#[cfg(feature = "js-esbuild")]
+#[test]
+fn test_js_minification_unintentional_closing_tag() {
+    eval_with_js_min(br#"<script>let a = "</" + "script>";</script>"#, br#"<script>let a="<\/script>";</script>"#);
+    eval_with_js_min(br#"<script>let a = "\u003c/script>";</script>"#, br#"<script>let a="<\/script>";</script>"#);
+}
+
 #[cfg(feature = "js-esbuild")]
 #[test]
 fn test_css_minification() {
diff --git a/src/unit/script.rs b/src/unit/script.rs
index 091da90..e519cf2 100644
--- a/src/unit/script.rs
+++ b/src/unit/script.rs
@@ -1,17 +1,19 @@
 use aho_corasick::{AhoCorasick, AhoCorasickBuilder};
 use lazy_static::lazy_static;
+
+#[cfg(feature = "js-esbuild")]
+use {
+    crate::proc::checkpoint::WriteCheckpoint,
+    crate::proc::EsbuildSection,
+    esbuild_rs::{TransformOptions, TransformOptionsBuilder},
+    std::sync::Arc,
+};
+
 use crate::cfg::Cfg;
 use crate::err::ProcessingResult;
 use crate::proc::MatchAction::*;
 use crate::proc::MatchMode::*;
 use crate::proc::Processor;
-#[cfg(feature = "js-esbuild")]
-use {
-    std::sync::Arc,
-    esbuild_rs::{TransformOptionsBuilder, TransformOptions},
-    crate::proc::EsbuildSection,
-    crate::proc::checkpoint::WriteCheckpoint,
-};
 
 #[cfg(feature = "js-esbuild")]
 lazy_static! {
@@ -31,7 +33,7 @@ lazy_static! {
 #[inline(always)]
 pub fn process_script(proc: &mut Processor, cfg: &Cfg, js: bool) -> ProcessingResult<()> {
     #[cfg(feature = "js-esbuild")]
-    let start = WriteCheckpoint::new(proc);
+        let start = WriteCheckpoint::new(proc);
     proc.require_not_at_end()?;
     proc.m(WhileNotSeq(&SCRIPT_END), Keep);
     // `process_tag` will require closing tag.