Skip to content

Let editors upload files to an article - #35

Merged
bonelifer merged 3 commits into
mainfrom
feature/file-uploads
Oct 12, 2026
Merged

bonelifer merged 3 commits into
mainfrom
feature/file-uploads

Conversation

@bonelifer

Copy link
Copy Markdown
Contributor

What this does

Lets editors upload images and files to an article, and place them in the text with [[File:name]]. This is the "File uploads" item from docs/TODO.md.

  • Storage. A new wiki_file table and the folder store/phpbbmodders_wiki/. Files belong to the article, not to one version of it. They are stored under random names with no extension, and the folder is created with a .htaccess that denies direct access.
  • Reading. Files are read only through the wiki's own controller, which checks that the reader may view the wiki and that the article has a live version (or that the reader may view inactive articles). Images that phpBB verifies as GIF, JPEG, PNG or WebP are shown inline; everything else, including SVG and HTML, is only ever sent as a download, with nosniff and a restrictive Content-Security-Policy.
  • Limits. phpBB's own upload class checks each file against the file types the board allows in every forum and the board's maximum file size, and rejects content that looks like HTML or script. A file name is cleaned so it can be written inside [[File:…]], and a name already used in the article is refused.
  • Permission. A new "Can upload files to articles" permission (u_wiki_upload). Nobody has it until an administrator gives it. The member who uploaded a file can delete it, and so can a moderator with the existing "Can delete versions of an article" permission. Deleting asks for confirmation. Deleting an article deletes its files from the table and the disk.
  • In the text. [[File:Diagram.png]] shows an uploaded image, [[File:notes.zip|the notes]] links to any other file with that link text, and a name that was never uploaded stays plain text with a hint. [[File:…]] looks only at the files of the article being shown, so in a forum post it stays plain text. Ordinary [[Article]] links are unchanged.
  • Page. Each article gets a Files section: the list with sizes, uploaders and the embed syntax, and the upload form for members who may upload.
  • German strings, a README paragraph, and removal of the TODO entry.

Testing

Claude ran the same two end-to-end scripts on phpBB 3.3.19 and phpBB 4.0 (master) boards, and a shorter run on throwaway MySQL 8.0 and PostgreSQL 16 boards:

  • the upload form shows only to members with the permission; PNG and ZIP uploads work; PHP, HTML, a text file renamed .png, an empty submit and a duplicate name are refused with worded errors; a file over the board's size limit is refused; an upload without the form token is refused;
  • files sit on disk without an extension, the PNG is served inline as image/png and the ZIP as a download, both with nosniff; a file id under another article is not found; a file of an article with no live version is hidden from a reader;
  • an embedded image loads in the page, a ZIP embeds as a link with its link text, a missing file stays plain text, and normal [[links]] still work;
  • another uploader cannot delete a file, a moderator can, delete asks first, an uploader can delete their own file, and deleting the article removes its files from the table and the disk;
  • on MySQL and PostgreSQL: the table and permission exist after enabling, and upload, serving, embedding, deleting one file and deleting the article all work (17 checks each).

Nine new unit tests cover file name cleaning, which types show inline, and the [[File:]] syntax against the real s9e text formatter configuration (it must become its own tag and must not turn into a link). The new and changed PHP files lint clean on PHP 8.5.

Known limits

  • An article saved before this change that already contains [[File:x]] was stored as a link to an article called "File:x". It shows as a link until the article is saved again or reparsed.
  • Only the board's global maximum size applies, not a per-group size; only file types allowed in every forum are accepted.
  • Files are not versioned: they stay with the article when a version is deleted. There are no thumbnails.

Investigated and written by Claude on behalf of William Jacoby (bonelifer).

bonelifer and others added 3 commits October 12, 2026 12:05
New wiki_file table and u_wiki_upload permission. Files belong to the
article and are stored under random names in store/phpbbmodders_wiki/,
checked against the board's allowed file types and size limit, and read
only through the wiki's own controller, which checks that the reader may
see the article. [[File:name]] and [[File:name|alt]] in the text show an
uploaded image or link to another kind of file. A file can be deleted by
its uploader or by a moderator, and deleting an article deletes its files.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@bonelifer
bonelifer merged commit a9807e4 into main Oct 12, 2026
58 checks passed
@bonelifer
bonelifer deleted the feature/file-uploads branch October 12, 2026 17:15
bonelifer added a commit that referenced this pull request Oct 12, 2026
## What this does

Fixes three actions that silently failed for any article whose name
contains a space (most of them):

- **Delete an article** (moderator controls, version history) reported
"The article has been deleted successfully" and deleted nothing.
- **Set an article inactive** reported "The article has been set to
inactive" and changed nothing.
- **Delete an uploaded file** (added in #35) answered "The file does not
exist." instead of deleting it.

The cause: `confirm_box()` posts back to an address whose path is
encoded a second time, so the route gave `Some%20Article` instead of
`Some Article` and every query on the name matched nothing. The handlers
now take the name from the hidden field the confirm box already carries
(`functions::confirmed_article()`), falling back to the route's name on
the first request, before the box. Names without a space were never
affected, which is why earlier tests, which used underscores, did not
see it.

The delete handler gains the request object as a constructor argument
for this (its service definition is updated).

## Testing

Claude reproduced all three failures on a phpBB 4.0 (master) board
seeded by the new wiki fixtures: the article delete showed success with
the row still in the table, and the file delete answered "The file does
not exist." After the change, on the same board and on throwaway phpBB
3.3.19 boards with MySQL 8.0 and with PostgreSQL 16, a browser run
confirmed that an uploader can delete their own file from "Markup
Guide", a moderator can set "Bulk Article 06" inactive, and a moderator
can delete "Markup Guide", whose file rows and files go with it (5
checks per board, all passing). Four new unit tests cover the helper:
the posted name wins over the route's, the route's name is used before
the box, HTML escaping is undone, and Unicode survives. All 66 wiki unit
tests pass.

---

Investigated and written by [Claude](https://www.anthropic.com/claude)
on behalf of William Jacoby ([bonelifer](https://github.com/bonelifer)).

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant