From ba7b7383fb98e28cbddd862a851897b96d1017a0 Mon Sep 17 00:00:00 2001 From: Daniel Mejia Date: Tue, 25 Aug 2026 20:38:27 -0400 Subject: [PATCH] com_courses: fix outline editor exhausting memory on newer courses Section::access() wrote the section's own id into the shared permissions `offering_id`, so Permissions::_calculate() then asked Course::offering() for an offering whose id is really a section id. When no offering row carries that id, Course::offering() discards the correctly-cached offering and caches an id-less one in its place -- Tables\Offering::load() ignores course_id for a numeric id, so it simply finds nothing. Every later $course->offering() returned that empty model, so Offering::units() passed offering_id => 0 to the units query. The _buildQuery() guards read a falsy scope id as "no filter" rather than "match nothing", dropping the WHERE clause entirely, so the query returned every unit on the hub -- then every asset group, then every asset. That is ~14,500 asset models, each with its own table object and SHOW FULL COLUMNS round trip, exhausting a 256MB limit in about six seconds. Only the editasset/editwiki/edittool layouts die; the outline builder survives because it reads units before the permissions calculation has poisoned the cached offering. Courses whose section id happens to equal their offering id are unaffected, which is why this only shows up on courses recent enough for the two id sequences to have drifted apart. Corrects the offering_id, and hardens the three scope-id guards so a zero can no longer widen a scoped query into a full table scan. admin/controllers/assets.php passed 0 to mean "every scope", so it now omits the key rather than relying on that ambiguity. Also guards edittool.php against calling files() on a handler that does not implement it -- a url/link asset reaches that layout with a Models\Assets\Url handler and fatals. Ticket: https://nanohub.org/support/ticket/513503 Co-Authored-By: Claude Opus 5 --- .../com_courses/admin/controllers/assets.php | 11 ++++++++++- core/components/com_courses/models/section.php | 2 +- core/components/com_courses/tables/asset.group.php | 3 ++- core/components/com_courses/tables/asset.php | 3 ++- core/components/com_courses/tables/unit.php | 4 +++- .../courses/outline/views/outline/tmpl/edittool.php | 2 +- 6 files changed, 19 insertions(+), 6 deletions(-) diff --git a/core/components/com_courses/admin/controllers/assets.php b/core/components/com_courses/admin/controllers/assets.php index b37ea9f5a85..6466943c2e4 100644 --- a/core/components/com_courses/admin/controllers/assets.php +++ b/core/components/com_courses/admin/controllers/assets.php @@ -86,8 +86,17 @@ public function displayTask() $tbl = new Tables\Asset($this->database); // print_r($this->view->filters); + // A scope id of 0 here means "every scope", but the table now reads a + // zero as "match nothing", so drop the key instead of passing it. + $where = $this->view->filters; + + if (empty($where['asset_scope_id'])) + { + unset($where['asset_scope_id']); + } + $rows = $tbl->find(array( - 'w' => $this->view->filters + 'w' => $where )); // print_r($rows); diff --git a/core/components/com_courses/models/section.php b/core/components/com_courses/models/section.php index 74dd518a2fd..2b4d6595f36 100644 --- a/core/components/com_courses/models/section.php +++ b/core/components/com_courses/models/section.php @@ -310,7 +310,7 @@ public function access($action='view', $item='section') if (!isset($this->_permissions)) { $this->_permissions = Permissions::getInstance(); - $this->_permissions->set('offering_id', $this->get('id')); + $this->_permissions->set('offering_id', $this->get('offering_id')); $this->_permissions->set('section_id', $this->get('id')); } return $this->_permissions->access($action, $item); diff --git a/core/components/com_courses/tables/asset.group.php b/core/components/com_courses/tables/asset.group.php index a85c71c2144..a7f79a8a3f5 100644 --- a/core/components/com_courses/tables/asset.group.php +++ b/core/components/com_courses/tables/asset.group.php @@ -101,7 +101,8 @@ private function _buildQuery($filters=array()) $where = array(); - if (isset($filters['unit_id']) && $filters['unit_id']) + // A unit_id of 0 must match no groups rather than dropping the clause + if (isset($filters['unit_id'])) { $where[] = "cag.unit_id=" . $this->_db->quote($filters['unit_id']); } diff --git a/core/components/com_courses/tables/asset.php b/core/components/com_courses/tables/asset.php index fde041a88ce..cd247003a11 100644 --- a/core/components/com_courses/tables/asset.php +++ b/core/components/com_courses/tables/asset.php @@ -127,7 +127,8 @@ private function _buildQuery($filters=array()) { $where[] = "ca.id=" . $this->_db->quote((int) $filters['asset_id']); } - if (!empty($filters['asset_scope_id'])) + // A scope id of 0 must match no assets rather than dropping the clause + if (isset($filters['asset_scope_id'])) { $where[] = "cag.id=" . $this->_db->quote((int) $filters['asset_scope_id']); } diff --git a/core/components/com_courses/tables/unit.php b/core/components/com_courses/tables/unit.php index 8e252801699..17022b0edbc 100644 --- a/core/components/com_courses/tables/unit.php +++ b/core/components/com_courses/tables/unit.php @@ -128,7 +128,9 @@ private function _buildQuery($filters=array()) $where = array(); - if (isset($filters['offering_id']) && $filters['offering_id']) + // Note the deliberate lack of a truthiness test: an offering_id of 0 + // has to match no units, not every unit in the table. + if (isset($filters['offering_id'])) { $where[] = "cu.offering_id=" . $this->_db->quote($filters['offering_id']); } diff --git a/core/plugins/courses/outline/views/outline/tmpl/edittool.php b/core/plugins/courses/outline/views/outline/tmpl/edittool.php index 9f39b14e7f0..6ab9f3c72f1 100644 --- a/core/plugins/courses/outline/views/outline/tmpl/edittool.php +++ b/core/plugins/courses/outline/views/outline/tmpl/edittool.php @@ -86,7 +86,7 @@
- files($asset) : array(); ?> + files($asset) : array(); ?>