Skip to content

Commit

Permalink
Xml errors fix (nushell#11487)
Browse files Browse the repository at this point in the history
# Description
Fixes nushell#11264
This PR adds checks in `to xml` to output error for malformed xml
entries:
* With columns that are not one of `tag`, `attributes` or `content`
* With no `tag` when entry is not a string
* With `tag` that is not a string
This PR also replaces `attrs` with `attributes` in example and
extra_usage of `to xml` (column was originally named attrs and renamed
to attributes, but this was missed in docs)

# User-Facing Changes
`to xml` will produce error for conditions described above instead of
silently returning nothing

# Tests + Formatting
Added tests for `to xml` to check handling of malformed xml entries
  • Loading branch information
NotLebedev committed Jan 5, 2024
1 parent 1ab9ec3 commit 5f7425a
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 6 deletions.
36 changes: 30 additions & 6 deletions crates/nu-command/src/formats/to/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ impl Command for ToXml {
fn extra_usage(&self) -> &str {
r#"Every XML entry is represented via a record with tag, attribute and content fields.
To represent different types of entries different values must be written to this fields:
1. Tag entry: `{tag: <tag name> attrs: {<attr name>: "<string value>" ...} content: [<entries>]}`
2. Comment entry: `{tag: '!' attrs: null content: "<comment string>"}`
3. Processing instruction (PI): `{tag: '?<pi name>' attrs: null content: "<pi content string>"}`
4. Text: `{tag: null attrs: null content: "<text>"}`. Or as plain `<text>` instead of record.
1. Tag entry: `{tag: <tag name> attributes: {<attr name>: "<string value>" ...} content: [<entries>]}`
2. Comment entry: `{tag: '!' attributes: null content: "<comment string>"}`
3. Processing instruction (PI): `{tag: '?<pi name>' attributes: null content: "<pi content string>"}`
4. Text: `{tag: null attributes: null content: "<text>"}`. Or as plain `<text>` instead of record.
Additionally any field which is: empty record, empty list or null, can be omitted."#
}
Expand All @@ -46,7 +46,7 @@ Additionally any field which is: empty record, empty list or null, can be omitte
vec![
Example {
description: "Outputs an XML string representing the contents of this table",
example: r#"{tag: note attributes: {} content : [{tag: remember attributes: {} content : [{tag: null attrs: null content : Event}]}]} | to xml"#,
example: r#"{tag: note attributes: {} content : [{tag: remember attributes: {} content : [{tag: null attributes: null content : Event}]}]} | to xml"#,
result: Some(Value::test_string(
"<note><remember>Event</remember></note>",
)),
Expand Down Expand Up @@ -110,6 +110,17 @@ fn to_xml_entry<W: Write>(
}

if let Value::Record { val: record, .. } = &entry {
if let Some(bad_column) = find_invalid_column(record) {
return Err(ShellError::CantConvert {
to_type: "XML".into(),
from_type: "record".into(),
span: entry_span,
help: Some(format!(
"Invalid column \"{}\" in xml entry. Only \"{}\", \"{}\" and \"{}\" are permitted",
bad_column, COLUMN_TAG_NAME, COLUMN_ATTRS_NAME, COLUMN_CONTENT_NAME
)),
});
}
// If key is not found it is assumed to be nothing. This way
// user can write a tag like {tag: a content: [...]} instead
// of longer {tag: a attributes: {} content: [...]}
Expand Down Expand Up @@ -144,7 +155,12 @@ fn to_xml_entry<W: Write>(
(Value::String { val: tag_name, .. }, attrs, children) => to_tag_like(
entry_span, tag_name, tag_span, attrs, children, top_level, writer,
),
_ => Ok(()),
_ => Err(ShellError::CantConvert {
to_type: "XML".into(),
from_type: "record".into(),
span: entry_span,
help: Some("Tag missing or is not a string".into()),
}),
}
} else {
Err(ShellError::CantConvert {
Expand All @@ -156,6 +172,14 @@ fn to_xml_entry<W: Write>(
}
}

fn find_invalid_column(record: &Record) -> Option<&String> {
const VALID_COLS: [&str; 3] = [COLUMN_TAG_NAME, COLUMN_ATTRS_NAME, COLUMN_CONTENT_NAME];
record
.cols
.iter()
.find(|col| !VALID_COLS.contains(&col.as_str()))
}

/// Convert record to tag-like entry: tag, PI, comment.
fn to_tag_like<W: Write>(
entry_span: Span,
Expand Down
36 changes: 36 additions & 0 deletions crates/nu-command/tests/format_conversions/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,39 @@ fn table_to_xml_text_and_from_xml_text_back_into_table() {

assert_eq!(actual.out, "true");
}

#[test]
fn to_xml_error_unknown_column() {
let actual = nu!(
cwd: "tests/fixtures/formats", pipeline(
r#"
{tag: a bad_column: b} | to xml
"#
));

assert!(actual.err.contains("Invalid column \"bad_column\""));
}

#[test]
fn to_xml_error_no_tag() {
let actual = nu!(
cwd: "tests/fixtures/formats", pipeline(
r#"
{attributes: {a: b c: d}} | to xml
"#
));

assert!(actual.err.contains("Tag missing"));
}

#[test]
fn to_xml_error_tag_not_string() {
let actual = nu!(
cwd: "tests/fixtures/formats", pipeline(
r#"
{tag: 1 attributes: {a: b c: d}} | to xml
"#
));

assert!(actual.err.contains("not a string"));
}

0 comments on commit 5f7425a

Please sign in to comment.