FEATURE: Support [description|attachment](upload://<short-sha>) in MD. (#7603)

app/assets/javascripts/discourse/components/composer-editor.js.es6

@@ -36,7 +36,7 @@ import {
 import {
   cacheShortUploadUrl,
   resolveAllShortUrls
-} from "pretty-text/image-short-url";
+} from "pretty-text/upload-short-url";
 
 import {
   INLINE_ONEBOX_LOADING_CSS_CLASS,

app/assets/javascripts/discourse/components/cook-text.js.es6

@@ -13,7 +13,7 @@ const CookText = Ember.Component.extend({
       // pretty text may only be loaded now
       Ember.run.next(() =>
         window
-          .requireModule("pretty-text/image-short-url")
+          .requireModule("pretty-text/upload-short-url")
           .resolveAllShortUrls(ajax)
       );
     });

app/assets/javascripts/discourse/lib/utilities.js.es6

@@ -444,15 +444,9 @@ export function getUploadMarkdown(upload) {
   ) {
     return uploadLocation(upload.url);
   } else {
-    return (
-      '<a class="attachment" href="' +
-      upload.url +
-      '">' +
-      upload.original_filename +
-      "</a> (" +
-      I18n.toHumanSize(upload.filesize) +
-      ")\n"
-    );
+    return `[${upload.original_filename} (${I18n.toHumanSize(
+      upload.filesize
+    )})|attachment](${upload.short_url})`;
   }
 }
 

app/assets/javascripts/markdown-it-bundle.js

@@ -14,6 +14,6 @@
 //= require ./pretty-text/engines/discourse-markdown/newline
 //= require ./pretty-text/engines/discourse-markdown/html-img
 //= require ./pretty-text/engines/discourse-markdown/text-post-process
-//= require ./pretty-text/engines/discourse-markdown/image-protocol
+//= require ./pretty-text/engines/discourse-markdown/upload-protocol
 //= require ./pretty-text/engines/discourse-markdown/inject-line-number
 //= require ./pretty-text/engines/discourse-markdown/d-wrap

app/assets/javascripts/pretty-text-bundle.js

@@ -11,4 +11,4 @@
 //= require ./pretty-text/sanitizer
 //= require ./pretty-text/oneboxer
 //= require ./pretty-text/inline-oneboxer
-//= require ./pretty-text/image-short-url
+//= require ./pretty-text/upload-short-url

app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6

@@ -1,6 +1,7 @@
 import { default as WhiteLister } from "pretty-text/white-lister";
 import { sanitize } from "pretty-text/sanitizer";
 import guid from "pretty-text/guid";
+import { ATTACHMENT_CSS_CLASS } from "pretty-text/upload-short-url";
 
 function deprecate(feature, name) {
   return function() {
@@ -187,6 +188,26 @@ function setupImageDimensions(md) {
   md.renderer.rules.image = renderImage;
 }
 
+function renderAttachment(tokens, idx, options, env, slf) {
+  const linkOpenToken = tokens[idx];
+  const linkTextToken = tokens[idx + 1];
+  const split = linkTextToken.content.split("|");
+  const isValid = !linkOpenToken.attrs[
+    linkOpenToken.attrIndex("data-orig-href")
+  ];
+
+  if (isValid && split.length === 2 && split[1] === ATTACHMENT_CSS_CLASS) {
+    linkOpenToken.attrs.unshift(["class", split[1]]);
+    linkTextToken.content = split[0];
+  }
+
+  return slf.renderToken(tokens, idx, options);
+}
+
+function setupAttachments(md) {
+  md.renderer.rules.link_open = renderAttachment;
+}
+
 let Helpers;
 
 export function setup(opts, siteSettings, state) {
@@ -276,6 +297,7 @@ export function setup(opts, siteSettings, state) {
   setupUrlDecoding(opts.engine);
   setupHoister(opts.engine);
   setupImageDimensions(opts.engine);
+  setupAttachments(opts.engine);
   setupBlockBBCode(opts.engine);
   setupInlineBBCode(opts.engine);
   setupTextPostProcessRuler(opts.engine);

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

@@ -1,62 +0,0 @@
-// add image to array if src has an upload
-function addImage(images, token) {
-  if (token.attrs) {
-    for (let i = 0; i < token.attrs.length; i++) {
-      if (token.attrs[i][1].indexOf("upload://") === 0) {
-        images.push([token, i]);
-        break;
-      }
-    }
-  }
-}
-
-function rule(state) {
-  let images = [];
-
-  for (let i = 0; i < state.tokens.length; i++) {
-    let blockToken = state.tokens[i];
-
-    if (blockToken.tag === "img") {
-      addImage(images, blockToken);
-    }
-
-    if (!blockToken.children) {
-      continue;
-    }
-
-    for (let j = 0; j < blockToken.children.length; j++) {
-      let token = blockToken.children[j];
-      if (token.tag === "img") {
-        addImage(images, token);
-      }
-    }
-  }
-
-  if (images.length > 0) {
-    let srcList = images.map(([token, srcIndex]) => token.attrs[srcIndex][1]);
-    let lookup = state.md.options.discourse.lookupImageUrls;
-    let longUrls = (lookup && lookup(srcList)) || {};
-
-    images.forEach(([token, srcIndex]) => {
-      let origSrc = token.attrs[srcIndex][1];
-      let mapped = longUrls[origSrc];
-      if (mapped) {
-        token.attrs[srcIndex][1] = mapped;
-      } else {
-        token.attrs[srcIndex][1] = state.md.options.discourse.getURL(
-          "/images/transparent.png"
-        );
-        token.attrs.push(["data-orig-src", origSrc]);
-      }
-    });
-  }
-}
-
-export function setup(helper) {
-  const opts = helper.getOptions();
-  if (opts.previewing) helper.whiteList(["img.resizable"]);
-  helper.whiteList(["img[data-orig-src]"]);
-  helper.registerPlugin(md => {
-    md.core.ruler.push("image-protocol", rule);
-  });
-}

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

@@ -0,0 +1,81 @@
+// 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]);
+        break;
+      }
+    }
+  }
+}
+
+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") {
+      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 lookup = state.md.options.discourse.lookupUploadUrls;
+    let longUrls = (lookup && lookup(srcList)) || {};
+
+    uploads.forEach(([token, srcIndex]) => {
+      let origSrc = token.attrs[srcIndex][1];
+      let mapped = longUrls[origSrc];
+
+      switch (token.tag) {
+        case "img":
+          if (mapped) {
+            token.attrs[srcIndex][1] = mapped.url;
+          } else {
+            token.attrs[srcIndex][1] = state.md.options.discourse.getURL(
+              "/images/transparent.png"
+            );
+
+            token.attrs.push(["data-orig-src", origSrc]);
+          }
+          break;
+        case "a":
+          if (mapped) {
+            token.attrs[srcIndex][1] = mapped.short_path;
+          } else {
+            token.attrs[srcIndex][1] = state.md.options.discourse.getURL(
+              "/404"
+            );
+
+            token.attrs.push(["data-orig-href", origSrc]);
+          }
+
+          break;
+      }
+    });
+  }
+}
+
+export function setup(helper) {
+  const opts = helper.getOptions();
+  if (opts.previewing) helper.whiteList(["img.resizable"]);
+  helper.whiteList(["img[data-orig-src]", "a[data-orig-href]"]);
+
+  helper.registerPlugin(md => {
+    md.core.ruler.push("upload-protocol", rule);
+  });
+}

app/assets/javascripts/pretty-text/image-short-url.js.es6

@@ -1,68 +0,0 @@
-let _cache = {};
-
-export function lookupCachedUploadUrl(shortUrl) {
-  return _cache[shortUrl];
-}
-
-export function lookupUncachedUploadUrls(urls, ajax) {
-  return ajax("/uploads/lookup-urls", {
-    method: "POST",
-    data: { short_urls: urls }
-  }).then(uploads => {
-    uploads.forEach(upload =>
-      cacheShortUploadUrl(upload.short_url, upload.url)
-    );
-    urls.forEach(url =>
-      cacheShortUploadUrl(url, lookupCachedUploadUrl(url) || "missing")
-    );
-    return uploads;
-  });
-}
-
-export function cacheShortUploadUrl(shortUrl, url) {
-  _cache[shortUrl] = url;
-}
-
-export function resetCache() {
-  _cache = {};
-}
-
-function _loadCachedShortUrls($images) {
-  $images.each((idx, image) => {
-    const $image = $(image);
-    const url = lookupCachedUploadUrl($image.data("orig-src"));
-
-    if (url) {
-      $image.removeAttr("data-orig-src");
-      if (url !== "missing") {
-        $image.attr("src", url);
-      }
-    }
-  });
-}
-
-function _loadShortUrls($images, ajax) {
-  const urls = $images.toArray().map(img => $(img).data("orig-src"));
-  return lookupUncachedUploadUrls(urls, ajax).then(() =>
-    _loadCachedShortUrls($images)
-  );
-}
-
-export function resolveAllShortUrls(ajax) {
-  let $shortUploadUrls = $("img[data-orig-src]");
-
-  if ($shortUploadUrls.length > 0) {
-    _loadCachedShortUrls($shortUploadUrls);
-
-    $shortUploadUrls = $("img[data-orig-src]");
-    if ($shortUploadUrls.length > 0) {
-      // this is carefully batched so we can do a leading debounce (trigger right away)
-      return Ember.run.debounce(
-        null,
-        () => _loadShortUrls($shortUploadUrls, ajax),
-        450,
-        true
-      );
-    }
-  }
-}

app/assets/javascripts/pretty-text/pretty-text.js.es6

@@ -26,7 +26,7 @@ export function buildOptions(state) {
     lookupPrimaryUserGroupByPostNumber,
     formatUsername,
     emojiUnicodeReplacer,
-    lookupImageUrls,
+    lookupUploadUrls,
     previewing,
     linkify,
     censoredWords
@@ -65,7 +65,7 @@ export function buildOptions(state) {
     lookupPrimaryUserGroupByPostNumber,
     formatUsername,
     emojiUnicodeReplacer,
-    lookupImageUrls,
+    lookupUploadUrls,
     censoredWords,
     allowedHrefSchemes: siteSettings.allowed_href_schemes
       ? siteSettings.allowed_href_schemes.split("|")

app/assets/javascripts/pretty-text/upload-short-url.js.es6

@@ -0,0 +1,111 @@
+let _cache = {};
+
+export function lookupCachedUploadUrl(shortUrl) {
+  return _cache[shortUrl] || {};
+}
+
+const MISSING = "missing";
+
+export function lookupUncachedUploadUrls(urls, ajax) {
+  return ajax("/uploads/lookup-urls", {
+    method: "POST",
+    data: { short_urls: urls }
+  }).then(uploads => {
+    uploads.forEach(upload => {
+      cacheShortUploadUrl(upload.short_url, {
+        url: upload.url,
+        short_path: upload.short_path
+      });
+    });
+
+    urls.forEach(url =>
+      cacheShortUploadUrl(url, {
+        url: lookupCachedUploadUrl(url).url || MISSING,
+        short_path: lookupCachedUploadUrl(url).short_path || MISSING
+      })
+    );
+
+    return uploads;
+  });
+}
+
+export function cacheShortUploadUrl(shortUrl, value) {
+  _cache[shortUrl] = value;
+}
+
+export function resetCache() {
+  _cache = {};
+}
+
+export const ATTACHMENT_CSS_CLASS = "attachment";
+
+function _loadCachedShortUrls($uploads) {
+  $uploads.each((idx, upload) => {
+    const $upload = $(upload);
+    let url;
+
+    switch (upload.tagName) {
+      case "A":
+        url = lookupCachedUploadUrl($upload.data("orig-href")).short_path;
+
+        if (url) {
+          $upload.removeAttr("data-orig-href");
+
+          if (url !== MISSING) {
+            $upload.attr("href", url);
+            const content = $upload.text().split("|");
+
+            if (content[1] === ATTACHMENT_CSS_CLASS) {
+              $upload.addClass(ATTACHMENT_CSS_CLASS);
+              $upload.text(content[0]);
+            }
+          }
+        }
+
+        break;
+      case "IMG":
+        url = lookupCachedUploadUrl($upload.data("orig-src")).url;
+
+        if (url) {
+          $upload.removeAttr("data-orig-src");
+
+          if (url !== MISSING) {
+            $upload.attr("src", url);
+          }
+        }
+
+        break;
+    }
+  });
+}
+
+function _loadShortUrls($uploads, ajax) {
+  const urls = $uploads.toArray().map(upload => {
+    const $upload = $(upload);
+    return $upload.data("orig-src") || $upload.data("orig-href");
+  });
+
+  return lookupUncachedUploadUrls(urls, ajax).then(() =>
+    _loadCachedShortUrls($uploads)
+  );
+}
+
+export function resolveAllShortUrls(ajax) {
+  const attributes = "img[data-orig-src], a[data-orig-href]";
+  let $shortUploadUrls = $(attributes);
+
+  if ($shortUploadUrls.length > 0) {
+    _loadCachedShortUrls($shortUploadUrls);
+
+    $shortUploadUrls = $(attributes);
+    if ($shortUploadUrls.length > 0) {
+      // this is carefully batched so we can do a leading debounce (trigger right away)
+      return Ember.run.debounce(
+        null,
+        () => _loadShortUrls($shortUploadUrls, ajax),
+        450,
+        true
+      );
+    }
+  }
+}

app/controllers/uploads_controller.rb

@@ -5,9 +5,9 @@ require_dependency 'upload_creator'
 require_dependency "file_store/local_store"
 
 class UploadsController < ApplicationController
-  requires_login except: [:show]
+  requires_login except: [:show, :show_short]
 
-  skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show]
+  skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show, :show_short]
 
   def create
     # capture current user for block later on
