JSON rendering: Avoid exception iterating on non-iterable objects
ClosedPublic
Actions

Authored by aklapper on Dec 11 2024, 20:14.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Commits

rARCf0176263a7e6: JSON rendering: Avoid exception iterating on non-iterable objects

Summary

Check for is_iterable($object) (available since PHP 7.1) to avoid an exception calling foreach on $object in the rare case that the object is a custom Phorge class (PHUIBoxView, PhutilSafeHTML) and not an array. cf https://we.phorge.it/rARC7570dd0da119627ff83bc6db3be06b51eb5b366b for a similar patch to handle PHP stdClass objects.

See downstream https://phabricator.wikimedia.org/T373316.

Test Plan

Unclear to reproduce the actual issue, likely something Feed related.
Open an existing JSON Paste at http://phorge.localhost/P1 and see that it still renders.
Open an existing JSON File at http://phorge.localhost/F1 and see that it still renders.

Diff Detail

Repository

rARC Arcanist

Branch

jsonParserNonIterable (branched from master)

Lint

Lint Passed

Unit

Test Failures

Build Status

Buildable 1663
Build 1663: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	798 ms	ArcanistBundleTestCase::testGitRepository EXCEPTION (Exception): Expected patch and actual patch for 5dec8bf28557f078d1987c4e8cfb53d08310f522 differ. Wrote actual patch to '/var/www/html/phorge/arcanist/src/parser/__tests__/patches//5dec8bf28557f078d1987c4e8cfb53d08310f522.gitpatch.real'. #0 /var/www/html/phorge/arcanist/src/parser/__tests__/ArcanistBundleTestCase.php(87): ArcanistBundleTestCase->runGitRepositoryTests(Object(PhutilDirectoryFixture)) #1 /var/www/html/phorge/arcanist/src/unit/engine/phutil/PhutilTestCase.php(639): ArcanistBundleTestCase->testGitRepository()
	48 ms	ArcanistBaseCommitParserTestCase::testBasics 2 assertion(s) passed.
	57 ms	ArcanistBaseCommitParserTestCase::testHalt 1 assertion(s) passed.
	46 ms	ArcanistBaseCommitParserTestCase::testJump 1 assertion(s) passed.
	52 ms	ArcanistBaseCommitParserTestCase::testJumpReturn 1 assertion(s) passed.
		View Full Test Results (1 Failed · 74 Passed)

Event Timeline

aklapper created this revision.Dec 11 2024, 20:14

Owners added a reviewer: O1: Blessed Committers.Dec 11 2024, 20:14

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptDec 11 2024, 20:14

aklapper requested review of this revision.Dec 11 2024, 20:14

Harbormaster completed remote builds in B1663: Diff 2588.Dec 11 2024, 20:14

I see a red light in a unit test but it works on me after the patch src/parser/__tests__/ArcanistBundleTestCase.php - any comment on that? Anyway sgtm since it needs the foreach() stuff later.

sgtm