ESP-IDF-SVC NVS contains() Bug Report - String Values Not Detected

[danielmeza]

ESP-IDF-SVC NVS contains() Bug Report - String Values Not Detected

Bug description

The contains() method in EspNvs returns false for string values that actually exist in NVS partitions, even though the same strings can be successfully retrieved using get_str(). This creates inconsistent behavior where data exists in flash but appears missing to the application.

Root Cause: The contains() method internally calls len(), which only checks for u64 and blob data types via nvs_get_u64() and nvs_get_blob(). It never calls nvs_get_str() to check for string values stored with nvs_set_str().

Would you like to work on a fix? Yes, we have identified the exact issue and can provide a complete solution.

To Reproduce

1. Create an NVS partition in your ESP32 project (default or custom)
2. Initialize an EspNvs or EspCustomNvs instance with a namespace
3. Store a string value using set_str(key, "some_string")
4. Call contains(key) - it returns false
5. Call get_str(key, buffer) - it successfully retrieves the string
6. Reboot the device and repeat steps 4-5 - same inconsistent behavior

Minimal reproduction code:

use esp_idf_svc::nvs::{EspDefaultNvsPartition, EspNvs};
// or for custom partitions: use esp_idf_svc::nvs::{EspCustomNvsPartition, EspCustomNvs};

fn reproduce_bug() -> Result<(), Box<dyn std::error::Error>> {
    // Using default partition (bug affects all partition types)
    let partition = EspDefaultNvsPartition::take()?;
    let mut nvs = EspNvs::new(partition, "test_ns", true)?;
    
    // Alternative: Custom partition
    // let partition = EspCustomNvsPartition::take("custom_partition")?;
    // let mut nvs = EspCustomNvs::new(partition, "test_ns", true)?;
    
    // Set a string value
    nvs.set_str("test_key", "test_value")?;
    
    // BUG: contains() returns false for string values
    let contains_result = nvs.contains("test_key")?;
    println!("contains(): {}", contains_result); // Prints: false
    
    // But get_str() works correctly
    let mut buffer = [0u8; 64];
    let get_result = nvs.get_str("test_key", &mut buffer)?;
    println!("get_str(): {:?}", get_result); // Prints: Some("test_value")
    
    // Inconsistent behavior: data exists but contains() can't find it
    assert!(contains_result != get_result.is_some()); // This assertion passes, proving the bug
    
    Ok(())
}

Expected behavior

contains(key) should return true for any existing NVS entry regardless of data type (u8, u16, u32, u64, i8, i16, i32, i64, string, blob). The method should be consistent with other NVS operations like get_str(), get_u32(), etc.

Expected behavior:

• nvs.set_str("key", "value") - stores string successfully ✅
• nvs.contains("key") - returns true ✅ (currently returns false ❌)
• nvs.get_str("key", buffer) - retrieves string successfully ✅

Root Cause Analysis

The bug is in esp-idf-svc/src/nvs.rs:

pub fn contains(&self, name: &str) -> Result<bool, EspError> {
    self.len(name).map(|v| v.is_some())  // ← calls len()
}

fn len(&self, name: &str) -> Result<Option<usize>, EspError> {
    let c_key = to_cstring_arg(name)?;
    let mut value: u_int64_t = 0;

    // check for u64 value
    match unsafe { nvs_get_u64(self.1, c_key.as_ptr(), &mut value as *mut _) } {
        ESP_ERR_NVS_NOT_FOUND => {
            // check for blob value, by getting blob length
            let mut len = 0;
            match unsafe {
                nvs_get_blob(self.1, c_key.as_ptr(), ptr::null_mut(), &mut len as *mut _)
            } {
                ESP_ERR_NVS_NOT_FOUND => Ok(None),  // ← STOPS HERE, never checks strings!
                // ... only handles blob case
            }
        }
        // ... only handles u64 case
    }
}

Problem: The len() method only checks nvs_get_u64() and nvs_get_blob(). It never calls nvs_get_str() for string values.

Proposed Solution

Option 1: Fix the len() method to check string values