@@ -56,8 +56,12 @@ class UploadsController < ApplicationController
     uploads = []
 
     if (params[:short_urls] && params[:short_urls].length > 0)
-      PrettyText::Helpers.lookup_image_urls(params[:short_urls]).each do |short_url, url|
-        uploads << { short_url: short_url, url: url }
+      PrettyText::Helpers.lookup_upload_urls(params[:short_urls]).each do |short_url, paths|
+        uploads << {
+          short_url: short_url,
+          url: paths[:url],
+          short_path: paths[:short_path]
+        }
       end
     end
 
@@ -76,23 +80,31 @@ class UploadsController < ApplicationController
           return render_404 unless local_store.has_been_uploaded?(upload.url)
         end
 
-        opts = {
-          filename: upload.original_filename,
-          content_type: MiniMime.lookup_by_filename(upload.original_filename)&.content_type,
-        }
-        opts[:disposition]   = "inline" if params[:inline]
-        opts[:disposition] ||= "attachment" unless FileHelper.is_supported_image?(upload.original_filename)
-
-        file_path = Discourse.store.path_for(upload)
-        return render_404 unless file_path
-
-        send_file(file_path, opts)
+        send_file_local_upload(upload)
       else
         render_404
       end
     end
   end
 
+  def show_short
+    if SiteSetting.prevent_anons_from_downloading_files && current_user.nil?
+      return render_404
+    end
+
+    sha1 = Upload.sha1_from_base62_encoded(params[:base62])
+
+    if upload = Upload.find_by(sha1: sha1)
+      if Discourse.store.internal?
+        send_file_local_upload(upload)
+      else
+        redirect_to Discourse.store.path_for(upload)
+      end
+    else
+      render_404
+    end
+  end
+
   def metadata
     params.require(:url)
     upload = Upload.get_from_url(params[:url])
@@ -165,4 +177,24 @@ class UploadsController < ApplicationController
     tempfile&.close!
   end
 
+  private
+
+  def send_file_local_upload(upload)
+    opts = {
+      filename: upload.original_filename,
+      content_type: MiniMime.lookup_by_filename(upload.original_filename)&.content_type
+    }
+
+    if params[:inline]
+      opts[:disposition] = "inline"
+    elsif !FileHelper.is_supported_image?(upload.original_filename)
+      opts[:disposition] = "attachment"
+    end
+
+    file_path = Discourse.store.path_for(upload)
+    return render_404 unless file_path
+
+    send_file(file_path, opts)
+  end
+
 end

app/models/upload.rb

@@ -117,7 +117,28 @@ class Upload < ActiveRecord::Base
   end
 
   def short_url
-    "upload://#{Base62.encode(sha1.hex)}.#{extension}"
+    "upload://#{short_url_basename}"
+  end
+
+  def short_path
+    self.class.short_path(sha1: self.sha1, extension: self.extension)
+  end
+
+  def self.short_path(sha1:, extension:)
+    @url_helpers ||= Rails.application.routes.url_helpers
+
+    @url_helpers.upload_short_path(
+      base62: self.base62_sha1(sha1),
+      extension: extension
+    )
+  end
+
+  def self.base62_sha1(sha1)
+    Base62.encode(sha1.hex)
+  end
+
+  def base62_sha1
+    Upload.base62_sha1(upload.sha1)
   end
 
   def local?
@@ -182,13 +203,17 @@ class Upload < ActiveRecord::Base
 
   def self.sha1_from_short_url(url)
     if url =~ /(upload:\/\/)?([a-zA-Z0-9]+)(\..*)?/
-      sha1 = Base62.decode($2).to_s(16)
+      self.sha1_from_base62_encoded($2)
+    end
+  end
 
-      if sha1.length > SHA1_LENGTH
-        nil
-      else
-        sha1.rjust(SHA1_LENGTH, '0')
-      end
+  def self.sha1_from_base62_encoded(encoded_sha1)
+    sha1 = Base62.decode(encoded_sha1).to_s(16)
+
+    if sha1.length > SHA1_LENGTH
+      nil
+    else
+      sha1.rjust(SHA1_LENGTH, '0')
     end
   end
 
@@ -322,6 +347,12 @@ class Upload < ActiveRecord::Base
     problems
   end
 
+  private
+
+  def short_url_basename
+    "#{Upload.base62_sha1(sha1)}.#{extension}"
+  end
+
 end
 
 # == Schema Information

