Skip to content

TypeError in Revised event when post content is NULL: $oldContent must be of type string, null given #4606

@ekumanov

Description

@ekumanov

Current Behavior

When a post whose content column is NULL is edited, the Flarum\Post\Event\Revised event constructor throws a TypeError because its third parameter $oldContent is typed as non-nullable string:

flarum.ERROR: TypeError: Flarum\Post\Event\Revised::__construct(): Argument #3 ($oldContent) must be of type string, null given, called in vendor/flarum/core/src/Post/CommentPost.php on line 51 and defined in vendor/flarum/core/src/Post/Event/Revised.php:17
Stack trace:
#0 vendor/flarum/core/src/Post/CommentPost.php(51): Flarum\Post\Event\Revised->__construct()
#1 vendor/flarum/core/src/Api/Resource/PostResource.php(190): Flarum\Post\CommentPost->revise()
#2 vendor/flarum/json-api-server/src/Schema/Concerns/SetsValue.php(151): Flarum\Api\Resource\PostResource->{closure:Flarum\Api\Resource\PostResource::fields():185}()
#3 vendor/flarum/json-api-server/src/Endpoint/Concerns/SavesData.php(205): Tobyz\JsonApiServer\Schema\Field\Field->setValue()
#4 vendor/flarum/core/src/Api/Endpoint/Update.php(60): Flarum\Api\Endpoint\Update->setValues()
...

CommentPost::revise() reads $oldContent = $this->content and forwards it to new Revised($this, $actor, $oldContent), but $this->content can legitimately be NULL — e.g. legacy/imported posts, or posts created by extensions that do not set content. The edit itself does still complete (HTTP-side), but the Revised event never fires and the error pollutes the log on every edit of an affected post.

Steps to Reproduce

  1. Have any post in the database with content IS NULL. We have one such post on a forum migrated from Flarum 1.x — it was already null before the 2.0 upgrade.
    -- to reproduce on a clean install:
    UPDATE posts SET content = NULL WHERE id = <some_comment_post_id>;
  2. Edit that post via the API (PATCH /api/posts/<id> with new content) or via the web UI.
  3. Observe storage/logs/flarum-YYYY-MM-DD.log: a TypeError is logged for each edit.

Expected Behavior

The Revised event should fire successfully even when the original content was NULL. Listeners that genuinely need a string can fall back to '' themselves.

Suggested Fix

Either:

  • Change Revised::$oldContent to ?string in framework/core/src/Post/Event/Revised.php — more honest (a null old content is meaningful: it tells listeners the post had no prior content), but theoretically breaks any extension that does string operations on $oldContent without a null check.
  • Or in CommentPost::revise(), coalesce at the call site: new Revised($this, $actor, $oldContent ?? ''). Fully backwards-compatible for listeners.

The second is the safest. Happy to send a PR if it would be useful.

Environment

  • Flarum core: 2.0.0-rc.1
  • PHP: 8.4.20
  • MySQL: 8.4.9
  • The string type on $oldContent is still present on the main branch of flarum/framework as of the date of this report, so this affects current dev as well.

Possible Solution

See "Suggested Fix" above.

Additional Context

Related (closed) issue: #3715 — that issue requested $oldContent be exposed on the Revised event, which is what introduced the non-nullable parameter. The fix correctly added the parameter but did not account for the fact that posts.content is nullable in the schema.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions