Fix #4772: Set all reader mode content as text instead of HTML (#4773)

Co-authored-by: Brandon T <JustBrandonT@gmail.com>
brave · Jan 4, 2022 · 1eb4afc · 1eb4afc
1 parent b954779
commit 1eb4afc
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 14 deletions.
diff --git a/Client/Frontend/Reader/Reader.html b/Client/Frontend/Reader/Reader.html
@@ -12,23 +12,22 @@
   <title id="reader-page-title"></title>
 </head>
 
-<body data-readerStyle='%READER-STYLE%'>
+<body>
   <div id="reader-header" class="header">
     <h1 id="reader-title"></h1>
-    <div id="reader-credits" class="credits">%READER-CREDITS%</div>
+    <div id="reader-credits" class="credits"></div>
     <script type="text/javascript" src="/reader-mode/javascript/Reader.js"></script>
-    <script nonce="%READER-TITLE-NONCE%">
-        document.getElementById("reader-page-title").textContent = "%READER-TITLE%";
-        document.getElementById("reader-title").textContent = "%READER-TITLE%";
-    </script>
   </div>
 
   <div id="reader-content" class="content">
-    %READER-CONTENT%
+      <!--
+          This is already escaped.
+          Any scripts attempting to run here will be blocked by the content security policy as they don't have the same NONCE set in the native Swift code
+      -->
+      %READER-CONTENT%
   </div>
 
   <div id="reader-message" class="message">
-    %READER-MESSAGE%
   </div>
 
   <ul id="reader-toolbar" class="toolbar toolbar-hidden">
@@ -46,6 +45,14 @@ <h1 id="reader-title"></h1>
     <li><a id="toggle-button" class="button toggle-button" href="#"></a></li>
   </ul>
 
+  <script nonce="%READER-TITLE-NONCE%">
+      document.body.setAttribute('data-readerStyle', '%READER-STYLE%');
+      document.getElementById("reader-page-title").textContent = "%READER-TITLE%";
+      document.getElementById("reader-title").textContent = "%READER-TITLE%";
+      document.getElementById("reader-credits").textContent = "%READER-CREDITS%";
+      document.getElementById("reader-message").textContent = "%READER-MESSAGE%";
+  </script>
+
 </body>
 
 </html>
diff --git a/Client/Frontend/Reader/ReaderMode.swift b/Client/Frontend/Reader/ReaderMode.swift
@@ -166,7 +166,7 @@ struct ReadabilityResult {
                 self.content = content
             }
             if let title = dict["title"] as? String {
-                self.title = title.htmlEntityEncodedString
+                self.title = title
             }
             if let credits = dict["byline"] as? String {
                 self.credits = credits
@@ -182,7 +182,7 @@ struct ReadabilityResult {
         let domain = object["domain"].string
         let url = object["url"].string
         let content = object["content"].string
-        let title = object["title"].string?.htmlEntityEncodedString
+        let title = object["title"].string
         let credits = object["credits"].string
 
         if domain == nil || url == nil || content == nil || title == nil || credits == nil {

diff --git a/Client/Frontend/Reader/ReaderModeUtils.swift b/Client/Frontend/Reader/ReaderModeUtils.swift
@@ -20,14 +20,29 @@ struct ReaderModeUtils {
             let tmplPath = Bundle.main.path(forResource: "Reader", ofType: "html"),
             let tmpl = try? String(contentsOfFile: tmplPath, encoding: .utf8) else { return nil }
 
-        return tmpl.replacingOccurrences(of: "%READER-CSS%", with: css)
+        return tmpl.replacingOccurrences(of: "%READER-TITLE-NONCE%", with: titleNonce) // This MUST be the first line/replacement!
+
+            .replacingOccurrences(of: "%READER-CSS%", with: css)
             .replacingOccurrences(of: "%READER-STYLE%", with: initialStyle.encode())
             .replacingOccurrences(of: "%READER-DOMAIN%", with: simplifyDomain(readabilityResult.domain))
             .replacingOccurrences(of: "%READER-URL%", with: readabilityResult.url)
-            .replacingOccurrences(of: "%READER-TITLE%", with: readabilityResult.title)
-            .replacingOccurrences(of: "%READER-CREDITS%", with: readabilityResult.credits)
+            .replacingOccurrences(of: "%READER-TITLE%", with: readabilityResult.title.javaScriptEscapedString?.unquotedIfNecessary ?? readabilityResult.title.htmlEntityEncodedString)
+            .replacingOccurrences(of: "%READER-CREDITS%", with: readabilityResult.credits.javaScriptEscapedString?.unquotedIfNecessary ?? readabilityResult.credits.htmlEntityEncodedString)
             .replacingOccurrences(of: "%READER-CONTENT%", with: readabilityResult.content)
-            .replacingOccurrences(of: "%READER-TITLE-NONCE%", with: titleNonce)
+            .replacingOccurrences(of: "%READER-MESSAGE%", with: "")
+    }
+}
 
+private extension String {
+    var unquotedIfNecessary: String {
+        var str = self
+        if str.first == "\"" || str.first == "'" {
+            str = String(str.dropFirst())
+        }
+
+        if str.last == "\"" || str.last == "'" {
+            str = String(str.dropLast())
+        }
+        return str
     }
 }