config/routes.rb

@@ -486,6 +486,7 @@ Discourse::Application.routes.draw do
 
   # used to download original images
   get "uploads/:site/:sha(.:extension)" => "uploads#show", constraints: { site: /\w+/, sha: /\h{40}/, extension: /[a-z0-9\.]+/i }
+  get "uploads/short-url/:base62(.:extension)" => "uploads#show_short", constraints: { site: /\w+/, base62: /[a-zA-Z0-9]+/, extension: /[a-z0-9\.]+/i }, as: :upload_short
   # used to download attachments
   get "uploads/:site/original/:tree:sha(.:extension)" => "uploads#show", constraints: { site: /\w+/, tree: /([a-z0-9]+\/)+/i, sha: /\h{40}/, extension: /[a-z0-9\.]+/i }
   # used to download attachments (old route)

lib/file_store/s3_store.rb

@@ -101,8 +101,13 @@ module FileStore
     end
 
     def path_for(upload)
-      url = upload.try(:url)
-      FileStore::LocalStore.new.path_for(upload) if url && url[/^\/[^\/]/]
+      url = upload&.url
+
+      if url && url[/^\/[^\/]/]
+        FileStore::LocalStore.new.path_for(upload)
+      else
+        url
+      end
     end
 
     def cdn_url(url)

lib/pretty_text.rb

@@ -159,7 +159,7 @@ module PrettyText
         __optInput.categoryHashtagLookup = __categoryLookup;
         __optInput.customEmoji = #{custom_emoji.to_json};
         __optInput.emojiUnicodeReplacer = __emojiUnicodeReplacer;
-        __optInput.lookupImageUrls = __lookupImageUrls;
+        __optInput.lookupUploadUrls = __lookupUploadUrls;
         __optInput.censoredWords = #{WordWatcher.words_for_action(:censor).join('|').to_json};
       JS
 

lib/pretty_text/helpers.rb

@@ -49,7 +49,7 @@ module PrettyText
       end
     end
 
-    def lookup_image_urls(urls)
+    def lookup_upload_urls(urls)
       map = {}
       result = {}
 
@@ -66,11 +66,16 @@ module PrettyText
           reverse_map[value] << key
         end
 
-        Upload.where(sha1: map.values).pluck(:sha1, :url).each do |row|
-          sha1, url = row
+        Upload.where(sha1: map.values).pluck(:sha1, :url, :extension).each do |row|
+          sha1, url, extension = row
 
           if short_urls = reverse_map[sha1]
-            short_urls.each { |short_url| result[short_url] = url }
+            short_urls.each do |short_url|
+              result[short_url] = {
+                url: url,
+                short_path: Upload.short_path(sha1: sha1, extension: extension)
+              }
+            end
           end
         end
       end

lib/pretty_text/shims.js

@@ -65,8 +65,8 @@ function __getURL(url) {
   return url;
 }
 
