Skip to content

Fix input item first character crash #422

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.

Sign up for GitHub

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

Merged
forntoh merged 1 commit into from
May 27, 2026
Merged

Fix input item first character crash #422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 57 additions & 3 deletions src/ItemInput.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class ItemInput : public MenuItem, public GraphicalMenuItem {
* @brief String value of item.
*/
char* value;
bool ownsValue;
/**
* @brief The index of first visible character.
*
Expand Down Expand Up @@ -67,6 +68,33 @@ class ItemInput : public MenuItem, public GraphicalMenuItem {
*/
fptrStr callback;

char* cloneValue(const char* src) const {
const char* source = src == NULL ? "" : src;
size_t length = strlen(source);
char* clonedValue = new char[length + 1];
memcpy(clonedValue, source, length);
clonedValue[length] = '\0';
return clonedValue;
}

bool setOwnedValue(const char* newValue) {
char* clonedValue = cloneValue(newValue);
if (ownsValue && value != NULL) {
delete[] value;
}
value = clonedValue;
ownsValue = true;
return true;
}

bool ensureOwnedValue() {
if (ownsValue) {
return false;
}
setOwnedValue(value);
return true;
}

inline GraphicalValueSelectionRenderer* getGraphicalValueSelectionRenderer(MenuRenderer* renderer) const {
if (renderer == NULL) {
return NULL;
Expand All @@ -84,7 +112,11 @@ class ItemInput : public MenuItem, public GraphicalMenuItem {
* the input is submitted.
*/
ItemInput(const char* text, char* value, fptrStr callback)
: MenuItem(text), value(value), callback(callback) {}
: MenuItem(text), value(value), ownsValue(false), callback(callback) {
if (value == NULL) {
setOwnedValue("");
}
}
/**
* Construct a new ItemInput object with no initial value.
*
Expand All @@ -93,7 +125,15 @@ class ItemInput : public MenuItem, public GraphicalMenuItem {
* the input is submitted.
*/
ItemInput(const char* text, fptrStr callback)
: ItemInput(text, (char*)"", callback) {}
: MenuItem(text), value(NULL), ownsValue(false), callback(callback) {
setOwnedValue("");
}

~ItemInput() {
if (ownsValue && value != NULL) {
delete[] value;
}
}
/**
* Get the current input value for this item.
*
Expand All @@ -111,7 +151,16 @@ class ItemInput : public MenuItem, public GraphicalMenuItem {
*/
bool setValue(char* value) {
if (this->value != value) {
if (ownsValue && this->value != NULL) {
delete[] this->value;
}

this->value = value;
ownsValue = false;
if (this->value == NULL) {
setOwnedValue("");
}

LOG(F("ItemInput::setValue"), value);
return true;
}
Expand Down Expand Up @@ -342,6 +391,7 @@ class ItemInput : public MenuItem, public GraphicalMenuItem {
if (strlen(value) == 0 || cursor == 0) {
return;
}
ensureOwnedValue();
remove(value, cursor - 1, 1);
cursor--;

Expand Down Expand Up @@ -387,8 +437,11 @@ class ItemInput : public MenuItem, public GraphicalMenuItem {
} else {
concat(value, character, buf);
}
delete[] value;
if (ownsValue && value != NULL) {
delete[] value;
}
value = buf;
ownsValue = true;
cursor++;

if (getGraphicalValueSelectionRenderer(renderer) != NULL) {
Expand All @@ -413,6 +466,7 @@ class ItemInput : public MenuItem, public GraphicalMenuItem {
* @brief Clear the value of the input field
*/
void clear(MenuRenderer* renderer) {
ensureOwnedValue();
value[0] = '\0';
cursor = 0;
view = 0;
Expand Down
17 changes: 14 additions & 3 deletions src/ItemInputCharset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,16 @@ class ItemInputCharset : public ItemInput {
void commitCharEdit(MenuRenderer* renderer) {
uint8_t length = strlen(value);
if (cursor < length) {
ensureOwnedValue();
value[cursor] = charset[charsetPosition];
} else {
char* buf = new char[length + 2];
concat(value, charset[charsetPosition], buf);
if (ownsValue && value != NULL) {
delete[] value;
}
value = buf;
ownsValue = true;
}
abortCharEdit(renderer);
LOG(F("ItemInputCharset::commitCharEdit"), charset[charsetPosition]);
Expand Down Expand Up @@ -182,10 +187,16 @@ class ItemInputCharset : public ItemInput {
uint8_t length = strlen(value);

if (cursor < length) {
char original = value[cursor];
value[cursor] = charset[charsetPosition];
char* preview = new char[length + 1];
memcpy(preview, value, length + 1);
preview[cursor] = charset[charsetPosition];

char* originalValue = value;
value = preview;
ItemInput::draw(renderer);
value[cursor] = original;
value = originalValue;

delete[] preview;
} else {
char* preview = new char[length + 2];
memcpy(preview, value, length);
Expand Down
55 changes: 55 additions & 0 deletions test/LcdMenu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,61 @@ unittest(clear_command_empties_input_and_resets_cursor) {
assertTrue(MenuItem::isEditing());
}

unittest(default_empty_input_types_first_char_safely) {
StubRenderer renderer;
LcdMenu menu(renderer);
ItemInput item("Name", NULL);

assertTrue(item.ownsValue);
assertTrue(item.process(&menu, ENTER));
assertTrue(item.process(&menu, 'A'));

assertEqual("A", item.getValue());
assertTrue(item.ownsValue);
}

unittest(set_value_static_empty_types_safely) {
StubRenderer renderer;
LcdMenu menu(renderer);
ItemInput item("Name", NULL);

item.setValue((char*)"");
assertFalse(item.ownsValue);

assertTrue(item.process(&menu, ENTER));
assertTrue(item.process(&menu, 'B'));

assertEqual("B", item.getValue());
assertTrue(item.ownsValue);
}

unittest(stack_initial_value_types_safely) {
StubRenderer renderer;
LcdMenu menu(renderer);
char stackValue[] = "X";
ItemInput item("Name", stackValue, NULL);

assertFalse(item.ownsValue);
assertTrue(item.process(&menu, ENTER));
assertTrue(item.process(&menu, 'Y'));

assertEqual("XY", item.getValue());
assertTrue(item.ownsValue);
assertEqual("X", stackValue);
}

unittest(clear_on_default_empty_input_is_safe) {
StubRenderer renderer;
LcdMenu menu(renderer);
ItemInput item("Name", NULL);

assertTrue(item.process(&menu, ENTER));
assertTrue(item.process(&menu, CLEAR));

assertEqual("", item.getValue());
assertTrue(item.ownsValue);
}

unittest(hide_disables_and_clears_display) {
TrackingRenderer renderer;
LcdMenu menu(renderer);
Expand Down
Loading