NVS Partition Generator Utility lost ability to parse multiline string values in v4.3 (IDFGH-5434) #7175

DaStoned · 2021-06-21T14:04:45Z

Environment

Development Kit: ESP32-DevKitC
Kit version: v4
Module or chip used: ESP32-WROOM-32E
IDF version: v4.3-rc-2-g88c2b69c68
Build System: idf.py
Compiler version: xtensa-esp32-elf-gcc (crosstool-NG esp-2020r3) 8.4.0
Operating System: Linux
Using an IDE?: No
Power Supply: USB

Problem Description

Given NVS partition data described in CSV file below, nvs_partition_gen.py fails to parse the multiline string value (a PEM format ECC key which contains newlines). This works in ESP IDF v4.2.2 and started to fail with v4.3.

key,type,encoding,value
identity,namespace,,
pubKey,data,string,"-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEeCiKY3/V2mAkDmpg5ebdN5DgRGkm
fWOkgc5rAvEGuYlWXGFABI4KIex3OCljygDt5R1f0uatA7lnrziunqACuQ==
-----END PUBLIC KEY-----
"

As a result I can no longer provision my devices with their keys and certificates.

Expected Behavior

In ESP IDF 4.2.2 this generates a valid NVS partition binary with the multi-line PEM key as a string:

$ nvs_partition_gen.py generate out/part_nvs_ego.UBIK-2D3D00.csv part_nvs_ego.UBIK-2D3D00.bin 0x4000
Creating NVS binary with version: V2 - Multipage Blob Support Enabled

Created NVS binary: ===> /home/tarmo/projects/ubik/dcov/src/node_provision_gcp/part_nvs_ego.UBIK-2D3D00.bin

Actual Behavior

In ESP IDF 4.3 this started to fail:

$ nvs_partition_gen.py generate out/part_nvs_ego.UBIK-2D3D00.csv part_nvs_ego.UBIK-2D3D00.bin 0x4000
Creating NVS binary with version: V2 - Multipage Blob Support Enabled

Error:
Length of key `MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEeCiKY3/V2mAkDmpg5ebdN5DgRGkm` should be <= 15 characters.

Warning: NVS binary not created...

Steps to reproduce

Copy-paste the content of sample CSV into a file
Run command for generating the NVS data partition