-function __lookupImageUrls(urls) {
-  return __helpers.lookup_image_urls(urls);
+function __lookupUploadUrls(urls) {
+  return __helpers.lookup_upload_urls(urls);
 }
 
 function __getTopicInfo(i) {

spec/components/file_store/s3_store_spec.rb

@@ -303,21 +303,12 @@ describe FileStore::S3Store do
   end
 
   describe ".path_for" do
-
-    def assert_path(path, expected)
-      upload = Upload.new(url: path)
-
-      path = store.path_for(upload)
-      expected = FileStore::LocalStore.new.path_for(upload) if expected
-
-      expect(path).to eq(expected)
-    end
-
     it "correctly falls back to local" do
-      assert_path("/hello", "/hello")
-      assert_path("//hello", nil)
-      assert_path("http://hello", nil)
-      assert_path("https://hello", nil)
+      local_upload = Fabricate(:upload)
+      s3_upload =  Fabricate(:upload_s3)
+
+      expect(Discourse.store.path_for(local_upload)).to eq(local_upload.url)
+      expect(Discourse.store.path_for(s3_upload)).to eq(s3_upload.url)
     end
   end
 

spec/components/pretty_text_spec.rb

@@ -1260,9 +1260,11 @@ HTML
 
   end
 
-  describe "image decoding" do
+  describe "upload decoding" do
 
     it "can decode upload:// for default setup" do
+      set_cdn_url('https://cdn.com')
+
       upload = Fabricate(:upload)
 
       raw = <<~RAW
@@ -1274,6 +1276,12 @@ HTML
           - ![upload](#{upload.short_url})
 
       ![upload](#{upload.short_url.gsub(".png", "")})
+
+      [some attachment](#{upload.short_url})
+
+      [some attachment|attachment](#{upload.short_url})
+
+      [some attachment|random](#{upload.short_url})
       RAW
 
       cooked = <<~HTML
@@ -1290,6 +1298,9 @@ HTML
         </li>
         </ul>
         <p><img src="#{upload.url}" alt="upload"></p>
+        <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>
       HTML
 
       expect(PrettyText.cook(raw)).to eq(cooked.strip)
@@ -1297,10 +1308,15 @@ HTML
 
     it "can place a blank image if we can not find the upload" do
 
-      raw = "![upload](upload://abcABC.png)"
+      raw = <<~MD
+      ![upload](upload://abcABC.png)
+
+      [some attachment|attachment](upload://abcdefg.png)
+      MD
 
       cooked = <<~HTML
-        <p><img src="/images/transparent.png" alt="upload" data-orig-src="upload://abcABC.png"></p>
+      <p><img src="/images/transparent.png" alt="upload" data-orig-src="upload://abcABC.png"></p>
+      <p><a href="/404" data-orig-href="upload://abcdefg.png">some attachment|attachment</a></p>
       HTML
 
       expect(PrettyText.cook(raw)).to eq(cooked.strip)

spec/requests/uploads_controller_spec.rb

@@ -3,6 +3,8 @@
 require 'rails_helper'
 
 describe UploadsController do
+  fab!(:user) { Fabricate(:user) }
+
   describe '#create' do
     it 'requires you to be logged in' do
       post "/uploads.json"
@@ -10,7 +12,9 @@ describe UploadsController do
     end
 
     context 'logged in' do
-      let!(:user) { sign_in(Fabricate(:user)) }
+      before do
+        sign_in(user)
+      end
 
       let(:logo) do
         Rack::Test::UploadedFile.new(file_from_fixtures("logo.png"))
@@ -195,27 +199,26 @@ describe UploadsController do
     end
   end
 
+  def upload_file(file, folder = "images")
+    fake_logo = Rack::Test::UploadedFile.new(file_from_fixtures(file, folder))
+    SiteSetting.authorized_extensions = "*"
+    sign_in(user)
+
+    post "/uploads.json", params: {
+      file: fake_logo,
+      type: "composer",
+    }
+
+    expect(response.status).to eq(200)
+
+    url = JSON.parse(response.body)["url"]
+    upload = Upload.get_from_url(url)
+    upload
+  end
+
   describe '#show' do
     let(:site) { "default" }
     let(:sha) { Digest::SHA1.hexdigest("discourse") }
-    fab!(:user) { Fabricate(:user) }
-
-    def upload_file(file, folder = "images")
-      fake_logo = Rack::Test::UploadedFile.new(file_from_fixtures(file, folder))
-      SiteSetting.authorized_extensions = "*"
-      sign_in(user)
-
-      post "/uploads.json", params: {
-        file: fake_logo,
-        type: "composer",
-      }
-
-      expect(response.status).to eq(200)
-
-      url = JSON.parse(response.body)["url"]
-      upload = Upload.where(url: url).first
-      upload
-    end
 
     context "when using external storage" do
       fab!(:upload) { upload_file("small.pdf", "pdf") }
@@ -284,7 +287,7 @@ describe UploadsController do
       it "returns 404 when an anonymous user tries to download a file" do
         skip("this only works when nginx/apache is asset server") if Discourse::Application.config.public_file_server.enabled
         upload = upload_file("small.pdf", "pdf")
-        delete "/session/#{user.username}.json" # upload a file, then sign out
+        delete "/session/#{user.username}.json"
 
         SiteSetting.prevent_anons_from_downloading_files = true
         get upload.url
@@ -293,9 +296,66 @@ describe UploadsController do
     end
   end
 
+  describe "#show_short" do
+    describe "local store" do
+      fab!(:image_upload) { upload_file("smallest.png") }
+
+      it "returns the right response" do
+        get image_upload.short_path
+
+        expect(response.status).to eq(200)
+
+        expect(response.headers["Content-Disposition"])
+          .to include("attachment; filename=\"#{image_upload.original_filename}\"")
+      end
+
+      it "returns the right response when `inline` param is given" do
+        get "#{image_upload.short_path}?inline=1"
+
+        expect(response.status).to eq(200)
+
+        expect(response.headers["Content-Disposition"])
+          .to include("inline; filename=\"#{image_upload.original_filename}\"")
+      end
+
+      it "returns the right response when base62 param is invalid " do
+        get "/uploads/short-url/12345.png"
+
+        expect(response.status).to eq(404)
+      end
+
+      it "returns the right response when anon tries to download a file " \
+         "when prevent_anons_from_downloading_files is true" do
+
+        delete "/session/#{user.username}.json"
+        SiteSetting.prevent_anons_from_downloading_files = true
+
+        get image_upload.short_path
+
+        expect(response.status).to eq(404)
+      end
+    end
+
+    describe "s3 store" do
+      let(:upload) { Fabricate(:upload_s3) }
+
+      before do
+        SiteSetting.enable_s3_uploads = true
+        SiteSetting.s3_access_key_id = "fakeid7974664"
+        SiteSetting.s3_secret_access_key = "fakesecretid7974664"
+      end
+
+      it "should redirect to the s3 URL" do
+        get upload.short_path
+
+        expect(response).to redirect_to(upload.url)
+      end
+    end
+  end
+
   describe '#lookup_urls' do
     it 'can look up long urls' do
-      sign_in(Fabricate(:user))
+      sign_in(user)
       upload = Fabricate(:upload)
 
       post "/uploads/lookup-urls.json", params: { short_urls: [upload.short_url] }
@@ -303,6 +363,7 @@ describe UploadsController do
 
       result = JSON.parse(response.body)
       expect(result[0]["url"]).to eq(upload.url)
+      expect(result[0]["short_path"]).to eq(upload.short_path)
     end
   end
 
@@ -327,7 +388,7 @@ describe UploadsController do
 
     describe 'when signed in' do
       before do
-        sign_in(Fabricate(:user))
+        sign_in(user)
       end
 
       describe 'when url is invalid' do

test/javascripts/acceptance/composer-attachment-test.js.es6

@@ -0,0 +1,39 @@
+import { acceptance } from "helpers/qunit-helpers";
+
+acceptance("Composer Attachment", {
+  loggedIn: true,
+  pretend(server, helper) {
+    server.post("/uploads/lookup-urls", () => {
+      return helper.response([
+        {
+          short_url: "upload://asdsad.png",
+          url: "/uploads/default/3X/1/asjdiasjdiasida.png",
+          short_path: "/uploads/short-url/asdsad.png"
+        }
+      ]);
+    });
+  }
+});
+
+QUnit.test("attachments are cooked properly", async assert => {
+  await visit("/t/internationalization-localization/280");
+  await click("#topic-footer-buttons .btn.create");
+
+  await fillIn(".d-editor-input", "[test](upload://abcdefg.png)");
+
+  assert.equal(
+    find(".d-editor-preview:visible")
+      .html()
+      .trim(),
+    '<p><a href="/404">test</a></p>'
+  );
+
+  await fillIn(".d-editor-input", "[test|attachment](upload://asdsad.png)");
+
+  assert.equal(
+    find(".d-editor-preview:visible")
+      .html()
+      .trim(),
+    '<p><a href="/uploads/short-url/asdsad.png" class="attachment">test</a></p>'
+  );
+});

test/javascripts/lib/image-short-url-test.js.es6

@@ -1,56 +0,0 @@
-import {
-  lookupCachedUploadUrl,
-  resolveAllShortUrls,
-  resetCache
-} from "pretty-text/image-short-url";
-import { ajax } from "discourse/lib/ajax";
-
-QUnit.module("lib:pretty-text/image-short-url", {
-  beforeEach() {
-    const response = object => {
-      return [200, { "Content-Type": "application/json" }, object];
-    };
-
-    const srcs = [
-      {
-        short_url: "upload://a.jpeg",
-        url: "/uploads/default/original/3X/c/b/1.jpeg"
-      },
-      {
-        short_url: "upload://b.jpeg",
-        url: "/uploads/default/original/3X/c/b/2.jpeg"
-      }
-    ];
-
-    // prettier-ignore
-    server.post("/uploads/lookup-urls", () => { //eslint-disable-line
-      return response(srcs);
-    });
-
-    fixture().html(
-      srcs.map(src => `<img data-orig-src="${src.url}">`).join("")
-    );
-  },
-
-  afterEach() {
-    resetCache();
-  }
-});
-
-QUnit.test("resolveAllShortUrls", async assert => {
-  let lookup;
-
-  lookup = lookupCachedUploadUrl("upload://a.jpeg");
-  assert.notOk(lookup);
-
-  await resolveAllShortUrls(ajax);
-
-  lookup = lookupCachedUploadUrl("upload://a.jpeg");
-  assert.equal(lookup, "/uploads/default/original/3X/c/b/1.jpeg");
-
-  lookup = lookupCachedUploadUrl("upload://b.jpeg");
-  assert.equal(lookup, "/uploads/default/original/3X/c/b/2.jpeg");
-
-  lookup = lookupCachedUploadUrl("upload://c.jpeg");
-  assert.notOk(lookup);
-});

test/javascripts/lib/upload-short-url-test.js.es6

@@ -0,0 +1,81 @@
+import {
+  lookupCachedUploadUrl,
+  resolveAllShortUrls,
+  resetCache
+} from "pretty-text/upload-short-url";
+import { ajax } from "discourse/lib/ajax";
+
+QUnit.module("lib:pretty-text/upload-short-url", {
+  beforeEach() {
+    const response = object => {
+      return [200, { "Content-Type": "application/json" }, object];
+    };
+
+    const imageSrcs = [
+      {
+        short_url: "upload://a.jpeg",
+        url: "/uploads/default/original/3X/c/b/1.jpeg",
+        short_path: "/uploads/short-url/a.jpeg"
+      },
+      {
+        short_url: "upload://b.jpeg",
+        url: "/uploads/default/original/3X/c/b/2.jpeg",
+        short_path: "/uploads/short-url/b.jpeg"
+      }
+    ];
+
+    const attachmentSrcs = [
+      {
+        short_url: "upload://c.pdf",
+        url: "/uploads/default/original/3X/c/b/3.pdf",
+        short_path: "/uploads/short-url/c.pdf"
+      }
+    ];
+
+    // prettier-ignore
+    server.post("/uploads/lookup-urls", () => { //eslint-disable-line
+      return response(imageSrcs.concat(attachmentSrcs));
+    });
+
+    fixture().html(
+      imageSrcs.map(src => `<img data-orig-src="${src.url}">`).join("") +
+        attachmentSrcs.map(src => `<a data-orig-href="${src.url}">`).join("")
+    );
+  },
+
+  afterEach() {
+    resetCache();
+  }
+});
+
+QUnit.test("resolveAllShortUrls", async assert => {
+  let lookup;
+
+  lookup = lookupCachedUploadUrl("upload://a.jpeg");
+  assert.deepEqual(lookup, {});
+
+  await resolveAllShortUrls(ajax);
+
+  lookup = lookupCachedUploadUrl("upload://a.jpeg");
+
+  assert.deepEqual(lookup, {
+    url: "/uploads/default/original/3X/c/b/1.jpeg",
+    short_path: "/uploads/short-url/a.jpeg"
+  });
+
+  lookup = lookupCachedUploadUrl("upload://b.jpeg");
+
+  assert.deepEqual(lookup, {
+    url: "/uploads/default/original/3X/c/b/2.jpeg",
+    short_path: "/uploads/short-url/b.jpeg"
+  });
+
+  lookup = lookupCachedUploadUrl("upload://c.jpeg");
+  assert.deepEqual(lookup, {});
+
+  lookup = lookupCachedUploadUrl("upload://c.pdf");
+  assert.deepEqual(lookup, {
+    url: "/uploads/default/original/3X/c/b/3.pdf",
+    short_path: "/uploads/short-url/c.pdf"
+  });
+});

test/javascripts/lib/utilities-test.js.es6

@@ -165,14 +165,19 @@ QUnit.test("allows valid uploads to go through", assert => {
   assert.not(bootbox.alert.calledOnce);
 });
 
-var testUploadMarkdown = function(filename) {
-  return getUploadMarkdown({
-    original_filename: filename,
-    filesize: 42,
-    thumbnail_width: 100,
-    thumbnail_height: 200,
-    url: "/uploads/123/abcdef.ext"
-  });
+var testUploadMarkdown = function(filename, opts = {}) {
+  return getUploadMarkdown(
+    Object.assign(
+      {
+        original_filename: filename,
+        filesize: 42,
+        thumbnail_width: 100,
+        thumbnail_height: 200,
+        url: "/uploads/123/abcdef.ext"
+      },
+      opts
+    )
+  );
 };
 
 QUnit.test("getUploadMarkdown", assert => {
@@ -184,9 +189,12 @@ QUnit.test("getUploadMarkdown", assert => {
     testUploadMarkdown("[foo|bar].png"),
     "![%5Bfoo%7Cbar%5D|100x200](/uploads/123/abcdef.ext)"
   );
-  assert.ok(
-    testUploadMarkdown("important.txt") ===
-      '<a class="attachment" href="/uploads/123/abcdef.ext">important.txt</a> (42 Bytes)\n'
+
+  const short_url = "uploads://asdaasd.ext";
+
+  assert.equal(
+    testUploadMarkdown("important.txt", { short_url }),
+    `[important.txt (42 Bytes)|attachment](${short_url})`
   );
 });