Skip to content

Keep the media details form's base version and stop refused uploads filtering the library #2629

Closed

Description

@pournasserian

Part of #2427 (F-MED). Follow-up to #2601: findings from an independent review of the merged media library screen. The security handling held up: inline types, nosniff, encoded file names, anti-forgery, authorization and redirects.

Findings

  1. Medium: a concurrent change can be silently overwritten. The asset page renders BaseVersion from the stored asset, not from the form it shows.
    • A opens version 1, and B saves version 2.
    • A saves with an invalid tag. The form is refused locally and comes back with BaseVersion=2 and no conflict message.
    • A fixes the tag and saves version 3, wiping out B's change.
  2. Medium: a refused upload filters the library. The return query names the typed folder folder, the same as the list's folder filter. After a refused upload, the list shows only root assets under "In", or nothing under "In Bad Folder".
  3. Low: ?sub=1 returns 500, because the flag is a bool and the issue's own example uses 1.
  4. Low: where-used rows don't link to the entry's audit history, as the issue's scope asks.
  5. Low: a refused upload with a long, valid alt text or caption redirects to a URL over the server's limit, which answers 414. The person loses the error and what they typed.
  6. Low: an upload over the size limit plus 1 MB has its connection reset rather than getting the "too large" message.
  7. Low: test gaps.
    • No test that a refused type, such as SVG or HTML, is reported.
    • No test that an anonymous upload goes to sign-in.
    • No test that a non-image is served as an attachment.
    • The screen tests run on SQLite only.

Scope

  • The hidden BaseVersion comes from the form being shown, so a refused form keeps the version it was made from, and the next save is refused as a conflict.
  • The upload's return query uses upload-folder, upload-tags, upload-alt and upload-caption, so it no longer collides with the list filters.
    • An alt text or caption too long for a safe URL is not carried back. The form says so, and the person types it again.
  • sub takes 1, true, 0 and false.
  • Where-used rows link to the entry's or entity's audit history, using the subject type its content type implies.
  • The upload endpoint refuses a request whose declared length is over the limit before reading it, with the "too large" message.
    • A chunked upload with no declared length still meets the server's limit and is reset. That is documented, not fixed.
  • Tests:
    • A refused SVG upload, an anonymous upload and a PDF served as an attachment.
    • The conflict after a locally refused save.
    • The list unfiltered after a refused upload.
    • sub=1.
    • A large declared length getting the friendly message.
    • The screen tests run on SQLite and PostgreSQL.

Acceptance

  • The overwrite scenario above ends in a conflict message, and B's change stays.
  • After a refused upload, the library lists every asset as before.
  • /media?folder=products&sub=1 lists products and its subfolders.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:mediaMedia library (MED)type:bugDefectv2FluentCMS v2 (v2 branch)

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions