Skip to content

fix: add JSON validation for text input fields #136

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
18202781743 merged 1 commit into from
Jan 26, 2026
Merged

fix: add JSON validation for text input fields #136

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
7 changes: 7 additions & 0 deletions dconfig-center/common/helper.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,14 @@
return s;
}

static bool isValidTextJsonValue(const QString &s)
{
QJsonParseError error;
QJsonDocument::fromJson(s.toUtf8(), &error);
return error.error == QJsonParseError::NoError;
}

static QString qvariantToCmd(const QVariant &v)

Check warning on line 238 in dconfig-center/common/helper.hpp

View workflow job for this annotation

GitHub Actions / cppcheck 

The function 'qvariantToCmd' is never used.

Check warning on line 238 in dconfig-center/common/helper.hpp

View workflow job for this annotation

GitHub Actions / static-check / static-check 

The function 'qvariantToCmd' is never used.
{
auto stringValue = qvariantToStringCompact(v);
auto jsonValue = QJsonValue::fromVariant(v);
Expand Down
4 changes: 4 additions & 0 deletions dconfig-center/dde-dconfig-editor/mainwindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,10 @@ void KeyContent::setBaseInfo(ConfigGetter *getter, const QString &language)
auto widget = new DLineEdit(this);
widget->setEnabled(canWrite);
connect(widget, &DLineEdit::editingFinished, widget, [this, widget](){
if (!isValidTextJsonValue(widget->text())) {
qWarning() << "invalid json value" << widget->text();
Comment on lines 724 to +726
Copy link

@sourcery-ai sourcery-ai bot Jan 23, 2026

Choose a reason for hiding this comment

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

suggestion: Invalid JSON is only logged, with no in-UI feedback or rollback of the edited value.

Since you return after logging, the invalid JSON leaves the edit text and focus unchanged and never emits valueChanged, so from the user’s perspective the change just “does nothing.” If the goal is to reject bad input, please also provide visible feedback (e.g., revert to last valid value or show an error/invalid state) so the failure isn’t silent in the UI.

Suggested implementation:

        auto widget = new DLineEdit(this);
        widget->setEnabled(canWrite);
        // track last valid value for rollback on invalid input
        widget->setProperty("lastValidText", widget->text());

        connect(widget, &DLineEdit::editingFinished, widget, [this, widget](){
            if (!isValidTextJsonValue(widget->text())) {
                qWarning() << "invalid json value" << widget->text();

                // revert to last valid value and select it to provide visible feedback
                const QString lastValidText = widget->property("lastValidText").toString();
                widget->setText(lastValidText);
                widget->selectAll();

                return;
            }

            // update last valid value on successful edit
            widget->setProperty("lastValidText", widget->text());

            widget->clearFocus();
            emit valueChanged(stringToQVariant(widget->text()));
        });

If the widget’s initial value is set elsewhere (e.g., widget->setText(initialValue);), it would be best to also call widget->setProperty("lastValidText", widget->text()); immediately after that, so the first invalid edit correctly rolls back to the loaded value instead of an empty string.

return;
}
widget->clearFocus();
emit valueChanged(stringToQVariant(widget->text()));
});
Expand Down
4 changes: 4 additions & 0 deletions dconfig-center/dde-dconfig-editor/oemdialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,10 @@ QWidget *OEMDialog::getItemWidget(ConfigGetter *getter, DStandardItem *item)
widget->setText(qvariantToString(v));
widget->setEnabled(canWrite);
connect(widget, &DLineEdit::textChanged, widget, [this, item](const QString &text){
if (!isValidTextJsonValue(text)) {
qWarning() << "invalid json value" << text;
return;
}
item->setData(stringToQVariant(text), ValueRole);
treeItemChanged(item);
});
Expand Down
4 changes: 4 additions & 0 deletions dconfig-center/dde-dconfig/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,10 @@ int CommandManager::setCommand()
#endif
manager->setValue(key, value.toDouble());
} else {
if (!isValidTextJsonValue(value)) {
outpuSTDError(QString("the value:[%1] is not a valid json text.").arg(value));
return 1;
}
manager->setValue(key, stringToQVariant(value));
}
} else {
Expand Down
Loading