Skip to content

JSON API: make sure, strings are valid UTF8#1922

Merged
BareosBot merged 3 commits intobareos:masterfrom
joergsteffens:dev/joergs/master/joblog-empty
Nov 19, 2024
Merged

JSON API: make sure, strings are valid UTF8#1922
BareosBot merged 3 commits intobareos:masterfrom
joergsteffens:dev/joergs/master/joblog-empty

Conversation

@joergsteffens
Copy link
Member

@joergsteffens joergsteffens commented Aug 11, 2024

Thank you for contributing to the Bareos Project!

Before, strings that have not been valid UTF8 have been silently ignored.
This change verifies the string and replaces invalid chars if needed. This should be the beast approach in most cases.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Required backport PRs have been created
  • Correct milestone is set
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

@joergsteffens joergsteffens self-assigned this Aug 11, 2024
@joergsteffens joergsteffens added this to the 24.0.0 milestone Aug 11, 2024
@sebsura sebsura linked an issue Aug 13, 2024 that may be closed by this pull request
@joergsteffens
Copy link
Member Author

As it requires an extra CPM package, we wait until CPM handling is defined.

Comment on lines +42 to +60

cpmfindpackage(
NAME utf8cpp VERSION 4.0.4 GITHUB_REPOSITORY nemtrif/utfcpp EXCLUDE_FROM_ALL
YES
)
Copy link
Member

Choose a reason for hiding this comment

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

Using CPMAddPackage() will do the right thing (and will be detected by cmake-format, too). Giving 3.2.1 as the version should use the locally available package on Debian 12 and Ubuntu 22/24. Giving the GIT_TAG will select that tag if no package was found with find_package():

Suggested change
cpmfindpackage(
NAME utf8cpp VERSION 4.0.4 GITHUB_REPOSITORY nemtrif/utfcpp EXCLUDE_FROM_ALL
YES
)
CPMAddPackage(
NAME utf8cpp
VERSION 3.2.1
GITHUB_REPOSITORY nemtrif/utfcpp
GIT_TAG "v4.0.4"
EXCLUDE_FROM_ALL YES
)

@joergsteffens joergsteffens force-pushed the dev/joergs/master/joblog-empty branch 3 times, most recently from e25e7b9 to 1fbee9b Compare November 7, 2024 20:43
@joergsteffens joergsteffens marked this pull request as ready for review November 7, 2024 20:43
@joergsteffens joergsteffens requested a review from arogge November 7, 2024 20:43
@arogge arogge changed the title JSON API: make sure, string are valid UTF8 JSON API: make sure, strings are valid UTF8 Nov 12, 2024
@joergsteffens joergsteffens force-pushed the dev/joergs/master/joblog-empty branch from 1fbee9b to 27f0b70 Compare November 12, 2024 15:06
@arogge arogge added the bug This addresses a bug label Nov 18, 2024
Copy link
Member

@arogge arogge left a comment

Choose a reason for hiding this comment

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

Looks great! Do we want a backport into Bareos 23?

joergsteffens and others added 3 commits November 19, 2024 16:35
Before, strings that have not been valid UTF8 have been silently
discarded.
@BareosBot BareosBot force-pushed the dev/joergs/master/joblog-empty branch from 9d665d8 to 2f6653c Compare November 19, 2024 16:35
@BareosBot BareosBot merged commit ba6e180 into bareos:master Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This addresses a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebUI fails to display job details when log contains empty line

3 participants