mirror of
https://github.com/gohugoio/hugo.git
synced 2026-08-24 15:28:54 +00:00
hugolib: Fix slice bounds panic when deleting multiple nodes at same path
The contentNodes cases in Delete/DeleteFunc spliced the slice inside a forward range loop, panicking when a second deletion hit the last index, and the shrunken slice was never written back to the tree. Let the Shifter return the updated node and re-insert it on partial deletes. Fixes #15207 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -25,7 +25,7 @@ type contentNodeShifter struct {
|
||||
conf config.AllProvider // Used for logging/debugging.
|
||||
}
|
||||
|
||||
func (s *contentNodeShifter) Delete(n contentNode, vec sitesmatrix.Vector) (contentNode, bool, bool) {
|
||||
func (s *contentNodeShifter) Delete(n contentNode, vec sitesmatrix.Vector) (contentNode, contentNode, bool, bool) {
|
||||
switch v := n.(type) {
|
||||
case contentNodesMap:
|
||||
deleted, wasDeleted := v[vec]
|
||||
@@ -33,49 +33,53 @@ func (s *contentNodeShifter) Delete(n contentNode, vec sitesmatrix.Vector) (cont
|
||||
delete(v, vec)
|
||||
resource.MarkStale(deleted)
|
||||
}
|
||||
return deleted, wasDeleted, len(v) == 0
|
||||
return v, deleted, wasDeleted, len(v) == 0
|
||||
case contentNodeForSite:
|
||||
if v.siteVector() != vec {
|
||||
return nil, false, false
|
||||
return v, nil, false, false
|
||||
}
|
||||
resource.MarkStale(v)
|
||||
return v, true, true
|
||||
return nil, v, true, true
|
||||
case contentNodes:
|
||||
var deleted contentNodes
|
||||
for i, nn := range v {
|
||||
if vv, ok, _ := s.Delete(nn, vec); ok {
|
||||
var deleted, remaining contentNodes
|
||||
for _, nn := range v {
|
||||
updated, vv, ok, isEmpty := s.Delete(nn, vec)
|
||||
if ok {
|
||||
deleted = append(deleted, vv)
|
||||
v = append(v[:i], v[i+1:]...)
|
||||
}
|
||||
if !isEmpty {
|
||||
remaining = append(remaining, updated)
|
||||
}
|
||||
}
|
||||
if len(deleted) == 0 {
|
||||
return nil, false, false
|
||||
return v, nil, false, false
|
||||
}
|
||||
return deleted, true, len(v) == 0
|
||||
return remaining, deleted, true, len(remaining) == 0
|
||||
default:
|
||||
v = v.(contentNodeSingle) // Ensure single node.
|
||||
resource.MarkStale(v)
|
||||
return v, true, true
|
||||
|
||||
vv := v.(contentNodeSingle) // Ensure single node.
|
||||
resource.MarkStale(vv)
|
||||
return nil, vv, true, true
|
||||
}
|
||||
}
|
||||
|
||||
func (s *contentNodeShifter) DeleteFunc(v contentNode, f func(n contentNode) bool) bool {
|
||||
func (s *contentNodeShifter) DeleteFunc(v contentNode, f func(n contentNode) bool) (contentNode, bool) {
|
||||
switch ss := v.(type) {
|
||||
case contentNodeSingle:
|
||||
if f(ss) {
|
||||
resource.MarkStale(ss)
|
||||
return true
|
||||
return nil, true
|
||||
}
|
||||
return false
|
||||
return ss, false
|
||||
case contentNodes:
|
||||
for i, n := range ss {
|
||||
var remaining contentNodes
|
||||
for _, n := range ss {
|
||||
if f(n) {
|
||||
resource.MarkStale(n)
|
||||
ss = append(ss[:i], ss[i+1:]...)
|
||||
} else {
|
||||
remaining = append(remaining, n)
|
||||
}
|
||||
}
|
||||
return len(ss) == 0
|
||||
return remaining, len(remaining) == 0
|
||||
case contentNodesMap:
|
||||
for k, n := range ss {
|
||||
if f(n) {
|
||||
@@ -83,7 +87,7 @@ func (s *contentNodeShifter) DeleteFunc(v contentNode, f func(n contentNode) boo
|
||||
delete(ss, k)
|
||||
}
|
||||
}
|
||||
return len(ss) == 0
|
||||
return ss, len(ss) == 0
|
||||
default:
|
||||
panic(fmt.Sprintf("DeleteFunc: unknown type %T", v))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
// Copyright 2025 The Hugo Authors. All rights reserved.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package hugolib
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
qt "github.com/frankban/quicktest"
|
||||
"github.com/gohugoio/hugo/hugolib/sitesmatrix"
|
||||
)
|
||||
|
||||
type testContentNodeForSite struct {
|
||||
vec sitesmatrix.Vector
|
||||
}
|
||||
|
||||
func (n *testContentNodeForSite) Path() string { return "/test" }
|
||||
func (n *testContentNodeForSite) nodeCategorySingle() {}
|
||||
func (n *testContentNodeForSite) siteVector() sitesmatrix.Vector { return n.vec }
|
||||
func (n *testContentNodeForSite) forEeachContentNode(f func(sitesmatrix.Vector, contentNode) bool) bool {
|
||||
return f(n.vec, n)
|
||||
}
|
||||
|
||||
// See issue 15207.
|
||||
func TestContentNodeShifterDeleteMultipleFromNodes(t *testing.T) {
|
||||
c := qt.New(t)
|
||||
|
||||
s := &contentNodeShifter{}
|
||||
vec := sitesmatrix.Vector{2, 0, 0}
|
||||
other := sitesmatrix.Vector{1, 0, 0}
|
||||
|
||||
newNodes := func() contentNodes {
|
||||
return contentNodes{
|
||||
&testContentNodeForSite{vec: other},
|
||||
&testContentNodeForSite{vec: vec},
|
||||
&testContentNodeForSite{vec: other},
|
||||
&testContentNodeForSite{vec: other},
|
||||
&testContentNodeForSite{vec: vec},
|
||||
}
|
||||
}
|
||||
|
||||
updated, deleted, wasDeleted, isEmpty := s.Delete(newNodes(), vec)
|
||||
c.Assert(wasDeleted, qt.IsTrue)
|
||||
c.Assert(isEmpty, qt.IsFalse)
|
||||
c.Assert(deleted.(contentNodes), qt.HasLen, 2)
|
||||
remaining := updated.(contentNodes)
|
||||
c.Assert(remaining, qt.HasLen, 3)
|
||||
for _, n := range remaining {
|
||||
c.Assert(n.(contentNodeForSite).siteVector(), qt.Equals, other)
|
||||
}
|
||||
|
||||
// Delete all.
|
||||
updated, deleted, wasDeleted, isEmpty = s.Delete(contentNodes{
|
||||
&testContentNodeForSite{vec: vec},
|
||||
&testContentNodeForSite{vec: vec},
|
||||
}, vec)
|
||||
c.Assert(wasDeleted, qt.IsTrue)
|
||||
c.Assert(isEmpty, qt.IsTrue)
|
||||
c.Assert(deleted.(contentNodes), qt.HasLen, 2)
|
||||
c.Assert(updated, qt.IsNil)
|
||||
|
||||
// Delete none.
|
||||
nodes := contentNodes{&testContentNodeForSite{vec: other}}
|
||||
updated, deleted, wasDeleted, isEmpty = s.Delete(nodes, vec)
|
||||
c.Assert(wasDeleted, qt.IsFalse)
|
||||
c.Assert(isEmpty, qt.IsFalse)
|
||||
c.Assert(deleted, qt.IsNil)
|
||||
c.Assert(updated.(contentNodes), qt.HasLen, 1)
|
||||
}
|
||||
@@ -258,12 +258,15 @@ func (s *testShifter) Insert(old, new *testValue) (*testValue, *testValue, bool)
|
||||
return new, old, true
|
||||
}
|
||||
|
||||
func (s *testShifter) Delete(n *testValue, dimension sitesmatrix.Vector) (*testValue, bool, bool) {
|
||||
return nil, true, true
|
||||
func (s *testShifter) Delete(n *testValue, dimension sitesmatrix.Vector) (*testValue, *testValue, bool, bool) {
|
||||
return nil, n, true, true
|
||||
}
|
||||
|
||||
func (s *testShifter) DeleteFunc(v *testValue, f func(*testValue) bool) bool {
|
||||
return f(v)
|
||||
func (s *testShifter) DeleteFunc(v *testValue, f func(*testValue) bool) (*testValue, bool) {
|
||||
if f(v) {
|
||||
return nil, true
|
||||
}
|
||||
return v, false
|
||||
}
|
||||
|
||||
func (s *testShifter) Shift(n *testValue, dimension sitesmatrix.Vector, fallback bool) (v *testValue, ok bool) {
|
||||
|
||||
@@ -49,12 +49,14 @@ type (
|
||||
// and a bool indicating if an existing record is updated.
|
||||
Insert(old, new T) (T, T, bool)
|
||||
|
||||
// Delete deletes T from the given dimension and returns the deleted T and whether the dimension was deleted and if it's empty after the delete.
|
||||
Delete(v T, dimension sitesmatrix.Vector) (T, bool, bool)
|
||||
// Delete deletes T from the given dimension.
|
||||
// It returns the updated T, the deleted T, whether anything was deleted
|
||||
// and whether T is empty after the delete.
|
||||
Delete(v T, dimension sitesmatrix.Vector) (T, T, bool, bool)
|
||||
|
||||
// DeleteFunc deletes nodes in v from the tree where the given function returns true.
|
||||
// It returns true if it's empty after the delete.
|
||||
DeleteFunc(v T, f func(n T) bool) bool
|
||||
// DeleteFunc deletes nodes in v where the given function returns true.
|
||||
// It returns the updated T and whether it's empty after the delete.
|
||||
DeleteFunc(v T, f func(n T) bool) (T, bool)
|
||||
|
||||
// Shift shifts v into the given dimension,
|
||||
// if fallback is true, it will fall back a fallback match if found.
|
||||
@@ -118,7 +120,7 @@ func (r *NodeShiftTree[T]) DeleteFuncRaw(key string, f func(T) bool) (T, int) {
|
||||
return lastDeleted, count
|
||||
}
|
||||
|
||||
isEmpty := r.shifter.DeleteFunc(v, func(n T) bool {
|
||||
updated, isEmpty := r.shifter.DeleteFunc(v, func(n T) bool {
|
||||
if f(n) {
|
||||
count++
|
||||
lastDeleted = n
|
||||
@@ -129,6 +131,8 @@ func (r *NodeShiftTree[T]) DeleteFuncRaw(key string, f func(T) bool) (T, int) {
|
||||
|
||||
if isEmpty {
|
||||
r.tree.Delete(key)
|
||||
} else if count > 0 {
|
||||
r.tree.Insert(key, updated)
|
||||
}
|
||||
|
||||
return lastDeleted, count
|
||||
@@ -161,10 +165,15 @@ func (r *NodeShiftTree[T]) delete(key string) (T, bool) {
|
||||
var wasDeleted bool
|
||||
var deleted T
|
||||
if v, ok := r.tree.Get(key); ok {
|
||||
var isEmpty bool
|
||||
deleted, wasDeleted, isEmpty = r.shifter.Delete(v, r.siteVector)
|
||||
var (
|
||||
updated T
|
||||
isEmpty bool
|
||||
)
|
||||
updated, deleted, wasDeleted, isEmpty = r.shifter.Delete(v, r.siteVector)
|
||||
if isEmpty {
|
||||
r.tree.Delete(key)
|
||||
} else if wasDeleted {
|
||||
r.tree.Insert(key, updated)
|
||||
}
|
||||
}
|
||||
return deleted, wasDeleted
|
||||
|
||||
Reference in New Issue
Block a user