Skip to content
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed a bug in YAML and TOML parsers that caused them to fail when parsing non-base10 numbers (e.g. hex, binary, octal).
- Fixed a bug in the `toInt` function that caused it to fail when parsing non-base10 numbers (e.g. hex, binary, octal).
- XML child element ordering now has more comprehensive round-trip handling. Thanks @takeokunn.
- XML writer now validates element and attribute names per the XML 1.0 spec, returning a clear error for invalid names instead of producing malformed XML. Thanks @lawrence3699.

## [v3.4.1] - 2026-03-30

Expand Down
60 changes: 59 additions & 1 deletion parsing/xml/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,62 @@ import (
"encoding/xml"
"fmt"
"strings"
"unicode/utf8"

"github.com/tomwright/dasel/v3/model"
"github.com/tomwright/dasel/v3/parsing"
)

// isValidXMLNameStartChar reports whether r is a valid first character of an
// XML Name, per the XML 1.0 (Fifth Edition) specification §2.3.
func isValidXMLNameStartChar(r rune) bool {
return r == ':' || r == '_' ||
(r >= 'A' && r <= 'Z') ||
(r >= 'a' && r <= 'z') ||
(r >= 0xC0 && r <= 0xD6) ||
(r >= 0xD8 && r <= 0xF6) ||
(r >= 0xF8 && r <= 0x2FF) ||
(r >= 0x370 && r <= 0x37D) ||
(r >= 0x37F && r <= 0x1FFF) ||
(r >= 0x200C && r <= 0x200D) ||
(r >= 0x2070 && r <= 0x218F) ||
(r >= 0x2C00 && r <= 0x2FEF) ||
(r >= 0x3001 && r <= 0xD7FF) ||
(r >= 0xF900 && r <= 0xFDCF) ||
(r >= 0xFDF0 && r <= 0xFFFD) ||
(r >= 0x10000 && r <= 0xEFFFF)
}

// isValidXMLNameChar reports whether r is a valid subsequent character of an
// XML Name, per the XML 1.0 (Fifth Edition) specification §2.3.
func isValidXMLNameChar(r rune) bool {
return isValidXMLNameStartChar(r) ||
r == '-' || r == '.' ||
(r >= '0' && r <= '9') ||
r == 0xB7 ||
(r >= 0x0300 && r <= 0x036F) ||
(r >= 0x203F && r <= 0x2040)
}

// isValidXMLName reports whether s is a valid XML element or attribute name.
func isValidXMLName(s string) bool {
if s == "" {
return false
}
first, size := utf8.DecodeRuneInString(s)
if (first == utf8.RuneError && size == 1) || !isValidXMLNameStartChar(first) {
return false
}
for i := size; i < len(s); {
r, w := utf8.DecodeRuneInString(s[i:])
if (r == utf8.RuneError && w == 1) || !isValidXMLNameChar(r) {
return false
}
i += w
}
return true
}

