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

Cumulocity operation failureReason field cannot contain quotes #2487

Closed
jarhodes314 opened this issue Nov 29, 2023 · 3 comments
Closed

Cumulocity operation failureReason field cannot contain quotes #2487

jarhodes314 opened this issue Nov 29, 2023 · 3 comments
Assignees
Labels
improvement User value
Milestone

Comments

@jarhodes314
Copy link
Contributor

Is your feature improvement request related to a problem? Please describe.
In #2474, I modified the code to debug print some values in errors that later get turned into failure reasons for operations. I was surprised to discover that the mapper removes quotes from failure reasons when serialising them to SmartREST, so this didn't work as intended.

Describe the solution you'd like
Quotes in the error message should be preserved when they are sent to Cumulocity.

Describe alternatives you've considered

Additional context

@rina23q
Copy link
Member

rina23q commented Nov 29, 2023

Double quotes in failure reason must be escaped by two double quotes like below.

502,c8y_UploadConfigFile,"contains""double""quote"

This sanitize function escapes double quotes by two double quotes, so maybe our failure reason generation doesn't use this function?

pub fn sanitize_for_smartrest(input: Vec<u8>, max_size: usize) -> String {
String::from_utf8(input)
.unwrap_or_else(|err| {
error!("The input contains invalid UTF-8: {err}");
String::default()
})
.chars()
.filter(|&c| c == '\r' || c == '\n' || c == '\t' || !c.is_control())
.collect::<String>()
.replace('"', "\"\"")

@jarhodes314
Copy link
Contributor Author

@gligorisaev I'm hoping the examples from the robot test changes in https://github.com/thin-edge/thin-edge.io/pull/2493/files are sufficient for you to understand what this change affects in tedge and to reproduce it.

@gligorisaev
Copy link
Contributor

QA has thoroughly checked the feature and here are the results:

  • Test for ticket exists in the test suite.
  • tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot
  • tests/RobotFramework/tests/cumulocity/configuration/configuration_with_file_transfer_https.robot
  • tests/RobotFramework/tests/cumulocity/log/log_operation.robot
  • QA has tested the function and it's functioning according description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement User value
Projects
None yet
Development

No branches or pull requests

4 participants