diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6 index 3a39a14fd24..b59728b5054 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6 @@ -36,7 +36,7 @@ import { import { cacheShortUploadUrl, resolveAllShortUrls -} from "pretty-text/upload-short-url"; +} from "pretty-text/image-short-url"; import { INLINE_ONEBOX_LOADING_CSS_CLASS, diff --git a/app/assets/javascripts/discourse/components/cook-text.js.es6 b/app/assets/javascripts/discourse/components/cook-text.js.es6 index 3e345df0540..9fefced7492 100644 --- a/app/assets/javascripts/discourse/components/cook-text.js.es6 +++ b/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/upload-short-url") + .requireModule("pretty-text/image-short-url") .resolveAllShortUrls(ajax) ); }); diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6 index 7c1e9a38c91..c022bfd5c98 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js.es6 +++ b/app/assets/javascripts/discourse/lib/utilities.js.es6 @@ -444,9 +444,15 @@ export function getUploadMarkdown(upload) { ) { return uploadLocation(upload.url); } else { - return `[${upload.original_filename} (${I18n.toHumanSize( - upload.filesize - )})|attachment](${upload.short_url})`; + return ( + '' + + upload.original_filename + + " (" + + I18n.toHumanSize(upload.filesize) + + ")\n" + ); } } diff --git a/app/assets/javascripts/markdown-it-bundle.js b/app/assets/javascripts/markdown-it-bundle.js index 2d0ec14ee52..6f88c5abbe2 100644 --- a/app/assets/javascripts/markdown-it-bundle.js +++ b/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/upload-protocol +//= require ./pretty-text/engines/discourse-markdown/image-protocol //= require ./pretty-text/engines/discourse-markdown/inject-line-number //= require ./pretty-text/engines/discourse-markdown/d-wrap diff --git a/app/assets/javascripts/pretty-text-bundle.js b/app/assets/javascripts/pretty-text-bundle.js index 23dc0122c3a..ed5331b6435 100644 --- a/app/assets/javascripts/pretty-text-bundle.js +++ b/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/upload-short-url +//= require ./pretty-text/image-short-url diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6 index 3c91bf03452..4e503d09035 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6 +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6 @@ -1,7 +1,6 @@ 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() { @@ -188,26 +187,6 @@ 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) { @@ -297,7 +276,6 @@ 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); diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/image-protocol.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown/image-protocol.js.es6 new file mode 100644 index 00000000000..bdcfba414e9 --- /dev/null +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/image-protocol.js.es6 @@ -0,0 +1,62 @@ +// 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); + }); +} diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/upload-protocol.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown/upload-protocol.js.es6 deleted file mode 100644 index 27cfb918600..00000000000 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/upload-protocol.js.es6 +++ /dev/null @@ -1,81 +0,0 @@ -// 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); - }); -} diff --git a/app/assets/javascripts/pretty-text/image-short-url.js.es6 b/app/assets/javascripts/pretty-text/image-short-url.js.es6 new file mode 100644 index 00000000000..e457210dd76 --- /dev/null +++ b/app/assets/javascripts/pretty-text/image-short-url.js.es6 @@ -0,0 +1,68 @@ +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 + ); + } + } +} diff --git a/app/assets/javascripts/pretty-text/pretty-text.js.es6 b/app/assets/javascripts/pretty-text/pretty-text.js.es6 index 33836cddd3f..da572d891ad 100644 --- a/app/assets/javascripts/pretty-text/pretty-text.js.es6 +++ b/app/assets/javascripts/pretty-text/pretty-text.js.es6 @@ -26,7 +26,7 @@ export function buildOptions(state) { lookupPrimaryUserGroupByPostNumber, formatUsername, emojiUnicodeReplacer, - lookupUploadUrls, + lookupImageUrls, previewing, linkify, censoredWords @@ -65,7 +65,7 @@ export function buildOptions(state) { lookupPrimaryUserGroupByPostNumber, formatUsername, emojiUnicodeReplacer, - lookupUploadUrls, + lookupImageUrls, censoredWords, allowedHrefSchemes: siteSettings.allowed_href_schemes ? siteSettings.allowed_href_schemes.split("|") diff --git a/app/assets/javascripts/pretty-text/upload-short-url.js.es6 b/app/assets/javascripts/pretty-text/upload-short-url.js.es6 deleted file mode 100644 index 8b0b4db2b4d..00000000000 --- a/app/assets/javascripts/pretty-text/upload-short-url.js.es6 +++ /dev/null @@ -1,111 +0,0 @@ -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 - ); - } - } -} diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 64852af0160..fb1ba6333d6 100644 --- a/app/controllers/uploads_controller.rb +++ b/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, :show_short] + requires_login except: [:show] - skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show, :show_short] + skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show] def create # capture current user for block later on @@ -56,12 +56,8 @@ class UploadsController < ApplicationController uploads = [] if (params[:short_urls] && params[:short_urls].length > 0) - 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] - } + PrettyText::Helpers.lookup_image_urls(params[:short_urls]).each do |short_url, url| + uploads << { short_url: short_url, url: url } end end @@ -80,31 +76,23 @@ class UploadsController < ApplicationController return render_404 unless local_store.has_been_uploaded?(upload.url) end - send_file_local_upload(upload) + 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) 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]) @@ -177,24 +165,4 @@ 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 diff --git a/app/models/upload.rb b/app/models/upload.rb index eaa322976c0..6a32afcc58a 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -117,28 +117,7 @@ class Upload < ActiveRecord::Base end def short_url - "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) + "upload://#{Base62.encode(sha1.hex)}.#{extension}" end def local? @@ -203,17 +182,13 @@ class Upload < ActiveRecord::Base def self.sha1_from_short_url(url) if url =~ /(upload:\/\/)?([a-zA-Z0-9]+)(\..*)?/ - self.sha1_from_base62_encoded($2) - end - end + sha1 = Base62.decode($2).to_s(16) - 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') + if sha1.length > SHA1_LENGTH + nil + else + sha1.rjust(SHA1_LENGTH, '0') + end end end @@ -347,12 +322,6 @@ class Upload < ActiveRecord::Base problems end - private - - def short_url_basename - "#{Upload.base62_sha1(sha1)}.#{extension}" - end - end # == Schema Information diff --git a/config/routes.rb b/config/routes.rb index c996544c2e0..0b42f7fba14 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -486,7 +486,6 @@ 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) diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index df3296f7abd..9ca94b98f79 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -101,13 +101,8 @@ module FileStore end def path_for(upload) - url = upload&.url - - if url && url[/^\/[^\/]/] - FileStore::LocalStore.new.path_for(upload) - else - url - end + url = upload.try(:url) + FileStore::LocalStore.new.path_for(upload) if url && url[/^\/[^\/]/] end def cdn_url(url) diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 79b6e66a4d7..32063ad5c4e 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -159,7 +159,7 @@ module PrettyText __optInput.categoryHashtagLookup = __categoryLookup; __optInput.customEmoji = #{custom_emoji.to_json}; __optInput.emojiUnicodeReplacer = __emojiUnicodeReplacer; - __optInput.lookupUploadUrls = __lookupUploadUrls; + __optInput.lookupImageUrls = __lookupImageUrls; __optInput.censoredWords = #{WordWatcher.words_for_action(:censor).join('|').to_json}; JS diff --git a/lib/pretty_text/helpers.rb b/lib/pretty_text/helpers.rb index d42ba3749bc..3337282268b 100644 --- a/lib/pretty_text/helpers.rb +++ b/lib/pretty_text/helpers.rb @@ -49,7 +49,7 @@ module PrettyText end end - def lookup_upload_urls(urls) + def lookup_image_urls(urls) map = {} result = {} @@ -66,16 +66,11 @@ module PrettyText reverse_map[value] << key end - Upload.where(sha1: map.values).pluck(:sha1, :url, :extension).each do |row| - sha1, url, extension = row + Upload.where(sha1: map.values).pluck(:sha1, :url).each do |row| + sha1, url = row if short_urls = reverse_map[sha1] - short_urls.each do |short_url| - result[short_url] = { - url: url, - short_path: Upload.short_path(sha1: sha1, extension: extension) - } - end + short_urls.each { |short_url| result[short_url] = url } end end end diff --git a/lib/pretty_text/shims.js b/lib/pretty_text/shims.js index 44f80690720..929dafd13ab 100644 --- a/lib/pretty_text/shims.js +++ b/lib/pretty_text/shims.js @@ -65,8 +65,8 @@ function __getURL(url) { return url; } -function __lookupUploadUrls(urls) { - return __helpers.lookup_upload_urls(urls); +function __lookupImageUrls(urls) { + return __helpers.lookup_image_urls(urls); } function __getTopicInfo(i) { diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb index 4474613c50d..1e497a66976 100644 --- a/spec/components/file_store/s3_store_spec.rb +++ b/spec/components/file_store/s3_store_spec.rb @@ -303,12 +303,21 @@ describe FileStore::S3Store do end describe ".path_for" do - it "correctly falls back to local" do - 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) + 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) end end diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index 71da86ead13..9ace739e48a 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -1260,11 +1260,9 @@ HTML end - describe "upload decoding" do + describe "image decoding" do it "can decode upload:// for default setup" do - set_cdn_url('https://cdn.com') - upload = Fabricate(:upload) raw = <<~RAW @@ -1276,12 +1274,6 @@ 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 @@ -1298,9 +1290,6 @@ HTML

