mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 08:06:54 +08:00
49474b0188#39133 backslash-escaped markdown characters in upload filenames at generation time. That worked for freshly uploaded files but exposed a latent bug in the rich-editor round-trip: markdown-it kept `\_` in the parsed image token's content, ProseMirror stored it verbatim in the `alt` attribute, and on save the serializer re-escaped each `\` to `\\`. Every edit doubled the backslashes (1 → 2 → 4 → … → 2^N) until the post exceeded `max_post_length` and became uneditable. Escaping the raw is fundamentally fragile — nothing treats the stored raw as canonical, so any parse/serialize cycle either drops the escape or re-applies it. Fix it at parse time instead: - Revert the generation-side escaping from #39133 in `UploadMarkdown`, `uploads.js`, `inline_uploads.rb`, `to-markdown.js` and `sanitizeAlt`. - Add a `literalize_upload_labels` core ruler in the markdown-it engine. After inline parsing runs, it walks `image` / `link_open` tokens whose URL starts with `upload://` and collapses their children into a single literal text token, rebuilt from the children `content` plus the `markup` of emphasis/strong/strikethrough delimiters. So `_foo_`, `**foo**`, `~~foo~~`, `` `foo` ``, `\_foo`, linkified URLs, hashtags and mentions inside upload labels all render literally. Reference-style links (`[label][ref]` with `[ref]: upload://…`) get the same treatment for free since they go through the same tokens. The raw now stays canonical: the filename goes in verbatim, cooks the same way on every pass, and the textarea and rich editor round-trip identically. Because escaping is gone, the structural characters `[`, `]` and `|` (which would break the link/image syntax and can't be escaped without reintroducing the doubling) are stripped from labels at every generation point — `UploadMarkdown`, the HTML-anchor and hotlinked-image conversions in `inline_uploads.rb`, `uploads.js` and `to-markdown.js`. The multi-token scan-forward in `renderAttachment` (engine.js) and in ProseMirror's `link.js` parser is kept: it still matters for non-upload attachment links like `[**bold**|attachment](https://example.com/x.pdf)`, where the label legitimately contains inline formatting the new ruler doesn't touch. `StripUploadLabelEscapes` (post-deploy migration) heals posts already damaged by the regression, batching a scoped `regexp_replace` across the `posts` table. The lookahead `(?=[^\]\[]*\]\(upload://)` bounds each match to an upload label — forbidding `[`/`]` between the escape and the closing `](upload://` keeps user-written `\_` escapes elsewhere in the raw intact. Scoping on the lookahead alone, rather than anchoring on the opening `[`, lets a single pass strip every escape in a label (e.g. `foo\_bar\_baz`), not just the first. https://meta.discourse.org/t/401231
71 lines
2.9 KiB
Ruby
Vendored
71 lines
2.9 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
RSpec.describe UploadMarkdown do
|
|
it "generates markdown for each different upload type (attachment, image, video, audio)" do
|
|
SiteSetting.authorized_extensions = "mp4|mp3|pdf|jpg|mmmppp444"
|
|
video = Fabricate(:upload, original_filename: "test_video.mp4", extension: "mp4")
|
|
audio = Fabricate(:upload, original_filename: "test_audio.mp3", extension: "mp3")
|
|
attachment = Fabricate(:upload, original_filename: "test_file.pdf", extension: "pdf")
|
|
image =
|
|
Fabricate(
|
|
:upload,
|
|
width: 100,
|
|
height: 200,
|
|
original_filename: "test_img.jpg",
|
|
extension: "jpg",
|
|
)
|
|
|
|
expect(UploadMarkdown.new(video).to_markdown).to eq(<<~MD.chomp)
|
|
|
|
MD
|
|
expect(UploadMarkdown.new(audio).to_markdown).to eq(<<~MD.chomp)
|
|
|
|
MD
|
|
expect(UploadMarkdown.new(attachment).to_markdown).to eq(<<~MD.chomp)
|
|
[test_file.pdf|attachment](#{attachment.short_url}) (#{attachment.human_filesize})
|
|
MD
|
|
expect(UploadMarkdown.new(image).to_markdown).to eq(<<~MD.chomp)
|
|
|
|
MD
|
|
|
|
unknown = Fabricate(:upload, original_filename: "test_video.mmmppp444", extension: "mmmppp444")
|
|
expect(UploadMarkdown.new(unknown).playable_media_markdown).to eq(<<~MD.chomp)
|
|
[test_video.mmmppp444|attachment](#{unknown.short_url}) (#{unknown.human_filesize})
|
|
MD
|
|
end
|
|
|
|
it "renders filenames with markdown formatting characters literally" do
|
|
SiteSetting.authorized_extensions = "txt"
|
|
|
|
{
|
|
"_test_file_.txt" => "<em>",
|
|
"*test*.txt" => "<em>",
|
|
"**bold**.txt" => "<strong>",
|
|
"~~strike~~.txt" => "<s>",
|
|
"`code`.txt" => "<code>",
|
|
}.each do |filename, bad_tag|
|
|
upload = Fabricate(:upload, original_filename: filename, extension: "txt")
|
|
cooked = PrettyText.cook(UploadMarkdown.new(upload).attachment_markdown)
|
|
|
|
expect(cooked).to include('class="attachment"'),
|
|
"expected attachment class for filename: #{filename}\ncooked: #{cooked}"
|
|
expect(cooked).not_to include(bad_tag),
|
|
"unexpected #{bad_tag} in cooked output for filename: #{filename}\ncooked: #{cooked}"
|
|
expect(cooked).to include(filename),
|
|
"expected filename in cooked output for: #{filename}\ncooked: #{cooked}"
|
|
end
|
|
end
|
|
|
|
it "strips structural markdown characters ([, ], |) from upload labels" do
|
|
SiteSetting.authorized_extensions = "txt|jpg"
|
|
|
|
attachment = Fabricate(:upload, original_filename: "a]b[c|d.txt", extension: "txt")
|
|
image =
|
|
Fabricate(:upload, width: 1, height: 1, original_filename: "x|y[z].jpg", extension: "jpg")
|
|
|
|
expect(UploadMarkdown.new(attachment).attachment_markdown).to eq(
|
|
"[abcd.txt|attachment](#{attachment.short_url}) (#{attachment.human_filesize})",
|
|
)
|
|
expect(UploadMarkdown.new(image).image_markdown).to eq("")
|
|
end
|
|
end
|
