Skip to content

fix: fix JSON validation to accept plain strings#138

Merged
18202781743 merged 2 commits intolinuxdeepin:masterfrom
18202781743:master
Feb 5, 2026
Merged

fix: fix JSON validation to accept plain strings#138
18202781743 merged 2 commits intolinuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743
Copy link
Contributor

@18202781743 18202781743 commented Feb 3, 2026

The stringToQVariant function was incorrectly rejecting plain strings
that weren't valid JSON by returning an empty QVariant. This caused
valid configuration values to be filtered out when they should have been
accepted as regular strings.

  1. Modified stringToQVariant to wrap single JSON values in an array
    for parsing
  2. Added fallback to return the original string if JSON parsing fails
  3. Enhanced isValidTextJsonValue to better distinguish between malformed
    JSON and plain text
  4. Added QJsonArray include for array operations

The fix ensures that:

  • Valid JSON objects and arrays are properly parsed
  • Single JSON values (numbers, booleans, null, quoted strings) are
    supported
  • Plain text strings that aren't JSON are accepted as-is
  • Only strings starting with { or [ that fail JSON parsing are rejected
    as malformed JSON

Log: Fixed issue where plain text strings were incorrectly filtered out
in configuration values

Influence:

  1. Test configuration values with plain text strings containing special
    characters
  2. Verify JSON objects and arrays are still properly validated
  3. Test single JSON values like numbers, booleans, and null
  4. Test strings that look like JSON but are malformed (should be
    rejected)
  5. Verify mixed content with both JSON and plain text scenarios

fix: 修复JSON验证以接受普通字符串

stringToQVariant函数之前错误地拒绝了不是有效JSON的普通字符串,返回空的
QVariant。这导致有效的配置值被错误过滤,而它们本应作为普通字符串被接受。

  1. 修改stringToQVariant函数,将单个JSON值包装在数组中进行解析
  2. 添加回退机制,当JSON解析失败时返回原始字符串
  3. 增强isValidTextJsonValue函数,更好地区分格式错误的JSON和纯文本
  4. 添加QJsonArray包含以支持数组操作

修复确保:

  • 有效的JSON对象和数组被正确解析
  • 单个JSON值(数字、布尔值、null、带引号的字符串)得到支持
  • 不是JSON的纯文本字符串被原样接受
  • 只有以{或[开头但JSON解析失败的字符串被拒绝为格式错误的JSON

Log: 修复了配置值中普通字符串被错误过滤的问题

Influence:

  1. 测试包含特殊字符的纯文本字符串配置值
  2. 验证JSON对象和数组是否仍被正确验证
  3. 测试单个JSON值,如数字、布尔值和null
  4. 测试看起来像JSON但格式错误的字符串(应被拒绝)
  5. 验证同时包含JSON和纯文本的混合内容场景

PMS: BUG-349663
Change-Id: Ieb2deccba3ba4e89266a9e8027f2377abbe26a48

Summary by Sourcery

Relax JSON value handling to correctly parse single JSON values and accept non-JSON text as plain strings in configuration values.

Bug Fixes:

  • Prevent plain text configuration strings from being incorrectly rejected when they are not valid JSON but should be treated as regular strings.

Enhancements:

  • Extend JSON parsing and validation to support standalone JSON primitives and better distinguish malformed JSON from ordinary text input.

The stringToQVariant function was incorrectly rejecting plain strings
that weren't valid JSON by returning an empty QVariant. This caused
valid configuration values to be filtered out when they should have been
accepted as regular strings.

1. Modified stringToQVariant to wrap single JSON values in an array
for parsing
2. Added fallback to return the original string if JSON parsing fails
3. Enhanced isValidTextJsonValue to better distinguish between malformed
JSON and plain text
4. Added QJsonArray include for array operations

The fix ensures that:
- Valid JSON objects and arrays are properly parsed
- Single JSON values (numbers, booleans, null, quoted strings) are
supported
- Plain text strings that aren't JSON are accepted as-is
- Only strings starting with { or [ that fail JSON parsing are rejected
as malformed JSON

Log: Fixed issue where plain text strings were incorrectly filtered out
in configuration values

Influence:
1. Test configuration values with plain text strings containing special
characters
2. Verify JSON objects and arrays are still properly validated
3. Test single JSON values like numbers, booleans, and null
4. Test strings that look like JSON but are malformed (should be
rejected)
5. Verify mixed content with both JSON and plain text scenarios

fix: 修复JSON验证以接受普通字符串

stringToQVariant函数之前错误地拒绝了不是有效JSON的普通字符串,返回空的
QVariant。这导致有效的配置值被错误过滤,而它们本应作为普通字符串被接受。

1. 修改stringToQVariant函数,将单个JSON值包装在数组中进行解析
2. 添加回退机制,当JSON解析失败时返回原始字符串
3. 增强isValidTextJsonValue函数,更好地区分格式错误的JSON和纯文本
4. 添加QJsonArray包含以支持数组操作

修复确保:
- 有效的JSON对象和数组被正确解析
- 单个JSON值(数字、布尔值、null、带引号的字符串)得到支持
- 不是JSON的纯文本字符串被原样接受
- 只有以{或[开头但JSON解析失败的字符串被拒绝为格式错误的JSON

Log: 修复了配置值中普通字符串被错误过滤的问题

Influence:
1. 测试包含特殊字符的纯文本字符串配置值
2. 验证JSON对象和数组是否仍被正确验证
3. 测试单个JSON值,如数字、布尔值和null
4. 测试看起来像JSON但格式错误的字符串(应被拒绝)
5. 验证同时包含JSON和纯文本的混合内容场景

PMS: BUG-349663
Change-Id: Ieb2deccba3ba4e89266a9e8027f2377abbe26a48
@18202781743 18202781743 requested review from BLumia and mhduiy February 3, 2026 04:28
@sourcery-ai
Copy link

sourcery-ai bot commented Feb 3, 2026

Reviewer's Guide

Adjusts JSON handling utilities so that plain text strings are accepted as-is, while still correctly parsing full JSON documents and single JSON values and more accurately rejecting only truly malformed JSON that looks like structured data.

Flow diagram for updated stringToQVariant JSON handling

flowchart TD
  A[Input string s] --> B[Parse s as JSON document]
  B --> C{JSON parse error?}
  C -->|No| D[Return doc.toVariant]
  C -->|Yes| E["Wrap s as [s]"]
  E --> F[Parse wrapped JSON as document]
  F --> G{Wrapped JSON parse error?}
  G -->|No| H[Get array from wrappedDoc]
  H --> I{Array is empty?}
  I -->|No| J[Return first element toVariant]
  I -->|Yes| K[Return s as plain string]
  G -->|Yes| K[Return s as plain string]
Loading

File-Level Changes

Change Details Files
Relax and extend JSON-to-QVariant conversion to support single JSON values and fall back to raw strings when parsing fails.
  • Keep the existing attempt to parse the input as a JSON document and return its QVariant representation on success
  • If document parsing fails, wrap the input in a one-element JSON array and re-parse to support numbers, booleans, null, and bare JSON values
  • On successful wrapped parsing, extract the first array element and convert it to a QVariant
  • If all JSON parsing attempts fail, return the original QString as the QVariant instead of an empty QVariant
dconfig-center/common/helper.hpp
Tighten JSON validation logic to distinguish malformed JSON from non-JSON plain text while still allowing single JSON values.
  • First try parsing the string as a JSON document (object or array), returning true on success
  • If that fails, retry parsing by wrapping the value in a JSON array to cover scalar JSON types and quoted JSON strings
  • If parsing still fails, explicitly reject only strings that start with '{' or '[' as malformed JSON, and accept all other strings as valid plain text
  • Trim the string before checking leading characters to avoid whitespace issues
dconfig-center/common/helper.hpp
Add required JSON array support.
  • Include the QJsonArray header to support JSON array operations used in wrapped parsing
dconfig-center/common/helper.hpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The JSON parsing logic is now duplicated between stringToQVariant and isValidTextJsonValue; consider extracting the "parse as document, then as single value" flow into a shared helper to keep behavior consistent and reduce maintenance overhead.
  • When wrapping the string as JSON (e.g., QString("[") + s + QString("]")), you can simplify and slightly optimize this by using QStringLiteral or direct concatenation like "[" + s + "]" to avoid unnecessary temporary QString constructions.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The JSON parsing logic is now duplicated between stringToQVariant and isValidTextJsonValue; consider extracting the "parse as document, then as single value" flow into a shared helper to keep behavior consistent and reduce maintenance overhead.
- When wrapping the string as JSON (e.g., QString("[") + s + QString("]")), you can simplify and slightly optimize this by using QStringLiteral or direct concatenation like "[" + s + "]" to avoid unnecessary temporary QString constructions.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes configuration value handling so non-JSON text is preserved as plain strings instead of being rejected, while still supporting JSON objects/arrays and standalone JSON primitives.

Changes:

  • Update stringToQVariant to parse standalone JSON primitives by wrapping input in an array, with a fallback to return the original string on parse failure.
  • Update isValidTextJsonValue to accept plain text as valid input while still rejecting malformed JSON that appears to be a structured JSON (starts with { or [).
  • Add QJsonArray include to support array extraction after wrapped parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mhduiy
mhduiy previously approved these changes Feb 5, 2026
1. Added QJsonObject include for proper JSON handling
2. Modified qvariantToString and qvariantToStringCompact to add quotes
around string values when converting QVariant to string
3. Changed stringToQVariant to handle JSON parsing more robustly with
clearer logic
4. Updated isValidTextJsonValue to validate JSON values more accurately
by checking trimmed input and proper JSON parsing
5. Fixed getCommand output to use qvariantToString directly instead of
adding extra quotes

Log: Fixed JSON string formatting in configuration output

Influence:
1. Test configuration value display to ensure string values are properly
quoted
2. Verify JSON array and object parsing from string input
3. Test edge cases like empty strings, whitespace-only strings, and
malformed JSON
4. Validate that numeric, boolean, and null JSON values are handled
correctly
5. Check that complex nested JSON structures are properly converted
6. Test command line output formatting for different data types

fix: 改进变体转换中的JSON字符串处理

1. 添加QJsonObject包含以支持正确的JSON处理
2. 修改qvariantToString和qvariantToStringCompact,在将QVariant转换为字符
串时为字符串值添加引号
3. 改进stringToQVariant,使用更清晰的逻辑更稳健地处理JSON解析
4. 更新isValidTextJsonValue,通过检查修剪后的输入和正确的JSON解析来更准
确地验证JSON值
5. 修复getCommand输出,直接使用qvariantToString而不是添加额外的引号

Log: 修复配置输出中的JSON字符串格式问题

Influence:
1. 测试配置值显示,确保字符串值被正确添加引号
2. 验证从字符串输入解析JSON数组和对象
3. 测试边界情况,如空字符串、仅空白字符的字符串和格式错误的JSON
4. 验证数字、布尔值和null JSON值是否正确处理
5. 检查复杂的嵌套JSON结构是否正确转换
6. 测试不同数据类型的命令行输出格式

PMS: BUG-349663
Change-Id: I1f2f441405b0ac73896b222339dc02c5c863d0f2
@deepin-ci-robot
Copy link

deepin pr auto review

Git Diff 代码审查报告

我已对提供的 Git diff 进行了全面审查,主要关注语法逻辑、代码质量、代码性能和代码安全等方面。以下是我的详细分析和建议:

1. 语法逻辑分析

1.1 JSON 字符串处理逻辑

qvariantToStringqvariantToStringCompact 函数中,添加了对字符串类型的特殊处理:

const QJsonValue jsonVal = QJsonValue::fromVariant(v);
if (jsonVal.isString()) {
    return QString("\"%1\"").arg(jsonVal.toString());
}

问题:这个逻辑存在潜在问题。当字符串本身包含引号时,可能会导致 JSON 格式错误。例如,如果输入字符串是 he said "hello",输出将是 "he said "hello"",这不是有效的 JSON。

建议:应该使用 QJsonDocumenttoJson() 方法来确保字符串被正确转义,而不是手动添加引号。

1.2 stringToQVariant 函数逻辑

QString wrapped = QString("[%1]").arg(s);
const auto &arrayDoc = QJsonDocument::fromJson(wrapped.toUtf8(), &error);
if (error.error == QJsonParseError::NoError) {
    QJsonArray array = arrayDoc.array();
    if (!array.isEmpty()) {
        return array.first().toVariant();
    }
}

问题:这种包装方法虽然能解析单个 JSON 值,但有一些边界情况未处理:

  1. 如果输入是空字符串,会返回空字符串,而不是解析失败
  2. 如果输入是多个值(如 "value1" "value2"),会只返回第一个值

建议:应该明确处理这些边界情况,并在输入不符合预期时返回原始字符串或错误。

1.3 isValidTextJsonValue 函数逻辑

QString trimmed = s.trimmed();
if (trimmed.isEmpty()) {
    return false;
}

问题:这个函数对空字符串的处理是正确的,但应该考虑更多边界情况,如只包含空格、制表符或换行符的字符串。

2. 代码质量评估

2.1 代码重复

qvariantToStringqvariantToStringCompact 函数有大量重复代码,只是最后的 toJson() 调用参数不同。

建议:可以提取公共逻辑到一个辅助函数中,减少代码重复。

2.2 测试覆盖

添加的 ut_helper.cpp 文件提供了全面的单元测试,这是一个很好的实践。测试用例覆盖了各种 JSON 类型、边界情况和错误情况。

建议:考虑添加更多关于特殊字符和 Unicode 字符的测试用例,以确保国际化支持。

2.3 代码注释

代码中添加了一些注释,但不够全面。

建议:为复杂的逻辑添加更详细的注释,解释为什么这样做,而不仅仅是做了什么。

3. 代码性能分析

3.1 字符串处理

isValidTextJsonValuestringToQVariant 函数中,多次进行字符串转换和解析:

QString wrapped = QString("[%1]").arg(trimmed);
QJsonDocument::fromJson(wrapped.toUtf8(), &error);

问题:字符串拼接和转换可能会影响性能,特别是在处理大量数据时。

建议:考虑优化字符串处理,减少不必要的转换。例如,可以预分配字符串空间或使用更高效的字符串拼接方法。

3.2 JSON 解析

多次调用 QJsonDocument::fromJson 进行解析:

QJsonDocument::fromJson(trimmed.toUtf8(), &error);
if (error.error == QJsonParseError::NoError) {
    return true;
}

QString wrapped = QString("[%1]").arg(trimmed);
QJsonDocument::fromJson(wrapped.toUtf8(), &error);

问题:对于有效的 JSON 值,会进行两次解析尝试,这可能会影响性能。

建议:可以考虑使用更高效的解析策略,例如先尝试直接解析,失败后再尝试包装解析。

4. 代码安全考虑

4.1 输入验证

stringToQVariant 函数直接使用输入字符串进行 JSON 解析,没有进行充分的输入验证:

const auto &doc = QJsonDocument::fromJson(s.toUtf8(), &error);

问题:恶意构造的输入可能导致解析错误或意外行为。

建议:应该对输入进行更严格的验证,限制输入长度或格式,防止潜在的注入攻击。

4.2 错误处理

虽然代码中有错误处理,但不够全面:

if (error.error == QJsonParseError::NoError) {
    // 成功解析为对象或数组
    return doc.toVariant();
}

问题:错误信息没有被记录或传播,可能导致难以诊断的问题。

建议:考虑添加日志记录或错误报告机制,以便在出现问题时能够快速定位。

4.3 测试用例中的潜在问题

在测试文件中,有一个测试用例被跳过:

TEST_F(ut_DConfigServer, removeUserData) {
    GTEST_SKIP() << "Skipping removeUserData test for now";

问题:跳过测试可能会隐藏潜在的问题。

建议:应该尽快修复被跳过的测试用例,或者明确标记为什么需要跳过,并计划何时修复。

5. 其他建议

5.1 头文件包含

添加了新的头文件包含:

#include <QJsonObject>
#include <QJsonArray>

问题:这些头文件可能不是必需的,因为代码中没有直接使用这些类型。

建议:检查是否真的需要这些头文件,如果不使用,应该移除以减少编译时间和依赖。

5.2 测试用例设计

测试用例设计得很好,覆盖了各种情况,但可以考虑添加性能测试,特别是对于大数据量的情况。

6. 总体评价

这次修改主要改进了 JSON 值的处理,增加了对单个 JSON 值的支持,并添加了全面的单元测试。代码整体质量较高,但在字符串处理、错误处理和性能优化方面还有改进空间。

建议在合并前考虑上述问题,特别是字符串处理中的引号转义问题,这可能会导致 JSON 格式错误。同时,应该尽快修复被跳过的测试用例,确保代码的完整性和可靠性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@18202781743 18202781743 merged commit c58e9de into linuxdeepin:master Feb 5, 2026
21 checks passed
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.

3 participants