upload

-

some attachment

-

some attachment

-

some attachment|random

HTML expect(PrettyText.cook(raw)).to eq(cooked.strip) @@ -1308,15 +1297,10 @@ HTML it "can place a blank image if we can not find the upload" do - raw = <<~MD - ![upload](upload://abcABC.png) - - [some attachment|attachment](upload://abcdefg.png) - MD + raw = "![upload](upload://abcABC.png)" cooked = <<~HTML -

upload

-

some attachment|attachment

+

upload

HTML expect(PrettyText.cook(raw)).to eq(cooked.strip) diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index b1d60522187..ad51bf88aa2 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/spec/requests/uploads_controller_spec.rb @@ -3,8 +3,6 @@ require 'rails_helper' describe UploadsController do - fab!(:user) { Fabricate(:user) } - describe '#create' do it 'requires you to be logged in' do post "/uploads.json" @@ -12,9 +10,7 @@ describe UploadsController do end context 'logged in' do - before do - sign_in(user) - end + let!(:user) { sign_in(Fabricate(:user)) } let(:logo) do Rack::Test::UploadedFile.new(file_from_fixtures("logo.png")) @@ -199,26 +195,27 @@ 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") } @@ -287,7 +284,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" + delete "/session/#{user.username}.json" # upload a file, then sign out SiteSetting.prevent_anons_from_downloading_files = true get upload.url @@ -296,66 +293,9 @@ 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(user) + sign_in(Fabricate(:user)) upload = Fabricate(:upload) post "/uploads/lookup-urls.json", params: { short_urls: [upload.short_url] } @@ -363,7 +303,6 @@ 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 @@ -388,7 +327,7 @@ describe UploadsController do describe 'when signed in' do before do - sign_in(user) + sign_in(Fabricate(:user)) end describe 'when url is invalid' do diff --git a/test/javascripts/acceptance/composer-attachment-test.js.es6 b/test/javascripts/acceptance/composer-attachment-test.js.es6 deleted file mode 100644 index fa16a62cf85..00000000000 --- a/test/javascripts/acceptance/composer-attachment-test.js.es6 +++ /dev/null @@ -1,39 +0,0 @@ -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(), - '

test

' - ); - - await fillIn(".d-editor-input", "[test|attachment](upload://asdsad.png)"); - - assert.equal( - find(".d-editor-preview:visible") - .html() - .trim(), - '

test

' - ); -}); diff --git a/test/javascripts/lib/image-short-url-test.js.es6 b/test/javascripts/lib/image-short-url-test.js.es6 new file mode 100644 index 00000000000..d7b2c53e857 --- /dev/null +++ b/test/javascripts/lib/image-short-url-test.js.es6 @@ -0,0 +1,56 @@ +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 => ``).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); +}); diff --git a/test/javascripts/lib/upload-short-url-test.js.es6 b/test/javascripts/lib/upload-short-url-test.js.es6 deleted file mode 100644 index ae0434cd028..00000000000 --- a/test/javascripts/lib/upload-short-url-test.js.es6 +++ /dev/null @@ -1,81 +0,0 @@ -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 => ``).join("") + - attachmentSrcs.map(src => ``).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" - }); -}); diff --git a/test/javascripts/lib/utilities-test.js.es6 b/test/javascripts/lib/utilities-test.js.es6 index f01b0c72e73..f19ea0e804c 100644 --- a/test/javascripts/lib/utilities-test.js.es6 +++ b/test/javascripts/lib/utilities-test.js.es6 @@ -165,19 +165,14 @@ QUnit.test("allows valid uploads to go through", assert => { assert.not(bootbox.alert.calledOnce); }); -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 - ) - ); +var testUploadMarkdown = function(filename) { + return getUploadMarkdown({ + original_filename: filename, + filesize: 42, + thumbnail_width: 100, + thumbnail_height: 200, + url: "/uploads/123/abcdef.ext" + }); }; QUnit.test("getUploadMarkdown", assert => { @@ -189,12 +184,9 @@ QUnit.test("getUploadMarkdown", assert => { testUploadMarkdown("[foo|bar].png"), "![%5Bfoo%7Cbar%5D|100x200](/uploads/123/abcdef.ext)" ); - - const short_url = "uploads://asdaasd.ext"; - - assert.equal( - testUploadMarkdown("important.txt", { short_url }), - `[important.txt (42 Bytes)|attachment](${short_url})` + assert.ok( + testUploadMarkdown("important.txt") === + 'important.txt (42 Bytes)\n' ); });