Side note: in addition to this issue, comments in the file (lines starting with #) are no longer supported, which is slightly annoying but not critical.

Other

My python version is 3.9.1 (note this didn't change when I upgraded ESP IDF) and below is output from pip freeze (which was upgraded quite a bit with the upgrade) if relevant:

bitstring==3.1.7
Brotli==1.0.9
cachetools==4.2.2
certifi==2021.5.30
cffi==1.14.5
chardet==4.0.0
click==8.0.1
construct==2.10.54
cryptography==3.4.7
ecdsa==0.17.0
Flask==0.12.5
Flask-Compress==1.10.1
Flask-SocketIO==2.9.6
future==0.18.2
gdbgui==0.13.2.0
gevent==1.5.0
google-api-core==1.30.0
google-auth==1.31.0
google-cloud-iot==2.0.1
google-cloud-secret-manager==2.1.0
googleapis-common-protos==1.53.0
greenlet==1.1.0
grpc-google-iam-v1==0.12.3
grpcio==1.38.0
idna==2.10
itsdangerous==2.0.1
Jinja2==3.0.1
kconfiglib==13.7.1
libcst==0.3.19
MarkupSafe==2.0.1
mypy-extensions==0.4.3
packaging==20.9
proto-plus==1.18.1
protobuf==3.17.3
pyasn1==0.4.8
pyasn1-modules==0.2.8
pycparser==2.20
pyelftools==0.27
pygdbmi==0.9.0.2
Pygments==2.9.0
pyparsing==2.3.1
pyserial==3.5
python-engineio==3.14.2
python-socketio==4.6.1
pytz==2021.1
PyYAML==5.4.1
reedsolo==1.5.4
requests==2.25.1
rsa==4.7.2
six==1.16.0
typing-extensions==3.10.0.0
typing-inspect==0.7.1
urllib3==1.26.5
Werkzeug==0.16.1

The text was updated successfully, but these errors were encountered:

shivani-tipnis · 2021-07-15T10:17:54Z

@DaStoned Thank you. We are looking into adding the fix.

dbahrdt · 2021-09-01T11:27:38Z

As far as I can tell it's also not possible to have commas within escaped strings.

I think the problem is in the following line:

esp-idf/components/nvs_flash/nvs_partition_generator/nvs_partition_gen.py

Line 936 in 5f38b76

value = line.split(',')

This simply splits the line at the commas. The problem mentioned in this issue is also related to that code since the file is read line by line and thus the newlines within escaped strings are lost.

shivani-tipnis · 2021-09-15T17:27:40Z

Hi @DaStoned

For IDF >=v4.3, recommend to use the file type to use multiline strings in the CSV.
You can use it this way -

key,type,encoding,value 
identity,namespace,, 
pubKey,file,string,pubKey.txt

And then you can have the string in the pubKey.txt as follows -

-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEeCiKY3/V2mAkDmpg5ebdN5DgRGkm
fWOkgc5rAvEGuYlWXGFABI4KIex3OCljygDt5R1f0uatA7lnrziunqACuQ==
-----END PUBLIC KEY-----

shivani-tipnis · 2021-09-15T17:40:55Z

Hi @dbahrdt
If the string contains a comma, recommend to use the file type in the same way as shown in above snippet about using multiline strings.
Thanks

DaStoned · 2021-09-21T12:33:28Z

Hi, thank you for the reply. That is a viable workaround, though not nearly as convenient as the earlier behaviour I'm sorry to say.

DaStoned · 2021-09-23T17:06:38Z

@shivani-tipnis - I see you've marked this issue as Done. Unfortunately I cannot accept this resolution. The proposed workaround is not viable for the problem that @dbahrdt reported, for strings containing commas. I will be generating per-device strings during the manufacturing process and will not know in advance which characters they contain. A CSV file must support a string field with commas (and all other regular, printable characters) if enclosed within quotes, or it's not usable for string data. E.g. this worked fine in v4.2 (NVS stored value "Hello,World") and stopped working in v4.3 (stored value "Hello"):

wifiFbPass,data,string,"Hello,World"

When looking at the history of nvs_partition_gen.py I see that you've replaced the Python csv module with a very simple split() function which is not really up to the task of parsing CSV.

Gentlemen. This script is proposed by Espressif as the tool for preparing device images in manufacturing, and you've effectively made it useless for that purpose in v4.3. Please show your commercial users some respect and fix this.

shivani-tipnis · 2021-09-24T06:11:03Z

Hi @DaStoned
We will consider your feedback and work on improving this further.
Thanks

DaStoned · 2021-09-24T07:57:53Z

Thank you!

…ts in the CSV file Closes #7175

will-emmerson · 2022-01-17T11:55:41Z

I've also just been affected by this. I think it's a good idea to force people to use the file type for more complicated strings as it's actually more explicit and less likely to go wrong. However it should definitely refuse to run (and suggest the fix) when a string contains a comma as currently it is silently failing.

DaStoned · 2022-01-17T12:55:39Z

I've also just been affected by this. I think it's a good idea to force people to use the file type for more complicated strings as it's actually more explicit and less likely to go wrong. However it should definitely refuse to run (and suggest the fix) when a string contains a comma as currently it is silently failing.

I agree that the silent failure is the worst problem.

But as a side note - the Python CSV module (which was used in previous versions of this same script) is perfectly capable of handling "complicated" strings. Replacing this with a naive split string on commas is the source of the current problem.

Please revert back to the Python CSV module (which did its job admirably) or use another, more deterministic text format (JSON, YAML,...)

dobairoland · 2022-06-21T08:30:31Z

Hi all. We are sorry it took us so long but this has now been addressed in the master branch (v5.0). The fix will be backported to previous releases shortly.

DaStoned · 2022-06-21T08:34:11Z

Hi all. We are sorry it took us so long but this has now been addressed in the master branch (v5.0). The fix will be backported to previous releases shortly.

Wonderful news. I'll be happy to verify once it reaches a 4.x release.

This fixes the issue where multiline strings and strings with delimiters inside the nvs input csv file were incorrectly parsed, and adds back the ability to add comment lines anywhere in the CSV file. The issue stems from the move away from the python built in csv module to manual parsing, which was made after moving away from using the csv module to parse mfg data. This reverts back to using the csv module for parsing and writing csv data in both mfg_gen and nvs_partition_gen, fixes the original issue in mfg_gen and improves code quality which makes the code more readable and maintainable. Closes #7175

…ts in the CSV file Closes espressif/esp-idf#7175

This fixes the issue where multiline strings and strings with delimiters inside the nvs input csv file were incorrectly parsed, and adds back the ability to add comment lines anywhere in the CSV file. The issue stems from the move away from the python built in csv module to manual parsing, which was made after moving away from using the csv module to parse mfg data. This reverts back to using the csv module for parsing and writing csv data in both mfg_gen and nvs_partition_gen, fixes the original issue in mfg_gen and improves code quality which makes the code more readable and maintainable. Closes espressif/esp-idf#7175

espressif-bot added the Status: Opened Issue is new label Jun 21, 2021

github-actions bot changed the title ~~NVS Partition Generator Utility lost ability to parse multiline string values in v4.3~~ NVS Partition Generator Utility lost ability to parse multiline string values in v4.3 (IDFGH-5434) Jun 21, 2021

espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jul 14, 2021

espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels Sep 16, 2021

espressif-bot closed this as completed in d8b1c3d Sep 24, 2021

Alvin1Zhang reopened this Sep 24, 2021

espressif-bot added Status: Opened Issue is new and removed Resolution: Done Issue is done internally Status: Done Issue is done internally labels Sep 24, 2021

espressif-bot pushed a commit that referenced this issue Oct 12, 2021

nvs_tool: Update Readme with supports of multiline strings and commen…

4a21a9e

…ts in the CSV file Closes #7175

espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jun 3, 2022

espressif-bot assigned dobairoland Jun 17, 2022

espressif-bot added Status: Reviewing Issue is being reviewed and removed Status: In Progress Work is in progress labels Jun 17, 2022

espressif-bot added Resolution: Done Issue is done internally Status: Resolved Issue is done internally and removed Status: Reviewing Issue is being reviewed labels Jun 17, 2022

espressif-bot assigned DNedic and unassigned dobairoland Jun 17, 2022

espressif-bot closed this as completed in bbc3add Jun 21, 2022

espressif-bot pushed a commit to espressif/esp-idf-nvs-partition-gen that referenced this issue Nov 20, 2023

nvs_tool: Update Readme with supports of multiline strings and commen…

5b33b81

…ts in the CSV file Closes espressif/esp-idf#7175

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NVS Partition Generator Utility lost ability to parse multiline string values in v4.3 (IDFGH-5434) #7175

NVS Partition Generator Utility lost ability to parse multiline string values in v4.3 (IDFGH-5434) #7175

DaStoned commented Jun 21, 2021 •

edited

shivani-tipnis commented Jul 15, 2021

dbahrdt commented Sep 1, 2021

shivani-tipnis commented Sep 15, 2021 •

edited

shivani-tipnis commented Sep 15, 2021 •

edited

DaStoned commented Sep 21, 2021

DaStoned commented Sep 23, 2021 •

edited

shivani-tipnis commented Sep 24, 2021

DaStoned commented Sep 24, 2021

will-emmerson commented Jan 17, 2022

DaStoned commented Jan 17, 2022

dobairoland commented Jun 21, 2022

DaStoned commented Jun 21, 2022

NVS Partition Generator Utility lost ability to parse multiline string values in v4.3 (IDFGH-5434) #7175

NVS Partition Generator Utility lost ability to parse multiline string values in v4.3 (IDFGH-5434) #7175

Comments

DaStoned commented Jun 21, 2021 • edited

Environment

Problem Description

Expected Behavior

Actual Behavior

Steps to reproduce

Other

shivani-tipnis commented Jul 15, 2021

dbahrdt commented Sep 1, 2021

shivani-tipnis commented Sep 15, 2021 • edited

shivani-tipnis commented Sep 15, 2021 • edited

DaStoned commented Sep 21, 2021

DaStoned commented Sep 23, 2021 • edited

shivani-tipnis commented Sep 24, 2021

DaStoned commented Sep 24, 2021

will-emmerson commented Jan 17, 2022

DaStoned commented Jan 17, 2022

dobairoland commented Jun 21, 2022

DaStoned commented Jun 21, 2022

DaStoned commented Jun 21, 2021 •

edited

shivani-tipnis commented Sep 15, 2021 •

edited

shivani-tipnis commented Sep 15, 2021 •

edited

DaStoned commented Sep 23, 2021 •

edited