WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#30518 closed defect (bug) (duplicate)

WP_Dependencies::recurse_deps can recurse infinitely

Reported by: ccomp5950 Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.0
Component: Script Loader Keywords: has-patch needs-unit-tests
Focuses: performance Cc:

Description

Script dependencies can recurse infinitely (unto memory exhaustion, etc.)

http://i.imgur.com/uVHDCpX.png

Patch attached, sets a maximum depth and when it is reached stops recursion.

Attachments (1)

30518.patch (1.1 KB) - added by ccomp5950 4 years ago.

Download all attachments as: .zip

Change History (13)

@ccomp5950
4 years ago

#1 @ccomp5950
4 years ago

  • Focuses javascript performance added
  • Keywords has-patch added

#2 @ccomp5950
4 years ago

  • Focuses javascript removed

#3 @johnbillion
4 years ago

  • Version changed from trunk to 4.0

#4 @wonderboymusic
3 years ago

  • Keywords reporter-feedback added

Thanks for the report - can you provide the code called in WC_Frontend_Scripts? I am curious what triggers this recursion on your end.

#5 @ccomp5950
3 years ago

Hi,

The code is from woo-commerce I believe, I can check back with the customer. (I work for their hosting company and after they threw their hands in the air asked us to take a look at it, diagnosing stuff like this is outside the scope of our managed services agreement but I like a challenge so took a look at it)

It's been nearly a month so I'm not sure, and since it was a demonstration site chances are it wasn't ever added to the backup system.

#6 @ccomp5950
3 years ago

  • Keywords reporter-feedback removed

#8 @SergeyBiryukov
3 years ago

#29622 was marked as a duplicate.

This ticket was mentioned in Slack in #core by ccomp5950. View the logs.


3 years ago

#10 @boonebgorges
3 years ago

  • Keywords needs-unit-tests added

Thanks for the report. Using a depth counter like this seems like a sorta hackish workaround that has the potential to cause problems with legitimately complex dependencies. How about doing some actual loop detection? Like maybe recurse_deps() should accept a third param that is a flat array of unique handles identified in the tree, and then if it detects that you're looking at a handle that's already been recursed, it will bail?

#11 @ccomp5950
3 years ago

I've spent the last bit or so trying to reproduce this error and the only way I can is to purposefully create a circular requirement of scripts. Either a script requiring itself or a script requiring a script that requires the original script.

So in essence this bug is probably a duplicate of this: #21520

#12 @johnbillion
3 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Agreed. Thanks for the feedback ccomp5950. Marking as dupe of #21520.

Note: See TracTickets for help on using tickets.