Skip to content

Commit 1ad0465

Browse files
committed
fix(OCISDEV-877): skip public shares with nil resource_id in ListPublicShares
1 parent ad85079 commit 1ad0465

2 files changed

Lines changed: 44 additions & 0 deletions

File tree

pkg/publicshare/manager/json/json.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,14 @@ func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters []
523523
continue
524524
}
525525

526+
if local.ResourceId == nil {
527+
log.Warn().
528+
Str("share_id", local.PublicShare.GetId().GetOpaqueId()).
529+
Str("share_token", local.Token).
530+
Msg("ListPublicShares: skipping share with nil resource_id")
531+
continue
532+
}
533+
526534
key := strings.Join([]string{local.ResourceId.StorageId, local.ResourceId.OpaqueId}, "!")
527535
if _, hit := cache[key]; !hit && !publicshare.IsCreatedByUser(&local.PublicShare, u) {
528536
sRes, err := client.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: local.ResourceId}})

pkg/publicshare/manager/json/json_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package json_test
2020

2121
import (
2222
"context"
23+
encjson "encoding/json"
2324
"os"
2425
"path/filepath"
2526
"sync"
@@ -130,6 +131,41 @@ var _ = Describe("Json", func() {
130131
})
131132
})
132133

134+
Describe("ListPublicShares", func() {
135+
It("skips shares whose persisted resource_id is nil instead of panicking", func() {
136+
// Create one valid share so the manager has a healthy row to compare against.
137+
validShare, err := m.CreatePublicShare(ctx, user1, sharedResource, &link.Grant{
138+
Permissions: &link.PublicSharePermissions{
139+
Permissions: &providerv1beta1.ResourcePermissions{},
140+
},
141+
})
142+
Expect(err).ToNot(HaveOccurred())
143+
144+
// Inject a corrupt row directly into the persistence file: the share's stored
145+
// JSON has no `resource_id`, so after unmarshal `local.ResourceId` is nil.
146+
// This mirrors the production state described in OCISDEV-862.
147+
raw, err := os.ReadFile(tmpFile.Name())
148+
Expect(err).ToNot(HaveOccurred())
149+
150+
db := map[string]interface{}{}
151+
Expect(encjson.Unmarshal(raw, &db)).To(Succeed())
152+
153+
db["corrupt-share-id"] = map[string]interface{}{
154+
"share": `{"id":{"opaque_id":"corrupt-share-id"},"token":"corrupt-token"}`,
155+
"password": "",
156+
}
157+
patched, err := encjson.Marshal(db)
158+
Expect(err).ToNot(HaveOccurred())
159+
Expect(os.WriteFile(tmpFile.Name(), patched, 0644)).To(Succeed())
160+
161+
// Listing must not panic and must return the valid share.
162+
shares, err := m.ListPublicShares(ctx, user1, []*link.ListPublicSharesRequest_Filter{}, false)
163+
Expect(err).ToNot(HaveOccurred())
164+
Expect(len(shares)).To(Equal(1))
165+
Expect(shares[0].Id.OpaqueId).To(Equal(validShare.Id.OpaqueId))
166+
})
167+
})
168+
133169
Describe("Load", func() {
134170
It("loads shares including state and mountpoint information", func() {
135171
existingShare, err := m.CreatePublicShare(ctx, user1, sharedResource, &link.Grant{

0 commit comments

Comments
 (0)