func newXMLWriter(options parsing.WriterOptions) (parsing.Writer, error) {
return &xmlWriter{
options: options,
Expand Down Expand Up @@ -60,6 +111,9 @@ func (j *xmlWriter) Write(value *model.Value) ([]byte, error) {
}

func (j *xmlWriter) toElement(key string, value *model.Value) (*xmlElement, error) {
if !isValidXMLName(key) {
return nil, fmt.Errorf("key %q is not a valid XML element name", key)
}
readProcessingInstructions := func() []*xmlProcessingInstruction {
if piMeta, ok := value.MetadataValue("xml_processing_instructions"); ok && piMeta != nil {
if pis, ok := piMeta.([]*xmlProcessingInstruction); ok {
Expand Down Expand Up @@ -151,8 +205,12 @@ func (j *xmlWriter) toElement(key string, value *model.Value) (*xmlElement, erro
func extractAttrsAndText(kvs []model.KeyValue, el *xmlElement) error {
for _, kv := range kvs {
if strings.HasPrefix(kv.Key, "-") {
attrName := kv.Key[1:]
if !isValidXMLName(attrName) {
return fmt.Errorf("invalid XML attribute name %q from map key %q", attrName, kv.Key)
}
Comment on lines 207 to +211
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

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

New behavior rejects invalid attribute names, but there isn’t a test that exercises this error path (e.g. a map containing an attribute key like "-<" or "-foo bar"). Adding an integration test for invalid attribute names would help prevent regressions and ensure the writer fails with a clear error when encountering invalid attribute keys.

Copilot uses AI. Check for mistakes.
attr := xmlAttr{
Name: kv.Key[1:],
Name: attrName,
}
var err error
attr.Value, err = valueToString(kv.Value)
Expand Down
17 changes: 17 additions & 0 deletions parsing/xml/writer_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,23 @@ func TestXmlWriter_CommentValidation(t *testing.T) {
})
}

// Test_isValidXMLName tests XML name validation per the XML 1.0 spec.
func Test_isValidXMLName(t *testing.T) {
valid := []string{"foo", "Foo", "_bar", "ns:local", "a1", "a-b", "a.b", "über"}
for _, name := range valid {
if !isValidXMLName(name) {
t.Errorf("Expected %q to be a valid XML name", name)
}
}

invalid := []string{"", "<", ">", "&", "foo bar", "1abc", "-abc", ".abc", "a<b", "a>b", "a&b"}
for _, name := range invalid {
if isValidXMLName(name) {
t.Errorf("Expected %q to be an invalid XML name", name)
}
}
}

// Test_valueToString tests the valueToString function for all supported types
func Test_valueToString(t *testing.T) {
t.Run("null value returns empty string", func(t *testing.T) {
Expand Down
70 changes: 70 additions & 0 deletions parsing/xml/writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,76 @@ func TestXmlReader_Write(t *testing.T) {
}
})

t.Run("invalid element name", func(t *testing.T) {
w, err := xml.XML.NewWriter(parsing.DefaultWriterOptions())
if err != nil {
t.Fatalf("Unexpected error: %s", err)
}

toEncode := model.NewMapValue()
_ = toEncode.SetMapKey("<", model.NewStringValue("value"))
_, err = w.Write(toEncode)
if err == nil {
t.Fatal("Expected error for invalid XML element name, got nil")
}
if !strings.Contains(err.Error(), `"<"`) || !strings.Contains(err.Error(), "not a valid XML element name") {
t.Fatalf("Expected error to mention the offending key, got: %s", err)
}
})
Comment on lines +263 to +278
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

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

These tests only assert that an error occurs, but they don’t verify the error message includes the offending key and indicates it’s an element-name validation failure. Since the PR’s goal is to return a clear error for invalid XML names, asserting on key/message content would help keep the error user-friendly over time.

Copilot uses AI. Check for mistakes.

t.Run("invalid element name ampersand", func(t *testing.T) {
w, err := xml.XML.NewWriter(parsing.DefaultWriterOptions())
if err != nil {
t.Fatalf("Unexpected error: %s", err)
}

toEncode := model.NewMapValue()
_ = toEncode.SetMapKey("&", model.NewStringValue("value"))
_, err = w.Write(toEncode)
if err == nil {
t.Fatal("Expected error for invalid XML element name, got nil")
}
if !strings.Contains(err.Error(), `"&"`) || !strings.Contains(err.Error(), "not a valid XML element name") {
t.Fatalf("Expected error to mention the offending key, got: %s", err)
}
})

t.Run("invalid element name with space", func(t *testing.T) {
w, err := xml.XML.NewWriter(parsing.DefaultWriterOptions())
if err != nil {
t.Fatalf("Unexpected error: %s", err)
}

toEncode := model.NewMapValue()
_ = toEncode.SetMapKey("foo bar", model.NewStringValue("value"))
_, err = w.Write(toEncode)
if err == nil {
t.Fatal("Expected error for invalid XML element name, got nil")
}
if !strings.Contains(err.Error(), `"foo bar"`) || !strings.Contains(err.Error(), "not a valid XML element name") {
t.Fatalf("Expected error to mention the offending key, got: %s", err)
}
})

t.Run("invalid attribute name", func(t *testing.T) {
w, err := xml.XML.NewWriter(parsing.DefaultWriterOptions())
if err != nil {
t.Fatalf("Unexpected error: %s", err)
}

toEncode := model.NewMapValue()
child := model.NewMapValue()
_ = child.SetMapKey("-<", model.NewStringValue("value"))
_ = toEncode.SetMapKey("foo", child)
_, err = w.Write(toEncode)
if err == nil {
t.Fatal("Expected error for invalid XML attribute name, got nil")
}
if !strings.Contains(err.Error(), "invalid XML attribute name") || !strings.Contains(err.Error(), `"-<"`) {
t.Fatalf("Expected error to mention the offending attribute key, got: %s", err)
}
})

t.Run("encode cdata", func(t *testing.T) {
w, err := xml.XML.NewWriter(parsing.DefaultWriterOptions())
if err != nil {
Expand Down
Loading