fn len(&self, name: &str) -> Result<Option<usize>, EspError> {
    let c_key = to_cstring_arg(name)?;
    
    // Check for u64 value
    let mut u64_value: u_int64_t = 0;
    match unsafe { nvs_get_u64(self.1, c_key.as_ptr(), &mut u64_value as *mut _) } {
        ESP_ERR_NVS_NOT_FOUND => {
            // Check for blob value
            let mut blob_len = 0;
            match unsafe { nvs_get_blob(self.1, c_key.as_ptr(), ptr::null_mut(), &mut blob_len as *mut _) } {
                ESP_ERR_NVS_NOT_FOUND => {
                    // ← ADD THIS: Check for string value
                    let mut str_len = 0;
                    match unsafe { nvs_get_str(self.1, c_key.as_ptr(), ptr::null_mut(), &mut str_len as *mut _) } {
                        ESP_ERR_NVS_NOT_FOUND => Ok(None),
                        err => {
                            esp!(err)?;
                            Ok(Some(str_len))
                        }
                    }
                }
                err => {
                    esp!(err)?;
                    Ok(Some(blob_len))
                }
            }
        }
        err => {
            esp!(err)?;
            Ok(Some(8)) // u64 is always 8 bytes
        }
    }
}

Option 2: Use nvs_find_key() in contains() method (Recommended)

pub fn contains(&self, name: &str) -> Result<bool, EspError> {
    let c_key = to_cstring_arg(name)?;
    let mut entry_type: nvs_type_t = nvs_type_t_NVS_TYPE_ANY;
    
    let result = unsafe { 
        nvs_find_key(self.1, c_key.as_ptr(), &mut entry_type as *mut _) 
    };
    
    match result {
        ESP_OK => Ok(true),
        ESP_ERR_NVS_NOT_FOUND => Ok(false),
        err => {
            esp!(err)?;
            Ok(false) // This line should never be reached due to esp!() macro
        }
    }
}

Option 3: Enhanced find_key() method that returns data type (Additional Feature)

The nvs_find_key function can also return the data type of the found entry. Consider adding this as a new method:

pub fn find_key(&self, name: &str) -> Result<Option<nvs_type_t>, EspError> {
    let c_key = to_cstring_arg(name)?;
    let mut entry_type: nvs_type_t = nvs_type_t_NVS_TYPE_ANY;
    
    let result = unsafe { 
        nvs_find_key(self.1, c_key.as_ptr(), &mut entry_type as *mut _) 
    };
    
    match result {
        ESP_OK => Ok(Some(entry_type)),
        ESP_ERR_NVS_NOT_FOUND => Ok(None),
        err => {
            esp!(err)?;
            Ok(None) // This line should never be reached due to esp!() macro
        }
    }
}

This would allow applications to:

// Check existence and get type in one call
if let Some(data_type) = nvs.find_key("my_key")? {
    match data_type {
        nvs_type_t_NVS_TYPE_STR => { /* handle string */ },
        nvs_type_t_NVS_TYPE_U32 => { /* handle u32 */ },
        nvs_type_t_NVS_TYPE_BLOB => { /* handle blob */ },
        _ => { /* handle other types */ },
    }
}

Options 2 and 3 are recommended because:

• nvs_find_key() is designed specifically to check for key existence regardless of data type
• It's more efficient than trying multiple nvs_get_*() calls
• It's more robust and handles all current and future NVS data types
• It matches the semantic expectation of a contains() method
• Uses the exact pattern from ESP-IDF commit 8740888 that introduced nvs_find_key() on Dec 8, 2023
• Option 3 additionally provides data type information, which can be useful for applications

Note: The nvs_find_key function was added to ESP-IDF specifically for this use case - checking key existence regardless of data type. The implementation above follows the exact error handling pattern used in the official ESP-IDF test suite.

Environment

• Crate (esp-idf-svc) version: 0.51.0
• ESP-IDF branch or tag: release/v5.2
• Target device (MCU): esp32
• OS: Windows 11

Impact

This bug affects anyone using string values in NVS partitions with esp-idf-svc, making it a critical issue for production applications. It causes:

1. Logic errors where applications think data doesn't exist when it actually does
2. Unnecessary re-initialization of configuration values
3. Data inconsistency between different NVS operation results
4. Debugging confusion where data appears in partition dumps but contains() returns false

Additional Notes

• The bug affects all NVS partitions (default and custom) and string values
• get_str(), set_str(), and other string operations work correctly
• Only the contains() method is affected
• Binary partition dumps show the string entries exist in flash
• This suggests the bug is in the esp-idf-svc wrapper, not the underlying ESP-IDF NVS implementation

[ivmarkov]

The main problem with the current EspNvs implementation is that it is actually a two-implementations-in-one-struct thing:

The first implementation is a one that always serializes/deserializes everything as an ESP-IDF NVS BLOB. It also features an optimization where if the BLOB value is <= 7 bytes, then the value is serialized/deserialized as an ESP-IDF NVS u64 type

• Pros:
  • Can serde any value, as long that value is representable as a &[u8] slice. More often than not, requires to use a Rust serde impl on top of it (say, postcard or json or something)
  • Implements the key-value "storage" trait from embedded-svc
• Cons:
  • Results in a more verbose representation of anything bigger than 7 bytes

===

The second implementation is just exposing the "native" ESP IDF NVS API which can efficiently serialize a bunch of primitive types (bool, i8, u8, &str (up to 4K chars), &[u8] and so on). Pros and cons obvious here, i.e. more efficient (w.r.t. binary representation) of the implementation, but is ESP-IDF specific.

===

UPDATE: Once upon in time, only the first implementation did exist, but folks (including myself) wanted to take advantage of the "native" ESP IDF NVS API (for interop with ESP IDF C code or other reasons) hence why the second implementation appeared.

The confusion comes from the fact that both of these implementations are implemented on top of the same struct - EspNvs, so there is sometimes confusion as to which method belongs to which implementation. The contains method in particular belongs to the first implementation, and the reason it only checks for u64 and BLOB is because the first implementation cannot deal with any other ESP IDF serde format. It is another topic that maybe - if it detects a different key than u64 or BLOB it should return an error (as it cannot deal with anything other than u64 and BLOB anyway) rather than reporting "hey, the value is not there".

I think the best approach might be, if we have a separate contains method (it is another topic how to name it so that it does not bring any more confusion) and which method would be specific to the second implementation. Perhaps contains_key? And then a thorough documentation on the existing contains method that it belongs to implementation 1, while contains_key belongs to implementation 2?

Or even better (but more work) implementing a newtype - say - EspKeyValueStorage on top of EspNvs where we move all the methods of implementation 1. How about that? I think the resulting clarity is worth the backwards compatibility breakage, and given that #583 will do a backward compatibility breakage for the forthcoming release anyway, we can live with this one as well.

[Vollbrecht]

I would have no problem with the introduction of something like a EspKeyValueStorage

[danielmeza]

Hi @ivmarkov thanks for the detailed explanation!
I agree with @Vollbrecht, having a separate EspKeyValueStorage with a Serde specific type constraints where required will be a proper option.
Shall we define a specs issue for that refactoring?

[ivmarkov]

Shall we define a specs issue for that refactoring?

Might be a bit too much for something which is not a rocket science? :)

You just have to pub struct EspKeyValueStorage<T>(EspNvs<T>) and then move all "implementation 1" methods from EspNvs to EspKeyValueStorage, with putting ".0" in the moved method bodies as appropriate.

This would then allow you to define a "real" contains method directly on EspNvs for "implementation 2".

I would just say - open a PR that does it and then we bang on it until it is in a good shape!

[ivmarkov]

@danielmeza BTW I don't understand this:

with a Serde specific type constraints where required

This should not be necessary at all? "Implementation 1" just works in terms of a (&str key, &[u8] value) pairs. No serde around. It is just that all Rust serde impls have methods to serialize into a u8 slice / deserialize from a u8 slice, so such a simple &[u8]-based API is a good fit for Rust serde.

[danielmeza]

@ivmarkov got it, I'll open a PR with the EspKeyValueStorage implementation.

[danielmeza]

@ivmarkov @Vollbrecht Hi guys, I have draft the PR #588