Paths

Table of Contentst

Differential D25855

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

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

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

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.Wed, Dec 11, 20:14

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

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

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

Harbormaster completed remote builds in B1663: Diff 2588.Wed, Dec 11, 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

This revision is now accepted and ready to land.Thu, Dec 12, 10:14

Revision Contents
Changeset List

Path

Size

Coverage (All)

Coverage (Touched)

Packages

src/

parser/

PhutilJSON.php

2 lines

74%

O1: Blessed Committers

Diff 2588

View Options

JSON rendering: Avoid exception iterating on non-iterable objectsAcceptedPublicActions