Skip to content

Commit 42a1fd4

Browse files
thc1006liamfallon
andauthored
Skip binary files in DB cache sync (kptdev#437)
* Skip binary files and NUL bytes in DB cache sync Binary files with invalid UTF-8 and files containing NUL bytes (0x00) both cause PostgreSQL TEXT columns to reject the INSERT. While utf8.ValidString() catches invalid UTF-8, NUL is technically valid UTF-8 per RFC 3629 but PostgreSQL still rejects it. Add strings.Contains(val, "\x00") check alongside utf8.ValidString() to also filter files containing NUL bytes. Add 5 test cases covering: mixed text/binary files, all text files, all binary files, empty resources, and NUL byte content that passes utf8.ValidString(). Fixes: nephio-project/nephio#1040 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> * Also validate resource_key (file path) for PostgreSQL TEXT safety resource_key is also a TEXT column, so file paths with invalid UTF-8 (e.g. Latin-1 encoded paths from older systems) would crash the INSERT. Added validation for both key and value, plus a test case. Fixes: nephio-project/nephio#1040 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> * Add defer Close and mock CacheInstance to test functions The 6 test functions added for binary file filtering were missing defer testRepo.Close(ctx) after OpenRepository, which could leak resources if assertions fail. Also added mock CacheInstance setup since Close() internally calls pkgScanRowsFromDB which requires CacheInstance to resolve repository references. Follows the same pattern used in TestRepositorySync_SyncOnce and TestNewRepositorySync. Fixes: nephio-project/nephio#1040 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> * Add nil guard for GetResources and improve test coverage - Guard against nil return from GetResources() to prevent panic, since the PackageRevision interface contract allows (nil, nil) - Add TestCacheExternalPRs_NilResources to cover the nil guard path - Use testify idiomatic assertions (t.Empty, t.Len) for consistency with the rest of the dbcache test suite Fixes: nephio-project/nephio#1040 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --------- Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> Co-authored-by: Liam Fallon <35595825+liamfallon@users.noreply.github.com>
1 parent eed4714 commit 42a1fd4

2 files changed

Lines changed: 626 additions & 1 deletion

File tree

pkg/cache/dbcache/dbreposync.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ package dbcache
1717
import (
1818
"context"
1919
"fmt"
20+
"strings"
2021
stdSync "sync"
2122
"time"
23+
"unicode/utf8"
2224

2325
porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
2426
configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
@@ -224,6 +226,24 @@ func (s *repositorySync) cacheExternalPRs(ctx context.Context, externalPrMap map
224226
return err
225227
}
226228

229+
// Guard against nil return from GetResources (interface contract allows it).
230+
var resources map[string]string
231+
if extPRResources == nil || extPRResources.Spec.Resources == nil {
232+
resources = make(map[string]string)
233+
} else {
234+
// Filter out files with invalid UTF-8 or NUL bytes to avoid PostgreSQL TEXT errors.
235+
// Both resource_key and resource_value are TEXT columns, so both must be validated.
236+
resources = make(map[string]string, len(extPRResources.Spec.Resources))
237+
for key, val := range extPRResources.Spec.Resources {
238+
if !utf8.ValidString(key) || strings.Contains(key, "\x00") ||
239+
!utf8.ValidString(val) || strings.Contains(val, "\x00") {
240+
klog.Warningf("repositorySync %+v: skipping file %q in PR %+v (not compatible with PostgreSQL TEXT)", s.repo.Key(), key, extPRKey)
241+
continue
242+
}
243+
resources[key] = val
244+
}
245+
}
246+
227247
if extAPIPR.CreationTimestamp.Time.IsZero() {
228248
extAPIPR.CreationTimestamp.Time = time.Now()
229249
}
@@ -239,7 +259,7 @@ func (s *repositorySync) cacheExternalPRs(ctx context.Context, externalPrMap map
239259
lifecycle: extAPIPR.Spec.Lifecycle,
240260
extPRID: extPRUpstreamLock,
241261
tasks: extAPIPR.Spec.Tasks,
242-
resources: extPRResources.Spec.Resources,
262+
resources: resources,
243263
}
244264
_, err = s.repo.savePackageRevision(ctx, &dbPR, true)
245265
if err != nil {

0 commit comments

Comments
 (0)