FEATURE: Support upload:// urls in img tags (#16277)

Previously, our `upload://` protocol urls were only supported in markdown image tags. This meant that our PullHotlinkedImages job was forced to convert `<img` tags to markdown. Depending on the exact syntax, this can actually cause the image to break.

This commit adds support for `upload://` inside regular HTML `<img` tags. In a future commit, we'll be able to use this to make our PullHotlinkedImages job much more robust.

Context at https://meta.discourse.org/t/152801

app/assets/javascripts/pretty-text/engines/discourse-markdown/upload-protocol.js

@@ -1,49 +1,118 @@
+import xss from "xss";
+
+const HTML_TYPES = ["html_block", "html_inline"];
+
 // add image to array if src has an upload
 function addImage(uploads, token) {
   if (token.attrs) {
     for (let i = 0; i < token.attrs.length; i++) {
       if (token.attrs[i][1].indexOf("upload://") === 0) {
-        uploads.push([token, i]);
+        uploads.push({ token, srcIndex: i, origSrc: token.attrs[i][1] });
         break;
       }
     }
   }
 }
 
+function attr(name, value) {
+  if (value) {
+    return `${name}="${xss.escapeAttrValue(value)}"`;
+  }
+
+  return name;
+}
+
+function uploadLocatorString(url) {
+  return `___REPLACE_UPLOAD_SRC_${url}___`;
+}
+
+function findUploadsInHtml(uploads, blockToken) {
+  // Slightly misusing our HTML sanitizer to look for upload://
+  // image src attributes, and replace them with a placeholder.
+  // Note that we can't use browser DOM APIs because this needs
+  // to run in mini-racer.
+  const fakeAllowList = {};
+
+  let foundImage = false;
+  const newContent = xss(blockToken.content, {
+    whiteList: fakeAllowList,
+    allowCommentTag: true,
+    onTag(tag, html, options) {
+      // We're not using this for sanitizing, so allow all tags through
+      options.isWhite = true;
+      fakeAllowList[tag] = [];
+    },
+    onTagAttr(tag, name, value) {
+      if (tag === "img" && name === "src" && value.startsWith("upload://")) {
+        uploads.push({ token: blockToken, srcIndex: null, origSrc: value });
+        foundImage = true;
+        return uploadLocatorString(value);
+      }
+      return attr(name, value);
+    },
+  });
+  if (foundImage) {
+    blockToken.content = newContent;
+  }
+}
+
+function processToken(uploads, token) {
+  if (token.tag === "img" || token.tag === "a") {
+    addImage(uploads, token);
+  } else if (HTML_TYPES.includes(token.type)) {
+    findUploadsInHtml(uploads, token);
+  }
+
+  if (token.children) {
+    for (let j = 0; j < token.children.length; j++) {
+      const childToken = token.children[j];
+      processToken(uploads, childToken);
+    }
+  }
+}
+
 function rule(state) {
   let uploads = [];
 
   for (let i = 0; i < state.tokens.length; i++) {
     let blockToken = state.tokens[i];
 
-    if (blockToken.tag === "img" || blockToken.tag === "a") {
+    processToken(uploads, blockToken);
-      addImage(uploads, blockToken);
-    }
-
-    if (!blockToken.children) {
-      continue;
-    }
-
-    for (let j = 0; j < blockToken.children.length; j++) {
-      let token = blockToken.children[j];
-
-      if (token.tag === "img" || token.tag === "a") {
-        addImage(uploads, token);
-      }
-    }
   }
 
   if (uploads.length > 0) {
-    let srcList = uploads.map(([token, srcIndex]) => token.attrs[srcIndex][1]);
+    let srcList = uploads.map((u) => u.origSrc);
+
+    // In client-side cooking, this lookup returns nothing
+    // This means we set data-orig-src, and let decorateCooked
+    // lookup the image URLs asynchronously
     let lookup = state.md.options.discourse.lookupUploadUrls;
     let longUrls = (lookup && lookup(srcList)) || {};
 
-    uploads.forEach(([token, srcIndex]) => {
+    uploads.forEach(({ token, srcIndex, origSrc }) => {
-      let origSrc = token.attrs[srcIndex][1];
       let mapped = longUrls[origSrc];
 
-      switch (token.tag) {
+      if (HTML_TYPES.includes(token.type)) {
-        case "img":
+        const locator = uploadLocatorString(origSrc);
+        let attrs = [];
+
+        if (mapped) {
+          attrs.push(
+            attr("src", mapped.url),
+            attr("data-base62-sha1", mapped.base62_sha1)
+          );
+        } else {
+          attrs.push(
+            attr(
+              "src",
+              state.md.options.discourse.getURL("/images/transparent.png")
+            ),
+            attr("data-orig-src", origSrc)
+          );
+        }
+
+        token.content = token.content.replace(locator, attrs.join(" "));
+      } else if (token.tag === "img") {
         if (mapped) {
           token.attrs[srcIndex][1] = mapped.url;
           token.attrs.push(["data-base62-sha1", mapped.base62_sha1]);
@@ -61,8 +130,7 @@ function rule(state) {
 
           token.attrs.push(["data-orig-src", origSrc]);
         }
-          break;
+      } else if (token.tag === "a") {
-        case "a":
         if (mapped) {
           // when secure media is enabled we want the full /secure-media-uploads/
           // url to take advantage of access control security
@@ -75,14 +143,10 @@ function rule(state) {
             token.attrs[srcIndex][1] = mapped.short_path;
           }
         } else {
-            token.attrs[srcIndex][1] = state.md.options.discourse.getURL(
+          token.attrs[srcIndex][1] = state.md.options.discourse.getURL("/404");
-              "/404"
-            );
 
           token.attrs.push(["data-orig-href", origSrc]);
         }
-
-          break;
       }
     });
   }

spec/lib/pretty_text_spec.rb

@@ -1877,6 +1877,12 @@ HTML
 
       ![upload](#{upload.short_url.gsub(".png", "")})
 
+      Inline img <img src="#{upload.short_url}">
+
+      <div>
+        Block img <img src="#{upload.short_url}">
+      </div>
+
       [some attachment](#{upload.short_url})
 
       [some attachment|attachment](#{upload.short_url})
@@ -1901,6 +1907,10 @@ HTML
         </li>
         </ul>
         <p><img src="#{cdn_url}" alt="upload" data-base62-sha1="#{upload.base62_sha1}"></p>
+        <p>Inline img <img src="#{cdn_url}" data-base62-sha1="#{upload.base62_sha1}"></p>
+        <div>
+          Block img <img src="#{cdn_url}" data-base62-sha1="#{upload.base62_sha1}">
+        </div>
         <p><a href="#{upload.short_path}">some attachment</a></p>
         <p><a class="attachment" href="#{upload.short_path}">some attachment</a></p>
         <p><a href="#{upload.short_path}">some attachment|random</a></p>