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

Parse JSON blobs more leniently #5502

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
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
77 changes: 74 additions & 3 deletions mullvad-daemon/src/settings/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//! Attempting to edit prohibited or invalid settings results in an error.
//! 2. Merging the changes. When the patch has been accepted, it can be applied to the existing
//! settings. How they're merged depends on the actual setting. See [MergeStrategy].
//! 3. Deserialize the resulting JSON back to a [Settings] instance, and, if valid, replace the
//! 3. Deserializing the resulting JSON back to a [Settings] instance, and, if valid, replacing the
//! existing settings.
//!
//! Permitted settings and merge strategies are defined in the [PERMITTED_SUBKEYS] constant.
Expand Down Expand Up @@ -133,8 +133,8 @@ pub async fn merge_validate_patch(
) -> Result<(), Error> {
let mut settings_value: serde_json::Value =
serde_json::to_value(settings.to_settings()).map_err(Error::SerializeSettings)?;
let patch_value: serde_json::Value =
serde_json::from_str(json_patch).map_err(Error::ParsePatch)?;

let patch_value = fixup_patch(json_patch)?;

validate_patch_value(PERMITTED_SUBKEYS, &patch_value, 0)?;
merge_patch_to_value(PERMITTED_SUBKEYS, &mut settings_value, &patch_value, 0)?;
Expand All @@ -150,6 +150,30 @@ pub async fn merge_validate_patch(
Ok(())
}

/// Parse JSON input with some extra leniency.
fn fixup_patch(json_patch: &str) -> Result<serde_json::Value, Error> {
let fixed_patch = add_missing_braces(&remove_comments(json_patch));
serde_json::from_str(&fixed_patch).map_err(Error::ParsePatch)
}

/// Remove lines beginning with '#'
fn remove_comments(json_patch: &str) -> String {
json_patch
.lines()
.filter(|line| !line.trim_start().starts_with('#'))
.collect::<Vec<_>>()
.join("\n")
}

/// Wrap input in {} if it isn't already
fn add_missing_braces(json_patch: &str) -> String {
let mut patch = json_patch.trim_start().to_owned();
if patch.get(0..1) != Some("{") {
patch = format!("{{\n{patch}\n}}");
}
patch
}

/// Replace overrides for existing values in the array if there's a matching hostname. For hostnames
/// that do not exist, just append the overrides.
fn merge_relay_overrides(
Expand Down Expand Up @@ -301,6 +325,53 @@ fn validate_patch_value(
}
}

#[test]
fn test_braces_json() {
let input = r#"
"a": [
{
"b": 1,
"c": 2
},
{}
]
"#;
let expected = r#"
{
"a": [
{
"b": 1,
"c": 2
},
{}
]
}
"#;

let fixed_json = add_missing_braces(input);

let expected_parsed: serde_json::Value = serde_json::from_str(expected).unwrap();
let trimmed_parsed: serde_json::Value = serde_json::from_str(&fixed_json).unwrap();

assert_eq!(trimmed_parsed, expected_parsed);
}

#[test]
fn test_comment_json() {
let input = r#"
# This is a comment!
{}
"#;
let expected = "{}";

let fixed_json = remove_comments(input);

let expected_parsed: serde_json::Value = serde_json::from_str(expected).unwrap();
let trimmed_parsed: serde_json::Value = serde_json::from_str(&fixed_json).unwrap();

assert_eq!(trimmed_parsed, expected_parsed);
}

#[test]
fn test_permitted_value() {
const PERMITTED_SUBKEYS: &PermittedKey = &PermittedKey::object(&[(
Expand Down
Loading