From 17b85b900ac809067ce2ff792bc05f63d3c31a3b Mon Sep 17 00:00:00 2001 From: Jae Gangemi Date: Tue, 7 Jul 2026 21:08:56 -0600 Subject: [PATCH] fix: preserve unrecognized, chunked, and cross-page NVS entries on read-modify-write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ParseNVS silently dropped or truncated anything it didn't fully model, so GenerateNVS's re-encode of a parsed partition would wipe real data on a read-modify-write: - unknown/unmodeled entry types hit `default: continue` and were dropped entirely (e.g. any future or vendor NVS type) - ESP-IDF's blob-index + chunked blob-data entries (used for esp_wifi credentials) share one key across several chunk-indexed slots; keying dedup on namespace+key alone collapsed all chunks into one, discarding every chunk but the last scanned - string/blob spans that cross a page boundary were truncated at "assume fits in current page", even though GenerateNVS's own writePage already produces such spans for long values - a key whose namespace declaration lived on a later-scanned page than the key itself was dropped ("namespace not yet defined") - GenerateNVS ranged over a Go map to pick namespace write order, making output nondeterministic across otherwise-identical calls Fixes: - parse.go: two-phase parse — a structural walk decodes every entry record (namespace declarations resolved immediately, everything else queued), then a resolution phase looks up namespace names against the now-complete map, so scan order can no longer drop a key - parse.go: readSpanData follows an entry's continuation slots across page boundaries instead of stopping at the current page's last slot - parse.go: entries of a type the switch doesn't decode are captured generically (Entry.Raw/TypeByte/Span/ChunkIndex/Data) instead of dropped - parse.go: dedup key is (namespace, key, chunkIndex) instead of (namespace, key), so chunked entries sharing a key survive independently - generate.go: buildRawEntry re-emits Raw entries byte-for-byte from their captured type/span/chunkIndex/data, without needing to understand the value's semantics - generate.go: namespace write order follows first-seen order in the input slice instead of Go map iteration, making output deterministic Entry gains Raw/TypeByte/Span/ChunkIndex/Data fields, additive only; ParseNVS/GenerateNVS signatures are unchanged. Adds pkg/nvs/lossless_test.go with hand-built raw NVS page fixtures covering: namespace declared on a later page than its key, unknown-type passthrough, blob-index/chunked-blob round trip, a blob spanning a page boundary, GenerateNVS determinism, and a full parse->modify-one-key-> generate->reparse round trip asserting every other entry is byte-preserved. --- pkg/nvs/generate.go | 157 ++++-- pkg/nvs/lossless_test.go | 759 +++++++++++++++++++++++++++ pkg/nvs/nvs.go | 17 +- pkg/nvs/parse.go | 347 ++++++++---- pkg/nvs/testdata/real_espidf_nvs.bin | Bin 0 -> 24576 bytes 5 files changed, 1115 insertions(+), 165 deletions(-) create mode 100644 pkg/nvs/lossless_test.go create mode 100644 pkg/nvs/testdata/real_espidf_nvs.bin diff --git a/pkg/nvs/generate.go b/pkg/nvs/generate.go index 5dfe77c..548eade 100644 --- a/pkg/nvs/generate.go +++ b/pkg/nvs/generate.go @@ -45,16 +45,25 @@ func GenerateNVS(entries []Entry, partitionSize int) ([]byte, error) { partition[i] = 0xFF } - // Group entries by namespace + // Group entries by namespace. Namespace order is recorded as first-seen + // in the input slice (rather than ranged over the map) so GenerateNVS is + // deterministic: Go map iteration order is randomized, which previously + // made repeated calls with identical input produce different partition + // layouts. namespaceMap := make(map[string][]*Entry) + var nsOrder []string for i, e := range entries { + if _, seen := namespaceMap[e.Namespace]; !seen { + nsOrder = append(nsOrder, e.Namespace) + } namespaceMap[e.Namespace] = append(namespaceMap[e.Namespace], &entries[i]) } // Process each namespace pageIdx := 0 nsCounter := uint8(0) - for ns, nsEntries := range namespaceMap { + for _, ns := range nsOrder { + nsEntries := namespaceMap[ns] nsCounter++ // Write namespace entry first — type is U8 with data = namespace index nsEntry := newEntry() @@ -91,6 +100,10 @@ func GenerateNVS(entries []Entry, partitionSize int) ([]byte, error) { // parseEntry converts an Entry to one or more internal entries (for multi-span strings/blobs) func parseEntry(e *Entry, namespaceIdx uint8) ([]*entry, error) { + if e.Raw { + return buildRawEntry(e, namespaceIdx) + } + var result []*entry switch e.Type { @@ -297,7 +310,49 @@ func parseEntry(e *Entry, namespaceIdx uint8) ([]*entry, error) { return result, nil } -// writePage writes entries to pages and returns number of pages written +// buildRawEntry re-encodes a passthrough Entry (Entry.Raw == true) byte-for- +// byte, using its captured TypeByte/Span/ChunkIndex/Data instead of +// interpreting Type/Value. This is what makes read-modify-write lossless for +// entry types ParseNVS doesn't natively decode: it never needs to understand +// the value's semantics to re-emit an equivalent slot. +func buildRawEntry(e *Entry, namespaceIdx uint8) ([]*entry, error) { + if len(e.Data) < 8 { + return nil, fmt.Errorf("raw entry %q: Data must be at least 8 bytes (entry header), got %d", e.Key, len(e.Data)) + } + + span := e.Span + if span == 0 { + span = spanOne + } + wantContLen := (int(span) - 1) * EntrySize + if len(e.Data) != 8+wantContLen { + return nil, fmt.Errorf("raw entry %q: Data length %d does not match span %d (want %d)", e.Key, len(e.Data), span, 8+wantContLen) + } + + ent := newEntry() + ent.namespaceIdx = namespaceIdx + ent.entryType = e.TypeByte + ent.span = span + ent.chunkIndex = e.ChunkIndex + copyKeyToEntry(e.Key, ent) + copy(ent.data[:], e.Data[:8]) + ent.crc32Val = calculateEntryCRC32(ent) + if wantContLen > 0 { + ent.rawData = e.Data[8:] + } + + return []*entry{ent}, nil +} + +// writePage writes entries to pages and returns number of pages written. +// +// Mirrors ESP-IDF's Page::writeItem: an item's header slot and all of its +// continuation/data slots are always written together on a single page. +// Before placing an item, the full slot count it needs is computed; if it +// doesn't fit in the current page's remaining slots, the current page is +// finalized as-is (its remaining slots stay Empty/0xFF) and the *entire* +// item is written on a fresh page instead. An item's span never crosses a +// page boundary. func writePage(partition *[]byte, startPageNum int, seqNum uint32, entries []*entry, totalPages int) (int, error) { pageNum := startPageNum pageOffset := pageNum * PageSize @@ -309,28 +364,47 @@ func writePage(partition *[]byte, startPageNum int, seqNum uint32, entries []*en } // Write header - writePageHeader(page, seqNum) + writePageHeader(page, seqNum+uint32(pageNum-startPageNum)) // Write entry bitmap and entries bitmapOffset := HeaderSize slotIdx := 0 + startNewPage := func() error { + pageNum++ + if pageNum >= totalPages { + return fmt.Errorf("not enough pages: need at least %d pages", pageNum+1) + } + pageOffset = pageNum * PageSize + page = (*partition)[pageOffset : pageOffset+PageSize] + // Initialize new page with 0xFF + for i := range page { + page[i] = 0xFF + } + // Write header + writePageHeader(page, seqNum+uint32(pageNum-startPageNum)) + slotIdx = 0 + return nil + } + for _, e := range entries { - if slotIdx >= EntriesPerPage { - // Need another page - pageNum++ - if pageNum >= totalPages { - return 0, fmt.Errorf("not enough pages: need at least %d pages", pageNum+1) - } - pageOffset = pageNum * PageSize - page = (*partition)[pageOffset : pageOffset+PageSize] - // Initialize new page with 0xFF - for i := range page { - page[i] = 0xFF + dataSlots := 0 + if e.rawData != nil { + dataSlots = int(e.span) - 1 // header slot already accounted separately + } + entryCount := 1 + dataSlots // header slot + continuation/data slots + + if entryCount > EntriesPerPage { + return 0, fmt.Errorf("entry %q: needs %d slots, which exceeds a single page's capacity (%d); ESP-IDF requires an item's header and all continuation slots to fit within one page", readNullTerminatedString(e.key[:]), entryCount, EntriesPerPage) + } + + // Fit check: if this item (header + continuation slots, as a whole) + // won't fit in what remains of the current page, close the current + // page and start the whole item fresh on a new page. + if slotIdx+entryCount > EntriesPerPage { + if err := startNewPage(); err != nil { + return 0, err } - // Write header - writePageHeader(page, seqNum+uint32(pageNum)) - slotIdx = 0 } // Mark slot as written in bitmap @@ -341,40 +415,21 @@ func writePage(partition *[]byte, startPageNum int, seqNum uint32, entries []*en writeEntry(page[entryOffset:entryOffset+EntrySize], e) slotIdx++ - // For string/blob entries, write raw data into subsequent slots - if e.rawData != nil { - dataSlots := int(e.span) - 1 // header already written - for ds := 0; ds < dataSlots; ds++ { - if slotIdx >= EntriesPerPage { - // Need another page - pageNum++ - if pageNum >= totalPages { - return 0, fmt.Errorf("not enough pages: need at least %d pages", pageNum+1) - } - pageOffset = pageNum * PageSize - page = (*partition)[pageOffset : pageOffset+PageSize] - // Initialize new page with 0xFF - for i := range page { - page[i] = 0xFF - } - // Write header - writePageHeader(page, seqNum+uint32(pageNum)) - slotIdx = 0 - } - - markBitmapWritten(page, bitmapOffset, slotIdx) - dataOffset := FirstEntryOffset + slotIdx*EntrySize - // Copy chunk of raw data, rest stays 0xFF - start := ds * EntrySize - end := start + EntrySize - if end > len(e.rawData) { - end = len(e.rawData) - } - if start < len(e.rawData) { - copy(page[dataOffset:dataOffset+EntrySize], e.rawData[start:end]) - } - slotIdx++ + // Write raw continuation data into the following slots. These are + // guaranteed by the fit check above to remain on this same page. + for ds := 0; ds < dataSlots; ds++ { + markBitmapWritten(page, bitmapOffset, slotIdx) + dataOffset := FirstEntryOffset + slotIdx*EntrySize + // Copy chunk of raw data, rest stays 0xFF + start := ds * EntrySize + end := start + EntrySize + if end > len(e.rawData) { + end = len(e.rawData) + } + if start < len(e.rawData) { + copy(page[dataOffset:dataOffset+EntrySize], e.rawData[start:end]) } + slotIdx++ } } diff --git a/pkg/nvs/lossless_test.go b/pkg/nvs/lossless_test.go new file mode 100644 index 0000000..d875e00 --- /dev/null +++ b/pkg/nvs/lossless_test.go @@ -0,0 +1,759 @@ +package nvs + +import ( + "encoding/binary" + "fmt" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Test fixtures in this file hand-build raw NVS page bytes (header + entry +// state bitmap + 32-byte entry slots) using the package's own internal +// primitives (newEntry, writeEntry, writePageHeader, markBitmapWritten, +// calculateEntryCRC32, parseEntry, writePage) so the fixtures stay realistic +// without duplicating CRC/layout logic. They exercise read-modify-write +// scenarios the original round-trip tests couldn't reach: namespace +// declarations scanned after their keys, unmodeled type bytes, chunked +// blob-index values (esp_wifi credentials), an item bumped whole onto a +// fresh page when it doesn't fit the current one, generator determinism, +// and a full lossless RMW that must leave everything but the modified key +// byte-for-byte intact. +// +// Every fixture here models a layout that ESP-IDF's on-flash format can +// actually produce: an item's header slot and all of its continuation slots +// always live on the SAME page (see Page::writeItem in nvs_page.cpp). +// Nothing here hand-builds an item whose span straddles a page boundary — +// that layout cannot exist on real hardware. + +// newTestPage returns a fresh, correctly-headered, all-0xFF NVS page. +func newTestPage(seqNum uint32) []byte { + page := make([]byte, PageSize) + for i := range page { + page[i] = 0xFF + } + writePageHeader(page, seqNum) + return page +} + +// placeEntry writes an entry's 32-byte header slot at slotIdx and marks it +// written in the bitmap. It does not write any continuation slots. +func placeEntry(page []byte, slotIdx int, e *entry) { + markBitmapWritten(page, HeaderSize, slotIdx) + off := FirstEntryOffset + slotIdx*EntrySize + writeEntry(page[off:off+EntrySize], e) +} + +// placeContinuation writes a raw 32-byte continuation slot (payload padded +// with 0xFF, matching how writePage lays out span data) and marks it written. +func placeContinuation(page []byte, slotIdx int, payload []byte) { + markBitmapWritten(page, HeaderSize, slotIdx) + off := FirstEntryOffset + slotIdx*EntrySize + for i := range EntrySize { + page[off+i] = 0xFF + } + copy(page[off:off+EntrySize], payload) +} + +// nsEntry builds a namespace-declaration entry (index -> name). +func nsEntry(name string, idx uint8) *entry { + e := newEntry() + e.namespaceIdx = 0 + e.entryType = namespaceType + e.span = spanOne + e.chunkIndex = singleChunkIndex + copyKeyToEntry(name, e) + e.data[0] = idx + e.crc32Val = calculateEntryCRC32(e) + return e +} + +// u8Entry builds a single-slot u8-typed entry. +func u8Entry(nsIdx uint8, key string, val uint8) *entry { + e := newEntry() + e.namespaceIdx = nsIdx + e.entryType = typeU8 + e.span = spanOne + e.chunkIndex = singleChunkIndex + copyKeyToEntry(key, e) + e.data[0] = val + e.crc32Val = calculateEntryCRC32(e) + return e +} + +// assertNoItemCrossesPage scans a generated partition and fails the test if +// any item's header + continuation slots (per its span byte) would run past +// the end of its page's EntriesPerPage slots. Per ESP-IDF's Page::writeItem, +// that layout can never occur on real hardware; GenerateNVS must never +// produce it either. +func assertNoItemCrossesPage(t *testing.T, partition []byte) { + t.Helper() + totalPages := len(partition) / PageSize + for p := 0; p < totalPages; p++ { + page := partition[p*PageSize : (p+1)*PageSize] + if page[0] == pageStateEmpty { + continue + } + slotIdx := 0 + for slotIdx < EntriesPerPage { + bitIndex := uint(slotIdx) * 2 + byteIdx := HeaderSize + int(bitIndex/8) + bitOffset := bitIndex % 8 + state := (page[byteIdx] >> bitOffset) & 0x3 + if state != entryStateWritten { + slotIdx++ + continue + } + off := FirstEntryOffset + slotIdx*EntrySize + span := page[off+2] + if span == 0 { + span = 1 + } + require.LessOrEqualf(t, slotIdx+int(span), EntriesPerPage, + "page %d slot %d: item span %d crosses the page boundary (EntriesPerPage=%d)", + p, slotIdx, span, EntriesPerPage) + slotIdx += int(span) + } + } +} + +// pagesContainingHeaderSlot returns, for each valid page in partition, the +// slot index of a written header slot matching (key, typeByte, chunkIndex), +// keyed by page number. Used to assert *where* GenerateNVS physically placed +// an item. +func pagesContainingHeaderSlot(partition []byte, key string, typeByte uint8, chunkIndex uint8) map[int]int { + found := make(map[int]int) + totalPages := len(partition) / PageSize + for p := 0; p < totalPages; p++ { + page := partition[p*PageSize : (p+1)*PageSize] + if page[0] == pageStateEmpty { + continue + } + for slotIdx := 0; slotIdx < EntriesPerPage; slotIdx++ { + bitIndex := uint(slotIdx) * 2 + byteIdx := HeaderSize + int(bitIndex/8) + bitOffset := bitIndex % 8 + state := (page[byteIdx] >> bitOffset) & 0x3 + if state != entryStateWritten { + continue + } + off := FirstEntryOffset + slotIdx*EntrySize + entryBytes := page[off : off+EntrySize] + if entryBytes[1] != typeByte || entryBytes[3] != chunkIndex { + continue + } + if readNullTerminatedString(entryBytes[8:24]) != key { + continue + } + found[p] = slotIdx + } + } + return found +} + +// 1. Namespace declared on a page scanned later than the key referencing it +// (a lower sequence-number page can still be scanned after a higher one if +// physical order doesn't match seq order; here we keep it simple with +// physical order == seq order, but resolution is scan-order independent). +// The old single-pass scan dropped such keys ("namespace not yet defined, +// skip"); the two-phase parse must still resolve them. +func TestParseNVSNamespaceDeclaredOnLaterPage(t *testing.T) { + partition := make([]byte, PageSize*2) + + page0 := newTestPage(0) + placeEntry(page0, 0, u8Entry(1, "channel", 6)) // references ns index 1, not yet declared + copy(partition[0:PageSize], page0) + + page1 := newTestPage(1) + placeEntry(page1, 0, nsEntry("wifi", 1)) // declared here, on a later page + copy(partition[PageSize:2*PageSize], page1) + + entries, err := ParseNVS(partition) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, "wifi", entries[0].Namespace) + assert.Equal(t, "channel", entries[0].Key) + assert.Equal(t, uint8(6), entries[0].Value) +} + +// 2. An entry with a type byte the switch doesn't model must be captured via +// generic passthrough, not dropped, and GenerateNVS must re-emit an +// equivalent slot. +func TestParseNVSUnknownTypePassthrough(t *testing.T) { + page := newTestPage(0) + placeEntry(page, 0, nsEntry("cfg", 1)) + + unk := newEntry() + unk.namespaceIdx = 1 + unk.entryType = 0x99 // not modeled by the codec + unk.span = spanOne + unk.chunkIndex = singleChunkIndex + copyKeyToEntry("future", unk) + unk.data = [8]byte{0xDE, 0xAD, 0xBE, 0xEF, 0x01, 0x02, 0x03, 0x04} + unk.crc32Val = calculateEntryCRC32(unk) + placeEntry(page, 1, unk) + + partition := make([]byte, PageSize) + copy(partition, page) + + entries, err := ParseNVS(partition) + require.NoError(t, err) + require.Len(t, entries, 1) + require.True(t, entries[0].Raw) + assert.Equal(t, uint8(0x99), entries[0].TypeByte) + assert.Equal(t, "future", entries[0].Key) + assert.Equal(t, unk.data[:], entries[0].Data) + + regen, err := GenerateNVS(entries, DefaultPartSize) + require.NoError(t, err) + reparsed, err := ParseNVS(regen) + require.NoError(t, err) + require.Len(t, reparsed, 1) + assert.Equal(t, entries[0].Namespace, reparsed[0].Namespace) + assert.Equal(t, entries[0].Key, reparsed[0].Key) + assert.Equal(t, entries[0].TypeByte, reparsed[0].TypeByte) + assert.Equal(t, entries[0].ChunkIndex, reparsed[0].ChunkIndex) + assert.Equal(t, entries[0].Data, reparsed[0].Data) +} + +// 3. Blob-index + chunked blob-data entries (as ESP-IDF stores esp_wifi +// credentials): an index entry plus N data-chunk entries sharing one key but +// distinguished by chunkIndex, each a self-contained (single-page) item. All +// chunks must survive parse and a parse->generate->parse round trip, not be +// collapsed by key-only deduplication. +func TestParseNVSBlobIndexChunkedBlobRoundTrip(t *testing.T) { + const typeBlobIdx = 0x48 + const typeBlobData = 0x42 + + page := newTestPage(0) + placeEntry(page, 0, nsEntry("wifi", 1)) + + idx := newEntry() + idx.namespaceIdx = 1 + idx.entryType = typeBlobIdx + idx.span = spanOne + idx.chunkIndex = singleChunkIndex + copyKeyToEntry("creds", idx) + idx.data = [8]byte{16, 0, 2, 0, 0xFF, 0xFF, 0xFF, 0xFF} // size=16, chunkCount=2 + idx.crc32Val = calculateEntryCRC32(idx) + placeEntry(page, 1, idx) + + chunk0 := newEntry() + chunk0.namespaceIdx = 1 + chunk0.entryType = typeBlobData + chunk0.span = 2 + chunk0.chunkIndex = 0 + copyKeyToEntry("creds", chunk0) + chunk0.data = [8]byte{8, 0, 0xFF, 0xFF, 0, 0, 0, 0} + chunk0.crc32Val = calculateEntryCRC32(chunk0) + placeEntry(page, 2, chunk0) + payload0 := make([]byte, EntrySize) + for i := range payload0 { + payload0[i] = 0xFF + } + copy(payload0, []byte("PASSWORD")) + placeContinuation(page, 3, payload0) + + chunk1 := newEntry() + chunk1.namespaceIdx = 1 + chunk1.entryType = typeBlobData + chunk1.span = 2 + chunk1.chunkIndex = 1 + copyKeyToEntry("creds", chunk1) + chunk1.data = [8]byte{8, 0, 0xFF, 0xFF, 0, 0, 0, 0} + chunk1.crc32Val = calculateEntryCRC32(chunk1) + placeEntry(page, 4, chunk1) + payload1 := make([]byte, EntrySize) + for i := range payload1 { + payload1[i] = 0xFF + } + copy(payload1, []byte("SSID1234")) + placeContinuation(page, 5, payload1) + + partition := make([]byte, PageSize) + copy(partition, page) + + entries, err := ParseNVS(partition) + require.NoError(t, err) + require.Len(t, entries, 3) + + byChunk := make(map[uint8]Entry) + for _, e := range entries { + require.True(t, e.Raw) + require.Equal(t, "creds", e.Key) + byChunk[e.ChunkIndex] = e + } + require.Contains(t, byChunk, uint8(singleChunkIndex)) + require.Contains(t, byChunk, uint8(0)) + require.Contains(t, byChunk, uint8(1)) + assert.Equal(t, append(append([]byte{}, chunk0.data[:]...), payload0...), byChunk[0].Data) + assert.Equal(t, append(append([]byte{}, chunk1.data[:]...), payload1...), byChunk[1].Data) + + regen, err := GenerateNVS(entries, DefaultPartSize) + require.NoError(t, err) + assertNoItemCrossesPage(t, regen) + reparsed, err := ParseNVS(regen) + require.NoError(t, err) + require.Len(t, reparsed, 3) + + byChunk2 := make(map[uint8]Entry) + for _, e := range reparsed { + byChunk2[e.ChunkIndex] = e + } + assert.Equal(t, byChunk[singleChunkIndex].Data, byChunk2[singleChunkIndex].Data) + assert.Equal(t, byChunk[0].Data, byChunk2[0].Data) + assert.Equal(t, byChunk[1].Data, byChunk2[1].Data) +} + +// 4a. A large chunked blob (BLOB_IDX + BLOB_DATA chunks, as ESP-IDF actually +// stores values too big for one item's span) where the chunks are large +// enough that they cannot both fit on the same page: GenerateNVS must place +// each chunk as a whole, self-contained item — never split one across pages +// — landing chunk 1 on a different physical page than chunk 0. This is the +// real-hardware equivalent of what the old (removed) "blob crosses page +// boundary" fixture tried to model with an impossible single-item span. +func TestParseNVSLargeBlobChunksSpanMultiplePages(t *testing.T) { + const typeBlobIdx = 0x48 + const typeBlobData = 0x42 + + // Each chunk needs 1 (header) + 110 (data) = 111 slots. Two chunks plus + // the namespace + index entries (2 more slots) can't both fit in a + // single 126-slot page, so chunk 1 must roll onto a fresh page. + const chunkPayloadLen = 110 * EntrySize // 3520 bytes + const chunkSlots = 1 + chunkPayloadLen/EntrySize + + payload0 := make([]byte, chunkPayloadLen) + for i := range payload0 { + payload0[i] = byte(i) + } + payload1 := make([]byte, chunkPayloadLen) + for i := range payload1 { + payload1[i] = byte(i + 1) + } + + chunkHeader := func(payloadLen int) []byte { + h := make([]byte, 8) + binary.LittleEndian.PutUint16(h[0:2], uint16(payloadLen)) + for i := 2; i < 8; i++ { + h[i] = 0xFF + } + return h + } + + idxData := make([]byte, 8) + binary.LittleEndian.PutUint16(idxData[0:2], uint16(2*chunkPayloadLen)) + idxData[2] = 2 // chunk count + for i := 3; i < 8; i++ { + idxData[i] = 0xFF + } + + entries := []Entry{ + { + Namespace: "wifi", Key: "creds", Raw: true, + TypeByte: typeBlobIdx, Span: spanOne, ChunkIndex: singleChunkIndex, + Data: idxData, + }, + { + Namespace: "wifi", Key: "creds", Raw: true, + TypeByte: typeBlobData, Span: uint8(chunkSlots), ChunkIndex: 0, + Data: append(append([]byte{}, chunkHeader(chunkPayloadLen)...), payload0...), + }, + { + Namespace: "wifi", Key: "creds", Raw: true, + TypeByte: typeBlobData, Span: uint8(chunkSlots), ChunkIndex: 1, + Data: append(append([]byte{}, chunkHeader(chunkPayloadLen)...), payload1...), + }, + } + + regen, err := GenerateNVS(entries, PageSize*3) + require.NoError(t, err) + assertNoItemCrossesPage(t, regen) + + locs := pagesContainingHeaderSlot(regen, "creds", typeBlobData, 0) + require.Len(t, locs, 1, "chunk 0 should land on exactly one page") + var chunk0Page int + for p := range locs { + chunk0Page = p + } + locs1 := pagesContainingHeaderSlot(regen, "creds", typeBlobData, 1) + require.Len(t, locs1, 1, "chunk 1 should land on exactly one page") + var chunk1Page int + for p := range locs1 { + chunk1Page = p + } + assert.NotEqual(t, chunk0Page, chunk1Page, "chunk 0 and chunk 1 should not fit on the same page and must land on different pages") + + reparsed, err := ParseNVS(regen) + require.NoError(t, err) + require.Len(t, reparsed, 3) + + byChunk := make(map[uint8]Entry) + for _, e := range reparsed { + require.True(t, e.Raw) + require.Equal(t, "creds", e.Key) + byChunk[e.ChunkIndex] = e + } + require.Contains(t, byChunk, uint8(singleChunkIndex)) + require.Contains(t, byChunk, uint8(0)) + require.Contains(t, byChunk, uint8(1)) + assert.Equal(t, entries[1].Data, byChunk[0].Data) + assert.Equal(t, entries[2].Data, byChunk[1].Data) +} + +// 4b. An item that does not fit in the current page's remaining tail slots +// must be placed WHOLE on a fresh page — never split — leaving the tail of +// the previous page Empty. Fill page 0 with a namespace declaration plus 123 +// single-slot u8 entries (124 slots used, 2 free), then add a 5-slot string +// entry that can't fit in those 2 remaining slots. +func TestGenerateNVSBumpsOversizedTailItemToNextPage(t *testing.T) { + var entries []Entry + for i := 0; i < 123; i++ { + entries = append(entries, Entry{ + Namespace: "cfg", Key: fmt.Sprintf("k%d", i), Type: "u8", Value: uint8(i % 256), + }) + } + // strLen=100 -> ceil(101/32)=4 data slots + 1 header = 5 slots; page 0 has + // only 2 slots left (126 - 1 ns - 123 u8 = 2), so this must move whole to + // page 1. + longStr := make([]byte, 100) + for i := range longStr { + longStr[i] = byte('a' + i%26) + } + entries = append(entries, Entry{Namespace: "cfg", Key: "biggie", Type: "string", Value: string(longStr)}) + + partition, err := GenerateNVS(entries, PageSize*2) + require.NoError(t, err) + assertNoItemCrossesPage(t, partition) + + // Page 0's tail (slots 124, 125) must be Empty, not holding part of + // "biggie". + page0 := partition[0:PageSize] + for _, slotIdx := range []int{124, 125} { + bitIndex := uint(slotIdx) * 2 + byteIdx := HeaderSize + int(bitIndex/8) + bitOffset := bitIndex % 8 + state := (page0[byteIdx] >> bitOffset) & 0x3 + assert.Equal(t, uint8(entryStateEmpty), state, "page 0 slot %d should be left Empty", slotIdx) + } + + // "biggie" must be found whole on page 1. + locs := pagesContainingHeaderSlot(partition, "biggie", typeString, singleChunkIndex) + require.Len(t, locs, 1) + for p := range locs { + assert.Equal(t, 1, p, "biggie should have been bumped whole onto page 1") + } + + // Everything parses back, including every u8 filler key and the moved + // string — nothing on page 1 is skipped or lost. + parsed, err := ParseNVS(partition) + require.NoError(t, err) + require.Len(t, parsed, 124) // 123 u8 fillers + biggie + + parsedMap := make(map[string]Entry) + for _, e := range parsed { + parsedMap[e.Key] = e + } + require.Contains(t, parsedMap, "biggie") + assert.Equal(t, string(longStr), parsedMap["biggie"].Value) + for i := 0; i < 123; i++ { + key := fmt.Sprintf("k%d", i) + require.Contains(t, parsedMap, key) + assert.Equal(t, uint8(i%256), parsedMap[key].Value) + } +} + +// 4c. Page sequence-number reordering: ParseNVS must scan pages in ascending +// header-seqNum order, not physical/flash order (see the sort.SliceStable in +// ParseNVS). Build a 2-page partition where the SAME (namespace, key) is +// written on both pages with different values, and vary which physical page +// carries the higher sequence number. Write chronology (seqNum), never +// physical placement, must decide which value survives dedup. +func TestParseNVSHigherSeqWinsWhenPhysicalOrderInverted(t *testing.T) { + buildPartition := func(page0Seq, page1Seq uint32, page0Val, page1Val uint8) []byte { + partition := make([]byte, PageSize*2) + + page0 := newTestPage(page0Seq) + placeEntry(page0, 0, nsEntry("cfg", 1)) + placeEntry(page0, 1, u8Entry(1, "value", page0Val)) + copy(partition[0:PageSize], page0) + + page1 := newTestPage(page1Seq) + placeEntry(page1, 0, u8Entry(1, "value", page1Val)) + copy(partition[PageSize:2*PageSize], page1) + + return partition + } + + t.Run("physical order inverted relative to seq order", func(t *testing.T) { + // Page 0 (lower physical offset) has the HIGHER seq number, so + // physical order [page0, page1] is the reverse of seq order + // [page1, page0]. If ParseNVS scanned physical order instead of + // seq order, or the sort comparator got inverted, page1's value + // (the older write) would incorrectly win. + partition := buildPartition(5, 2, 99, 11) + + entries, err := ParseNVS(partition) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, "cfg", entries[0].Namespace) + assert.Equal(t, "value", entries[0].Key) + assert.Equal(t, uint8(99), entries[0].Value, "higher-seq page (page 0) should win despite lower physical offset") + }) + + t.Run("physical order matches seq order", func(t *testing.T) { + // Page 1 (higher physical offset) now holds the higher seq + // number, matching physical order. The higher-seq value must + // still win, proving the outcome is keyed on seqNum and not on + // physical page index in either direction. + partition := buildPartition(2, 5, 11, 99) + + entries, err := ParseNVS(partition) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, "cfg", entries[0].Namespace) + assert.Equal(t, "value", entries[0].Key) + assert.Equal(t, uint8(99), entries[0].Value, "higher-seq page (page 1) should win") + }) +} + +// 5. GenerateNVS must produce identical bytes across repeated calls with the +// same input; Go map iteration order previously made this flaky/nondeterministic. +func TestGenerateNVSDeterministic(t *testing.T) { + entries := []Entry{ + {Namespace: "wifi", Key: "ssid", Type: "string", Value: "Net"}, + {Namespace: "pool", Key: "host", Type: "string", Value: "pool.example.com"}, + {Namespace: "cfg", Key: "flag", Type: "u8", Value: uint8(1)}, + {Namespace: "misc", Key: "x", Type: "u32", Value: uint32(7)}, + } + + first, err := GenerateNVS(entries, DefaultPartSize) + require.NoError(t, err) + assertNoItemCrossesPage(t, first) + + for i := 0; i < 10; i++ { + again, err := GenerateNVS(entries, DefaultPartSize) + require.NoError(t, err) + assert.Equal(t, first, again, "iteration %d differed", i) + } +} + +// 6. Full lossless RMW: parse a realistic multi-namespace image (primitives + +// a chunked blob-index value + a page-contained blob), modify exactly one +// key, regenerate, and reparse. The modified key must reflect the change and +// every other entry must be byte-preserved. Every item in this fixture is +// page-contained, per ESP-IDF's on-flash format. +func TestFullLosslessRoundTripModifyOneKey(t *testing.T) { + const nsCfg, nsWifi = uint8(1), uint8(2) + + var allEntries []*entry + allEntries = append(allEntries, nsEntry("cfg", nsCfg), nsEntry("wifi", nsWifi)) + + flagEnts, err := parseEntry(&Entry{Namespace: "cfg", Key: "flag", Type: "u8", Value: uint8(1)}, nsCfg) + require.NoError(t, err) + allEntries = append(allEntries, flagEnts...) + + nameEnts, err := parseEntry(&Entry{Namespace: "cfg", Key: "name", Type: "string", Value: "device"}, nsCfg) + require.NoError(t, err) + allEntries = append(allEntries, nameEnts...) + + // A blob large enough to require several continuation slots but still + // comfortably page-contained (1 header + 25 data slots = 26 of 126). + bigBlob := make([]byte, 800) + for i := range bigBlob { + bigBlob[i] = byte(i) + } + blobEnts, err := parseEntry(&Entry{Namespace: "wifi", Key: "bigblob", Type: "blob", Value: bigBlob}, nsWifi) + require.NoError(t, err) + allEntries = append(allEntries, blobEnts...) + + idx := newEntry() + idx.namespaceIdx = nsWifi + idx.entryType = 0x48 + idx.span = spanOne + idx.chunkIndex = singleChunkIndex + copyKeyToEntry("creds", idx) + idx.data = [8]byte{16, 0, 2, 0, 0xFF, 0xFF, 0xFF, 0xFF} + idx.crc32Val = calculateEntryCRC32(idx) + allEntries = append(allEntries, idx) + + chunk0 := newEntry() + chunk0.namespaceIdx = nsWifi + chunk0.entryType = 0x42 + chunk0.span = 2 + chunk0.chunkIndex = 0 + copyKeyToEntry("creds", chunk0) + chunk0.data = [8]byte{8, 0, 0xFF, 0xFF, 0, 0, 0, 0} + chunk0.crc32Val = calculateEntryCRC32(chunk0) + chunk0.rawData = []byte("PASSWORD") + allEntries = append(allEntries, chunk0) + + chunk1 := newEntry() + chunk1.namespaceIdx = nsWifi + chunk1.entryType = 0x42 + chunk1.span = 2 + chunk1.chunkIndex = 1 + copyKeyToEntry("creds", chunk1) + chunk1.data = [8]byte{8, 0, 0xFF, 0xFF, 0, 0, 0, 0} + chunk1.crc32Val = calculateEntryCRC32(chunk1) + chunk1.rawData = []byte("SSID1234") + allEntries = append(allEntries, chunk1) + + const totalPages = 6 + partition := make([]byte, PageSize*totalPages) + for i := range partition { + partition[i] = 0xFF + } + _, err = writePage(&partition, 0, 0, allEntries, totalPages) + require.NoError(t, err) + assertNoItemCrossesPage(t, partition) + + parsed, err := ParseNVS(partition) + require.NoError(t, err) + require.Len(t, parsed, 6) // flag, name, bigblob, creds(idx), creds(chunk0), creds(chunk1) + + // Snapshot originals for later comparison. + type key struct { + ns, k string + chunk uint8 + } + originals := make(map[key]Entry) + for _, e := range parsed { + originals[key{e.Namespace, e.Key, e.ChunkIndex}] = e + } + + // Modify exactly one key. + modified := make([]Entry, len(parsed)) + copy(modified, parsed) + found := false + for i := range modified { + if modified[i].Namespace == "cfg" && modified[i].Key == "flag" { + modified[i].Value = uint8(2) + found = true + } + } + require.True(t, found, "did not find cfg:flag to modify") + + regen, err := GenerateNVS(modified, PageSize*totalPages) + require.NoError(t, err) + assertNoItemCrossesPage(t, regen) + reparsed, err := ParseNVS(regen) + require.NoError(t, err) + require.Len(t, reparsed, 6) + + reparsedMap := make(map[key]Entry) + for _, e := range reparsed { + reparsedMap[key{e.Namespace, e.Key, e.ChunkIndex}] = e + } + + // The modified key changed. + flag, ok := reparsedMap[key{"cfg", "flag", singleChunkIndex}] + require.True(t, ok) + assert.Equal(t, uint8(2), flag.Value) + + // Everything else is byte-preserved. + for k, orig := range originals { + if k.ns == "cfg" && k.k == "flag" { + continue + } + got, ok := reparsedMap[k] + require.True(t, ok, "entry %+v missing after round trip", k) + if orig.Raw { + assert.True(t, got.Raw, "%+v: expected Raw", k) + assert.Equal(t, orig.TypeByte, got.TypeByte, "%+v: TypeByte", k) + assert.Equal(t, orig.Span, got.Span, "%+v: Span", k) + assert.Equal(t, orig.ChunkIndex, got.ChunkIndex, "%+v: ChunkIndex", k) + assert.Equal(t, orig.Data, got.Data, "%+v: Data", k) + } else { + assert.Equal(t, orig.Type, got.Type, "%+v: Type", k) + assert.Equal(t, orig.Value, got.Value, "%+v: Value", k) + } + } +} + +// 7. Real-world regression fixture: testdata/real_espidf_nvs.bin is a 24576 +// byte (6 page) NVS v2 partition produced by ESP-IDF's own +// nvs_partition_gen.py (not hand-built by this package), containing three +// namespaces — wifi_cfg (string ssid/pass, u8 channel/provisioned), app_cfg +// (string hostname, u32 counter), and blob_ns holding a single chunked blob +// ("bigblob") too large for one item: a BLOB_IDX header entry plus two +// BLOB_DATA chunks (chunk 0: 3616 payload bytes, chunk 1: 2528 payload bytes, +// 6144 bytes total). This locks the codec against a real esp-idf image +// instead of only synthetic fixtures. +func TestParseNVSRealESPIDFBlobPartitionRoundTrips(t *testing.T) { + data, err := os.ReadFile("testdata/real_espidf_nvs.bin") + require.NoError(t, err) + require.Len(t, data, DefaultPartSize) + + entries, err := ParseNVS(data) + require.NoError(t, err) + require.Len(t, entries, 9) + + byNSKey := make(map[string]Entry) + rawByChunk := make(map[uint8]Entry) + for _, e := range entries { + if e.Namespace == "blob_ns" && e.Key == "bigblob" { + require.True(t, e.Raw, "blob_ns:bigblob chunk %d should be captured via raw passthrough", e.ChunkIndex) + rawByChunk[e.ChunkIndex] = e + continue + } + byNSKey[e.Namespace+"/"+e.Key] = e + } + + assert.Equal(t, "MyHomeNetwork", byNSKey["wifi_cfg/ssid"].Value) + assert.Equal(t, "SuperSecretPassphrase123", byNSKey["wifi_cfg/pass"].Value) + assert.Equal(t, uint8(6), byNSKey["wifi_cfg/channel"].Value) + assert.Equal(t, uint8(1), byNSKey["wifi_cfg/provisioned"].Value) + assert.Equal(t, "esp32-device-01", byNSKey["app_cfg/hostname"].Value) + assert.Equal(t, uint32(424242), byNSKey["app_cfg/counter"].Value) + + // BLOB_IDX header (chunkIndex singleChunkIndex) plus two BLOB_DATA chunks. + require.Contains(t, rawByChunk, uint8(singleChunkIndex)) + require.Contains(t, rawByChunk, uint8(0)) + require.Contains(t, rawByChunk, uint8(1)) + assert.Equal(t, uint8(0x48), rawByChunk[singleChunkIndex].TypeByte) + assert.Equal(t, uint8(0x42), rawByChunk[0].TypeByte) + assert.Equal(t, uint8(0x42), rawByChunk[1].TypeByte) + + // Each chunk's Data is an 8-byte item header followed by its verbatim + // continuation-slot payload; the blob's total content length is the sum + // of the two chunks' payloads (Data minus the 8-byte header each). + chunk0Payload := len(rawByChunk[0].Data) - 8 + chunk1Payload := len(rawByChunk[1].Data) - 8 + assert.Equal(t, 3616, chunk0Payload) + assert.Equal(t, 2528, chunk1Payload) + assert.Equal(t, 6144, chunk0Payload+chunk1Payload) + + // Round trip: regenerate the parsed model and reparse; every namespace, + // key, value, and raw blob chunk must come back byte-identical. + regen, err := GenerateNVS(entries, DefaultPartSize) + require.NoError(t, err) + assertNoItemCrossesPage(t, regen) + + reparsed, err := ParseNVS(regen) + require.NoError(t, err) + require.Len(t, reparsed, 9) + + type key struct { + ns, k string + chunk uint8 + } + origByKey := make(map[key]Entry) + for _, e := range entries { + origByKey[key{e.Namespace, e.Key, e.ChunkIndex}] = e + } + for _, got := range reparsed { + orig, ok := origByKey[key{got.Namespace, got.Key, got.ChunkIndex}] + require.True(t, ok, "entry %s/%s chunk %d missing from original parse", got.Namespace, got.Key, got.ChunkIndex) + if orig.Raw { + assert.True(t, got.Raw, "%s/%s: expected Raw", got.Namespace, got.Key) + assert.Equal(t, orig.TypeByte, got.TypeByte, "%s/%s: TypeByte", got.Namespace, got.Key) + assert.Equal(t, orig.Span, got.Span, "%s/%s: Span", got.Namespace, got.Key) + assert.Equal(t, orig.Data, got.Data, "%s/%s: Data", got.Namespace, got.Key) + } else { + assert.Equal(t, orig.Type, got.Type, "%s/%s: Type", got.Namespace, got.Key) + assert.Equal(t, orig.Value, got.Value, "%s/%s: Value", got.Namespace, got.Key) + } + } +} diff --git a/pkg/nvs/nvs.go b/pkg/nvs/nvs.go index d65da48..2cda4ca 100644 --- a/pkg/nvs/nvs.go +++ b/pkg/nvs/nvs.go @@ -38,6 +38,21 @@ const ( type Entry struct { Namespace string Key string - Type string // "u8", "u16", "u32", "i8", "i16", "i32", "string", "blob" + Type string // "u8", "u16", "u32", "i8", "i16", "i32", "string", "blob", or "raw" Value interface{} + + // Raw entries are captured via generic passthrough when ParseNVS encounters + // a type byte it does not natively decode (future/vendor NVS types, or the + // blob-index/blob-data entries ESP-IDF uses for chunked values such as + // esp_wifi credentials). When Raw is true, GenerateNVS ignores Type/Value + // and re-emits the slot(s) byte-for-byte from TypeByte/Span/ChunkIndex/Data + // so a read-modify-write round trip never drops or corrupts data it does + // not understand. + Raw bool + TypeByte uint8 // raw NVS entry-type byte (entryBytes[1]) + Span uint8 // number of 32-byte slots this entry occupies, header included + ChunkIndex uint8 // NVS chunk index; 0xFF (singleChunkIndex) means "not chunked" + Data []byte // raw payload: first 8 bytes are the entry header's "data" field + // (entryBytes[24:32]); any remaining bytes are the verbatim continuation + // slot(s) content, length exactly (Span-1)*EntrySize. } diff --git a/pkg/nvs/parse.go b/pkg/nvs/parse.go index 5f4f632..72da4c7 100644 --- a/pkg/nvs/parse.go +++ b/pkg/nvs/parse.go @@ -3,44 +3,71 @@ package nvs import ( "encoding/binary" "fmt" + "sort" ) +// rawSlotEntry is the structural (type-agnostic) decode of one NVS entry +// record: namespace/key/type/span/chunkIndex plus the 8-byte header "data" +// field. It does not yet know the namespace *name* (only its index) because +// the namespace declaration for that index may live on a page scanned later +// than the key that references it. +type rawSlotEntry struct { + pageNum int + slotIdx int + namespaceIdx uint8 + entryType uint8 + span uint8 + chunkIndex uint8 + key string + headerData []byte // 8 bytes, entryBytes[24:32] +} + +// pageRecord is a validated page ready for the structural scan, tagged with +// its physical index and the sequence number from its header. +type pageRecord struct { + pageNum int + seqNum uint32 + page []byte +} + // ParseNVS reads an NVS partition binary and returns a flat slice of entries. // It handles: -// - Page validation (state, version, header CRC) -// - Entry bitmap decoding -// - Namespace mapping -// - Multi-span entries (strings and blobs) -// - Value decoding based on type -// - Deduplication (last write wins) +// - Page validation (state, version, header CRC) +// - Entry bitmap decoding +// - Namespace mapping, independent of scan order (a key may be declared +// before its namespace's declaration entry, e.g. on a page scanned later) +// - Multi-slot entries (strings and blobs); per ESP-IDF's on-flash format an +// item's header + all continuation slots always live on a single page, so +// each is read entirely within its own page +// - Value decoding based on type, with lossless generic passthrough for any +// type this codec does not natively model (see Entry.Raw) +// - Deduplication (last write wins), keyed by namespace+key+chunkIndex so +// chunked entries (e.g. esp_wifi blob-index credentials) that legitimately +// share a key are not collapsed into one. Pages are scanned in ascending +// order of their header sequence number (not physical/flash order, which +// ESP-IDF explicitly does not guarantee reflects write chronology), so +// later-seq writes naturally overwrite earlier ones for the same key. func ParseNVS(data []byte) ([]Entry, error) { // Validate data length is a multiple of PageSize if len(data)%PageSize != 0 { return nil, fmt.Errorf("data length (%d) must be a multiple of PageSize (%d)", len(data), PageSize) } - // Map from namespace index to namespace name - namespaceMap := make(map[uint8]string) - - // Map from "namespace:key" to entry, for deduplication (last write wins) - entryMap := make(map[string]*Entry) - - // Walk pages totalPages := len(data) / PageSize + + // Pass 1: validate each page and collect the ones worth scanning, + // tagged with their header sequence number. + var pages []pageRecord for pageNum := 0; pageNum < totalPages; pageNum++ { pageOffset := pageNum * PageSize page := data[pageOffset : pageOffset+PageSize] - // Check page state state := page[0] if state == pageStateEmpty { - // Skip empty pages continue } - // Validate page version at byte 8 if page[8] != pageVersion { - // Skip pages with wrong version continue } @@ -51,12 +78,42 @@ func ParseNVS(data []byte) ([]Entry, error) { return nil, fmt.Errorf("page %d header CRC mismatch: expected 0x%x, got 0x%x", pageNum, expectedCRC, actualCRC) } - // Read entry bitmap at HeaderSize (bytes 32-63) - // Walk through bitmap, looking for entries with state = entryStateWritten (0b10) - processedSlots := make(map[int]bool) // Track multi-span entries + seqNum := binary.LittleEndian.Uint32(page[4:8]) + pages = append(pages, pageRecord{pageNum: pageNum, seqNum: seqNum, page: page}) + } + + // Scan pages in ascending sequence-number order, not physical order: + // ESP-IDF decouples page write chronology from physical placement, so the + // sequence number in the page header is the only reliable ordering for + // "last write wins" semantics. Ties (which shouldn't occur in a valid + // partition) fall back to physical order for determinism. + sort.SliceStable(pages, func(i, j int) bool { + if pages[i].seqNum != pages[j].seqNum { + return pages[i].seqNum < pages[j].seqNum + } + return pages[i].pageNum < pages[j].pageNum + }) + + // Map from namespace index to namespace name. Populated as namespace + // declarations are encountered during the structural walk below; only + // consulted after that walk completes, so scan order never causes a key + // to be dropped for "namespace not yet defined". + namespaceMap := make(map[uint8]string) + + // Structural walk: decode every written entry record on every valid + // page, in sequence-number order. Namespace declarations are resolved + // immediately (they're never namespaced themselves); everything else is + // queued for phase 2. + var rawEntries []rawSlotEntry + + for _, pr := range pages { + page := pr.page + pageNum := pr.pageNum + + processedSlots := make(map[int]bool) // Track multi-slot entries for slotIdx := 0; slotIdx < EntriesPerPage; slotIdx++ { - // Skip if already processed as part of multi-span + // Skip if already processed as part of a multi-slot entry if processedSlots[slotIdx] { continue } @@ -82,85 +139,159 @@ func ParseNVS(data []byte) ([]Entry, error) { namespaceIdx := entryBytes[0] entryType := entryBytes[1] span := entryBytes[2] - // chunkIndex := entryBytes[3] // Not used for parsing + if span == 0 { + span = 1 + } + chunkIndex := entryBytes[3] // crc32Val := binary.LittleEndian.Uint32(entryBytes[4:8]) // TODO: validate CRC key := readNullTerminatedString(entryBytes[8:24]) - dataBytes := entryBytes[24:32] - - // Mark all slots used by this entry as processed - for s := 0; s < int(span); s++ { - processedSlots[slotIdx+s] = true + headerData := append([]byte(nil), entryBytes[24:32]...) + + // Mark slots used by this entry's continuation as processed. + // ESP-IDF never splits an item across pages: an item's header + + // continuation slots are always fully contained within one page + // (see Page::writeItem in nvs_page.cpp). If a span byte claims + // more continuation slots than remain on this page, that layout + // cannot come from a real ESP-IDF partition; treat it as + // corrupt and clamp to what's actually present on this page + // rather than reaching into the next page's slots. + contNeeded := int(span) - 1 + s := slotIdx + 1 + for contNeeded > 0 && s < EntriesPerPage { + processedSlots[s] = true + s++ + contNeeded-- + } + if contNeeded > 0 { + span -= uint8(contNeeded) } // Handle namespace entries (namespaceIdx == 0) if namespaceIdx == 0 && entryType == namespaceType { - nsIndex := dataBytes[0] + nsIndex := headerData[0] namespaceMap[nsIndex] = key continue } - // Look up namespace name - namespaceName, ok := namespaceMap[namespaceIdx] - if !ok { - // Namespace not yet defined, skip for now - continue - } + rawEntries = append(rawEntries, rawSlotEntry{ + pageNum: pageNum, + slotIdx: slotIdx, + namespaceIdx: namespaceIdx, + entryType: entryType, + span: span, + chunkIndex: chunkIndex, + key: key, + headerData: headerData, + }) + } + } + + // Phase 2: resolve namespace names (now fully known, regardless of the + // scan order in which declarations vs. keys were encountered) and decode + // each entry's value, deduplicating on (namespace, key, chunkIndex) so + // last-write-wins semantics hold without collapsing distinct chunks of a + // chunked value that happen to share a key. rawEntries is already in + // ascending page-sequence order, so a later overwrite of an earlier map + // entry here means "higher sequence number wins". + type dedupKey struct { + namespace string + key string + chunkIndex uint8 + } + entryMap := make(map[dedupKey]*Entry) + var order []dedupKey // first-seen order, for stable output + + for _, re := range rawEntries { + namespaceName, ok := namespaceMap[re.namespaceIdx] + if !ok { + // Orphaned key: no namespace declaration found anywhere in the + // partition for this index. Nothing to resolve it to; drop it. + continue + } - // Decode value based on type - var value interface{} - var err error + var value interface{} + var err error + raw := false - switch entryType { - case typeU8: - value = dataBytes[0] + pageOffset := re.pageNum * PageSize + page := data[pageOffset : pageOffset+PageSize] - case typeU16: - value = binary.LittleEndian.Uint16(dataBytes[0:2]) + switch re.entryType { + case typeU8: + value = re.headerData[0] - case typeU32: - value = binary.LittleEndian.Uint32(dataBytes[0:4]) + case typeU16: + value = binary.LittleEndian.Uint16(re.headerData[0:2]) - case typeI8: - value = int8(dataBytes[0]) + case typeU32: + value = binary.LittleEndian.Uint32(re.headerData[0:4]) - case typeI16: - value = int16(binary.LittleEndian.Uint16(dataBytes[0:2])) + case typeI8: + value = int8(re.headerData[0]) - case typeI32: - value = int32(binary.LittleEndian.Uint32(dataBytes[0:4])) + case typeI16: + value = int16(binary.LittleEndian.Uint16(re.headerData[0:2])) - case typeString: - value, err = readStringEntry(page, pageNum, slotIdx, dataBytes, span) - if err != nil { - return nil, err - } + case typeI32: + value = int32(binary.LittleEndian.Uint32(re.headerData[0:4])) - case typeBlob: - value, err = readBlobEntry(page, pageNum, slotIdx, dataBytes, span) - if err != nil { - return nil, err - } + case typeString: + value, err = readStringEntry(page, re.slotIdx, re.headerData, re.span) + if err != nil { + return nil, err + } - default: - // Skip unknown types - continue + case typeBlob: + value, err = readBlobEntry(page, re.slotIdx, re.headerData, re.span) + if err != nil { + return nil, err } - // Create entry and store in map (deduplication: last write wins) - mapKey := fmt.Sprintf("%s:%s", namespaceName, key) - entryMap[mapKey] = &Entry{ - Namespace: namespaceName, - Key: key, - Type: typeToString(entryType), - Value: value, + default: + // Unknown/unmodeled type: capture generically instead of + // dropping it, so a read-modify-write round trip is lossless + // even for entries this codec doesn't semantically understand + // (e.g. ESP-IDF blob-index/blob-data chunk entries). + raw = true + } + + dk := dedupKey{namespace: namespaceName, key: re.key, chunkIndex: re.chunkIndex} + if _, exists := entryMap[dk]; !exists { + order = append(order, dk) + } + + if raw { + spanData := readSpanData(page, re.slotIdx, re.span) + rawData := make([]byte, 0, len(re.headerData)+len(spanData)) + rawData = append(rawData, re.headerData...) + rawData = append(rawData, spanData...) + entryMap[dk] = &Entry{ + Namespace: namespaceName, + Key: re.key, + Type: "raw", + Raw: true, + TypeByte: re.entryType, + Span: re.span, + ChunkIndex: re.chunkIndex, + Data: rawData, } + continue + } + + entryMap[dk] = &Entry{ + Namespace: namespaceName, + Key: re.key, + Type: typeToString(re.entryType), + Value: value, + ChunkIndex: re.chunkIndex, } } - // Convert map to flat slice - var result []Entry - for _, e := range entryMap { - result = append(result, *e) + // Convert map to flat slice in first-seen order (values reflect the last + // write per the deduplication above). + result := make([]Entry, 0, len(order)) + for _, dk := range order { + result = append(result, *entryMap[dk]) } return result, nil @@ -176,33 +307,41 @@ func readNullTerminatedString(b []byte) string { return string(b) } -// readStringEntry reads a string entry from data and subsequent span entries -func readStringEntry(page []byte, pageNum int, slotIdx int, headerData []byte, span uint8) (string, error) { - strLen := binary.LittleEndian.Uint16(headerData[0:2]) - if strLen == 0 { - return "", nil +// readSpanData reads the raw continuation payload for an entry that occupies +// `span` 32-byte slots (header slot included), starting at slotIdx, entirely +// within `page`. ESP-IDF never splits an item's entries across pages (see +// Page::writeItem in nvs_page.cpp): if an item doesn't fit in the remaining +// slots of the current page, the whole item — header and all continuation +// slots together — is written on a fresh page instead. So a span is always +// read within a single page; it never reaches into a neighboring page. +func readSpanData(page []byte, slotIdx int, span uint8) []byte { + remaining := int(span) - 1 + if remaining <= 0 { + return nil } - // Read data from subsequent span slots var buf []byte - currentSlot := slotIdx + 1 - - for i := 0; i < int(span)-1; i++ { - if currentSlot >= EntriesPerPage { - // Would need to read from next page, but for simplicity assume fits in current page - // In a full implementation, would handle page boundaries - break - } - - dataOffset := FirstEntryOffset + currentSlot*EntrySize + slot := slotIdx + 1 + for i := 0; i < remaining && slot < EntriesPerPage; i, slot = i+1, slot+1 { + dataOffset := FirstEntryOffset + slot*EntrySize if dataOffset+EntrySize > len(page) { break } - buf = append(buf, page[dataOffset:dataOffset+EntrySize]...) - currentSlot++ } + return buf +} + +// readStringEntry reads a string entry's value from within a single page. +func readStringEntry(page []byte, slotIdx int, headerData []byte, span uint8) (string, error) { + strLen := binary.LittleEndian.Uint16(headerData[0:2]) + if strLen == 0 { + return "", nil + } + + buf := readSpanData(page, slotIdx, span) + // Trim to actual length and remove null terminator if int(strLen) > len(buf) { strLen = uint16(len(buf)) @@ -214,32 +353,14 @@ func readStringEntry(page []byte, pageNum int, slotIdx int, headerData []byte, s return string(result), nil } -// readBlobEntry reads a blob entry from data and subsequent span entries -func readBlobEntry(page []byte, pageNum int, slotIdx int, headerData []byte, span uint8) ([]byte, error) { +// readBlobEntry reads a blob entry's value from within a single page. +func readBlobEntry(page []byte, slotIdx int, headerData []byte, span uint8) ([]byte, error) { blobLen := binary.LittleEndian.Uint16(headerData[0:2]) if blobLen == 0 { return []byte{}, nil } - // Read data from subsequent span slots - var buf []byte - currentSlot := slotIdx + 1 - - for i := 0; i < int(span)-1; i++ { - if currentSlot >= EntriesPerPage { - // Would need to read from next page, but for simplicity assume fits in current page - // In a full implementation, would handle page boundaries - break - } - - dataOffset := FirstEntryOffset + currentSlot*EntrySize - if dataOffset+EntrySize > len(page) { - break - } - - buf = append(buf, page[dataOffset:dataOffset+EntrySize]...) - currentSlot++ - } + buf := readSpanData(page, slotIdx, span) // Trim to actual length (no null terminator for blobs) if int(blobLen) > len(buf) { diff --git a/pkg/nvs/testdata/real_espidf_nvs.bin b/pkg/nvs/testdata/real_espidf_nvs.bin new file mode 100644 index 0000000000000000000000000000000000000000..7291494902e17feb59ab5b8e2e2101dc104e2b71 GIT binary patch literal 24576 zcmeI%_gB+U6bJC6)hZ6eja%HPRX|bPimQSmihI?z1R8}VBn8BU0~bzEae#Y*g5W~I zz4yR{3&A~5+!KkZrIk}X{sH=ZKeRpVdoTBW+9vPbDqT52#j*vLM@aI=OT_r}Cz5Qm z46oOz{3+7`Y{UR!HabUBJlo7k?o9>J8cOcqXbQ``Oi3Y9Ae|HKHB6vRPjRNdG8RJ5&Bxk}Y))oawWt5v&B-Fo#K*f(s{Sl*>xkXEtR;}B# zZP&g-$4;)DyL9c=UD3nMy=O0v-kyE>diCo+VBny^Lxv6;K0-NCrPg>;KE7Id)M!6{ z9m582ygo1}cudIHapNaU44pK2%G9uF(`U?_6+U~;+mSF zT)cES?aI|_*KefXymkA|-HdygS@$12eDwIq(`V0LWWRis^ZL!(cke%Z{Pg+D*KfJs z^8~CP-#>o;`2XYlhx0$Kf4Kf*|AYM>_P^NwBmXhxe}=rD6tl)Z<^Ssxt6ct+{uk^k zb@e0z(uSv6=AW5Xf*?<8pNjkv`8DbbsIQ>Dg!&rli>R-nzWi_1*QE+lNDijumis@# zmLMd*-R&^n_P`4WKmY;|fB*y_009U<00Izz00bZa0SG_<0uX=z1Rwwb2tWV=5P$## zAOHafKmY;|fB*y_009U<00Izz00bZa0SG_<0uX=z1Rwwb2tWV=5P$##AOHafKmY>& Gy}%D$n|Hnd literal 0 HcmV?d00001