1.1.1#696
Merged
Merged
Conversation
ParsedownExtra can throw PHP Error (not Exception) when DOMDocument returns unexpected structure on malformed HTML input (e.g. a label containing a bare <head> tag). The existing catch block in SurveyStudy::addItems used `Exception`, which silently re-threw any `Error`, crashing the survey import with a fatal. Widen all seven call sites to `catch (\Throwable $e)` and fall back to the raw text on failure, logging via formr_log_exception. Sites fixed: - SurveyStudy::addItems – item label + choice label - Run::saveSettings – description, public_blurb, footer_text, privacy, tos - Email::save – email body - Pause::save – pause body
…cover Page.php, alert authors - Replace the seven copy-pasted try/catch guards with a single parsedown_text_safe() helper in Functions.php - Guard the previously missed chained ->text() call in Page::create() - On parse failure, alert the study author naming the affected field instead of silently storing unparsed text - Upgrade erusev/parsedown 1.7.4 -> 1.8.0 and erusev/parsedown-extra 0.8.1 -> 0.9.0 (verified the upgrade alone does not fix the DOMDocument crash: 0.9.0 ships the same unguarded processTagRoutine) - Add tests/ParsedownTextSafeTest.php covering fallback, author alert, HTML-escaping of the source label, and the real <head> crash input
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
Call to a member function getAttribute() on null/TypeError: DOMNode::replaceChild(): Argument #1 ($node) must be of type DOMNode, null given) when a survey item label or run text field contains malformed HTML (e.g. a bare<head>tag). ParsedownExtra's DOMDocument block processor (processTagRoutine) dereferences DOM nodes without null checks and crashes with a PHPError— not anException— so the existingcatch (Exception)guard inSurveyStudy::addItemsdid not intercept it.text()call sites now go through a newparsedown_text_safe()helper (Functions.php): catches\Throwable, stores the raw (unparsed) text, logs viaformr_log_exception, and shows the study author a warning naming the affected field. Covered sites: survey item labels, choice labels, run description/public_blurb/footer_text/privacy/tos, email body, pause text, and page body (the chained call inPage::create()was unguarded in the first iteration of this PR).Changes
erusev/parsedown1.7.4 → 1.8.0 anderusev/parsedown-extra0.8.1 → 0.9.0 (February 2026 maintenance releases). Verified the upgrade alone does not fix the crash — 0.9.0 ships the same unguardedprocessTagRoutine— hence the call-site guard.Tests
tests/ParsedownTextSafeTest.php: fallback to raw text on parserError, author alert naming the source field, HTML-escaping of the source label, and the real-world<head>crash input against the live vendor lib. Full unit lane green (162 tests).