Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StreamString SSO bug #6035

Merged
merged 27 commits into from
May 2, 2019
Merged

Conversation

mongozmaki
Copy link
Contributor

With SSO implementation in String, StreamString::write generates wrong strings under some circumstances.
Reason is that String::len() returns strlen(sso_buf) if SSO=true but with newly written data (in StreamString::write) the null-termination missing at the time len() is called.
Furthermore, len() is called twice which is inefficient if SSO=true.

Testcode:

StreamString s;
s.print('{');
s.print('\"');
s.print(String("message"));
s.print('\"');
Serial.println(s);

Expected: '{"message"'
Actual: '{"message��"'

Harald Frostel added 24 commits September 26, 2018 13:36
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thanks! Would you be able to add a test to the host tests that catches the error (you already have the code, just need to encapsulate it).

This also needs to apply to the ESP32 core. Do you want to submit a patch there or would you like me to?

.gitignore Outdated Show resolved Hide resolved
@earlephilhower earlephilhower added this to the 2.5.1 milestone May 2, 2019
earlephilhower added a commit to earlephilhower/arduino-esp32 that referenced this pull request May 2, 2019
As found by @mongozmaki in esp8266/Arduino#6035

With SSO implementation in String, StreamString::write generates wrong
strings under some circumstances.  Reason is that String::len() returns
strlen(sso_buf) if SSO=true but with newly written data
(in StreamString::write) the null-termination missing at the time len()
is called.

Furthermore, len() is called twice which is inefficient if SSO=true.
@earlephilhower earlephilhower merged commit a994b75 into esp8266:master May 2, 2019
@mongozmaki mongozmaki deleted the fix/StreamString branch May 2, 2019 23:14
mongozmaki pushed a commit to mongozmaki/Arduino that referenced this pull request May 2, 2019
mongozmaki pushed a commit to mongozmaki/Arduino that referenced this pull request May 2, 2019
@mongozmaki
Copy link
Contributor Author

Hi!
I added a pull request for the test case.
I was not sure where to put it so I put it in host/core/test_string.cpp. I could not test the test case but it should work.

I will submit the same to the ESP32

me-no-dev pushed a commit to espressif/arduino-esp32 that referenced this pull request May 11, 2019
As found by @mongozmaki in esp8266/Arduino#6035

With SSO implementation in String, StreamString::write generates wrong
strings under some circumstances.  Reason is that String::len() returns
strlen(sso_buf) if SSO=true but with newly written data
(in StreamString::write) the null-termination missing at the time len()
is called.

Furthermore, len() is called twice which is inefficient if SSO=true.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants