WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 3 years ago

#24461 closed defect (bug) (fixed)

_get_term_children can loop forever (well until it times out) if there is a loop in the hierarchy

Reported by: westi Owned by: boonebgorges
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.6
Component: Taxonomy Keywords: has-patch
Focuses: Cc:

Description

_get_term_children call's itself recursively to build the array of children. If there is for some reason a loop in the hierarchy it gets stuck and loops for ever trying to build the list of descendents.

Seeing as we fetch the whole hierarchy with _get_term_hierarchy we can probably just use that to build an array of descendents without recursion.

Otherwise we should leverage our loop detection code to find and maybe break loops.

Attachments (2)

patch.diff (1.1 KB) - added by sgrant 3 years ago.
Test to reproduce a fatal error in _get_term_children
24461.patch (3.9 KB) - added by boonebgorges 3 years ago.

Download all attachments as: .zip

Change History (12)

#2 @markjaquith
5 years ago

  • Milestone changed from Awaiting Review to Future Release

#3 @rmarks
5 years ago

Can someone point me toward the loop detection code? I'm willing to write a patch.

#4 @rmarks
5 years ago

  • Cc rmarks@… added

This ticket was mentioned in IRC in #wordpress-dev by SergeyBiryukov. View the logs.


4 years ago

#7 @boonebgorges
4 years ago

  • Keywords good-first-bug added

#8 @sgrant
3 years ago

I wrote a test that I think is reproducing this behaviour (see attached diff), but I'm not sure how best to apply the filter to fix it. Is this the right approach for this issue? Thanks!

Also, it appears that the same parent/child setup can send get_term_children (no leading underscore) into a fatal error as well.

@sgrant
3 years ago

Test to reproduce a fatal error in _get_term_children

#9 @boonebgorges
3 years ago

  • Keywords has-patch added; needs-patch good-first-bug removed
  • Milestone changed from Future Release to 4.2

sgrant - Thanks for the test. We should just be able to keep track on the fly of which items are identified as part of the hierarchy branch, then pass those $ancestors by reference when we recurse, and make sure we don't recurse if we've already identified the term in question as part of the branch. See 24461.patch.

The patch subsumes [27837], since the case where a term has itself as a parent is just one specific instance of the general problem of hierarchy loops. 24461.patch is a more general solution.

Could I get a second set of eyes on this?

@boonebgorges
3 years ago

#10 @boonebgorges
3 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 31207:

Bail out of hierarchy loops in _get_term_children().

This prevents infinite loops that lead to PHP nesting limit fatal errors.

Props boonebgorges, sgrant.
Fixes #24461.

Note: See TracTickets for help on using tickets.