diff --git a/cmd/dataxl/conversion.go b/cmd/dataxl/conversion.go index 7514300..8c499ff 100644 --- a/cmd/dataxl/conversion.go +++ b/cmd/dataxl/conversion.go @@ -22,13 +22,21 @@ func convert(input []byte, from, to, sheet string, pretty bool) ([]byte, error) if err != nil { return nil, err } - return encodeStructured(tableToRecords(t), to, pretty) + records, err := tableToRecords(t) + if err != nil { + return nil, err + } + return encodeStructured(records, to, pretty) case isTabular(to): value, err := parseStructured(input, from) if err != nil { return nil, err } - return encodeTable(valueToTable(value), to, sheet) + t := valueToTable(value) + if len(t.Header) == 0 && len(t.Rows) > 0 { + return nil, fmt.Errorf("structured value has records but no scalar fields to represent as a table") + } + return encodeTable(t, to, sheet) default: value, err := parseStructured(input, from) if err != nil { diff --git a/cmd/dataxl/conversion_matrix_test.go b/cmd/dataxl/conversion_matrix_test.go new file mode 100644 index 0000000..fe19c64 --- /dev/null +++ b/cmd/dataxl/conversion_matrix_test.go @@ -0,0 +1,64 @@ +package main + +import ( + "reflect" + "testing" +) + +func TestConversionMatrixPreservesRepresentativeTable(t *testing.T) { + formats := []string{"json", "yaml", "toml", "csv", "tsv", "xlsx"} + records := []any{ + map[string]any{ + "active": true, + "id": int64(1), + "name": "Alice", + "note": " first line\nsecond line ", + }, + map[string]any{ + "active": false, + "id": int64(2), + "name": "ボブ", + "note": "comma, tab\t and quote \"", + }, + } + want := valueToTable(records) + + fixtures := make(map[string][]byte, len(formats)) + for _, format := range formats { + var ( + data []byte + err error + ) + if isTabular(format) { + data, err = encodeTable(want, format, "Sheet1") + } else { + data, err = encodeStructured(records, format, true) + } + if err != nil { + t.Fatalf("build %s fixture: %v", format, err) + } + fixtures[format] = data + } + + for _, from := range formats { + for _, to := range formats { + t.Run(from+"_to_"+to, func(t *testing.T) { + converted, err := convert(fixtures[from], from, to, "Sheet1", true) + if err != nil { + t.Fatalf("convert %s -> %s: %v", from, to, err) + } + normalized, err := convert(converted, to, "tsv", "Sheet1", false) + if err != nil { + t.Fatalf("normalize %s output through TSV: %v", to, err) + } + got, err := parseTable(normalized, "tsv", "Sheet1") + if err != nil { + t.Fatalf("parse normalized TSV: %v", err) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("%s -> %s table = %#v, want %#v", from, to, got, want) + } + }) + } + } +} diff --git a/cmd/dataxl/data_integrity_test.go b/cmd/dataxl/data_integrity_test.go new file mode 100644 index 0000000..c572bf9 --- /dev/null +++ b/cmd/dataxl/data_integrity_test.go @@ -0,0 +1,373 @@ +package main + +import ( + "bytes" + "encoding/json" + "fmt" + "reflect" + "strings" + "testing" +) + +func TestDelimitedInputPreservesCellWhitespace(t *testing.T) { + tests := []struct { + format string + input string + }{ + {format: "csv", input: "name,note\nAlice, keep both sides \n"}, + {format: "tsv", input: "name\tnote\nAlice\t keep both sides \n"}, + } + for _, tc := range tests { + t.Run(tc.format, func(t *testing.T) { + got, err := convert([]byte(tc.input), tc.format, "json", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + var records []map[string]any + if err := json.Unmarshal(got, &records); err != nil { + t.Fatal(err) + } + if value := records[0]["note"]; value != " keep both sides " { + t.Fatalf("note = %#v, want surrounding whitespace preserved", value) + } + }) + } +} + +func TestDelimitedInputStripsUTF8BOMFromHeader(t *testing.T) { + tests := []struct { + format string + delimiter string + }{ + {format: "csv", delimiter: ","}, + {format: "tsv", delimiter: "\t"}, + } + for _, tc := range tests { + t.Run(tc.format, func(t *testing.T) { + input := "\ufeffname" + tc.delimiter + "active\nAlice" + tc.delimiter + "true\n" + got, err := convert([]byte(input), tc.format, "json", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + var records []map[string]any + if err := json.Unmarshal(got, &records); err != nil { + t.Fatal(err) + } + want := []map[string]any{{"name": "Alice", "active": true}} + if !reflect.DeepEqual(records, want) { + t.Fatalf("records = %#v, want %#v", records, want) + } + }) + } +} + +func TestRecordsFromValuePreservesWrapperSiblings(t *testing.T) { + for _, wrapper := range []string{"items", "records", "rows"} { + t.Run(wrapper, func(t *testing.T) { + value := map[string]any{ + "metadata": "keep me", + wrapper: []any{map[string]any{"id": json.Number("1")}}, + } + got := recordsFromValue(value) + want := []any{value} + if !reflect.DeepEqual(got, want) { + t.Fatalf("records = %#v, want complete object %#v", got, want) + } + }) + } +} + +func TestRecordsFromValueUnwrapsSoleWrapper(t *testing.T) { + rows := []any{map[string]any{"id": json.Number("1")}} + for _, wrapper := range []string{"items", "records", "rows"} { + t.Run(wrapper, func(t *testing.T) { + if got := recordsFromValue(map[string]any{wrapper: rows}); !reflect.DeepEqual(got, rows) { + t.Fatalf("records = %#v, want sole wrapper rows %#v", got, rows) + } + }) + } +} + +func TestParseStructuredRejectsTrailingJSONValue(t *testing.T) { + _, err := parseStructured([]byte(`{"first":1} {"second":2}`), "json") + if err == nil || !strings.Contains(err.Error(), "multiple JSON values") { + t.Fatalf("error = %v, want multiple JSON values", err) + } +} + +func TestParseStructuredRejectsDuplicateJSONKeys(t *testing.T) { + for _, input := range []string{ + `{"id":1,"id":2}`, + `{"nested":{"name":"first","name":"second"}}`, + } { + if _, err := parseStructured([]byte(input), "json"); err == nil || !strings.Contains(err.Error(), "duplicate JSON object key") { + t.Fatalf("error = %v for %s, want duplicate-key error", err, input) + } + } +} + +func TestParseStructuredRejectsInvalidJSONUTF8(t *testing.T) { + input := []byte{'{', '"', 'k', 'e', 'y', '"', ':', '"', 0xff, '"', '}'} + if _, err := parseStructured(input, "json"); err == nil || !strings.Contains(err.Error(), "not valid UTF-8") { + t.Fatalf("error = %v, want UTF-8 error", err) + } +} + +func TestJSONParserPreservesEmptyArrayAndLargeNumber(t *testing.T) { + largeNumber := strings.Repeat("9", 400) + for _, input := range []string{"[]", `{"value":` + largeNumber + `}`} { + got, err := convert([]byte(input), "json", "json", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + if string(got) != input { + t.Fatalf("JSON = %s, want %s", got, input) + } + } +} + +func TestParseStructuredPreservesEveryYAMLDocument(t *testing.T) { + got, err := parseStructured([]byte("first: 1\n---\nsecond: 2\n"), "yaml") + if err != nil { + t.Fatal(err) + } + want := []any{ + map[string]any{"first": 1}, + map[string]any{"second": 2}, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("documents = %#v, want %#v", got, want) + } +} + +func TestParseStructuredAllowsTrailingWhitespace(t *testing.T) { + for _, tc := range []struct { + format string + input string + }{ + {format: "json", input: "{\"name\":\"Alice\"}\n\t "}, + {format: "yaml", input: "name: Alice\n\n"}, + } { + t.Run(tc.format, func(t *testing.T) { + if _, err := parseStructured([]byte(tc.input), tc.format); err != nil { + t.Fatalf("valid input rejected: %v", err) + } + }) + } +} + +func TestWrapperSiblingsSurviveJSONTSVRoundTrip(t *testing.T) { + input := []byte(`{"metadata":"keep me","items":[{"id":1},{"id":2}]}`) + tsv, err := convert(input, "json", "tsv", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + if lines := bytes.Count(tsv, []byte{'\n'}); lines != 2 { + t.Fatalf("TSV has %d lines, want header plus one complete-object row:\n%s", lines, tsv) + } + for _, column := range []string{"items[0].id", "items[1].id", "metadata"} { + if !bytes.Contains(tsv, []byte(column)) { + t.Fatalf("TSV missing %q:\n%s", column, tsv) + } + } + + restored, err := convert(tsv, "tsv", "json", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + var records []map[string]any + if err := json.Unmarshal(restored, &records); err != nil { + t.Fatal(err) + } + if got := fmt.Sprint(records[0]["metadata"]); got != "keep me" { + t.Fatalf("metadata = %q after round trip", got) + } +} + +func TestDelimitedInputRejectsRowsWiderThanHeader(t *testing.T) { + _, err := convert([]byte("a,b\n1,2,3\n"), "csv", "json", "Sheet1", false) + if err == nil || !strings.Contains(err.Error(), "row 2 has 3 fields") { + t.Fatalf("error = %v, want wider-row error", err) + } +} + +func TestDelimitedInputRejectsInvalidUTF8(t *testing.T) { + input := []byte{'n', 'a', 'm', 'e', '\n', 0xff, '\n'} + _, err := convert(input, "csv", "json", "Sheet1", false) + if err == nil || !strings.Contains(err.Error(), "not valid UTF-8") { + t.Fatalf("error = %v, want UTF-8 error", err) + } +} + +func TestDelimitedSingleEmptyCellRowRoundTrip(t *testing.T) { + want := table{Header: []string{"value"}, Rows: [][]string{{""}}} + for _, format := range []string{"csv", "tsv"} { + t.Run(format, func(t *testing.T) { + encoded, err := encodeTable(want, format, "Sheet1") + if err != nil { + t.Fatal(err) + } + got, err := parseTable(encoded, format, "Sheet1") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("round trip = %#v, want %#v; encoded %q", got, want, encoded) + } + }) + } +} + +func TestStructuredRecordsWithoutScalarFieldsAreRejectedForTables(t *testing.T) { + for _, input := range []string{`{}`, `[{}]`} { + for _, format := range []string{"csv", "tsv", "xlsx"} { + t.Run(format+"_"+input, func(t *testing.T) { + _, err := convert([]byte(input), "json", format, "Sheet1", false) + if err == nil || !strings.Contains(err.Error(), "no scalar fields") { + t.Fatalf("error = %v, want unrepresentable-table error", err) + } + }) + } + } +} + +func TestEmptyRecordListCanRoundTripThroughDelimitedFormats(t *testing.T) { + for _, format := range []string{"csv", "tsv"} { + t.Run(format, func(t *testing.T) { + tabular, err := convert([]byte(`[]`), "json", format, "Sheet1", false) + if err != nil { + t.Fatal(err) + } + got, err := convert(tabular, format, "json", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + if string(got) != `[]` { + t.Fatalf("round trip = %s, want []", got) + } + }) + } +} + +func TestTableToRecordsRejectsColumnsThatWouldLoseData(t *testing.T) { + tests := []struct { + name string + table table + want string + }{ + { + name: "duplicate header", + table: table{Header: []string{"id", "id"}, Rows: [][]string{{"1", "2"}}}, + want: "duplicate header", + }, + { + name: "blank header with value", + table: table{Header: []string{"id", ""}, Rows: [][]string{{"1", "orphan"}}}, + want: "header is blank", + }, + { + name: "parent and child conflict", + table: table{Header: []string{"user", "user.name"}, Rows: [][]string{{"Alice", "Bob"}}}, + want: "expected an object", + }, + { + name: "object and array conflict", + table: table{Header: []string{"value.name", "value[0]"}, Rows: [][]string{{"Alice", "Bob"}}}, + want: "expected an array", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := tableToRecords(tc.table) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("error = %v, want one containing %q", err, tc.want) + } + }) + } +} + +func TestNestedArraysSurviveJSONTSVRoundTrip(t *testing.T) { + input := []byte(`{"matrix":[[1,2],[3,4]]}`) + tsv, err := convert(input, "json", "tsv", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + restored, err := convert(tsv, "tsv", "json", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + var got []map[string]any + if err := json.Unmarshal(restored, &got); err != nil { + t.Fatal(err) + } + want := []map[string]any{{ + "matrix": []any{ + []any{float64(1), float64(2)}, + []any{float64(3), float64(4)}, + }, + }} + if !reflect.DeepEqual(got, want) { + t.Fatalf("round trip = %#v, want %#v\nTSV:\n%s", got, want, tsv) + } +} + +func TestDelimiterKeysSurviveJSONTSVRoundTrip(t *testing.T) { + input := []byte(`{"a.b":{"x[y]":1,"":2},"plain":3}`) + tsv, err := convert(input, "json", "tsv", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + parsed, err := parseTable(tsv, "tsv", "Sheet1") + if err != nil { + t.Fatal(err) + } + for _, header := range []string{`["a.b"][""]`, `["a.b"]["x[y]"]`, "plain"} { + if !containsString(parsed.Header, header) { + t.Fatalf("TSV missing escaped header %q:\n%s", header, tsv) + } + } + restored, err := convert(tsv, "tsv", "json", "Sheet1", false) + if err != nil { + t.Fatal(err) + } + var got []map[string]any + if err := json.Unmarshal(restored, &got); err != nil { + t.Fatal(err) + } + want := []map[string]any{{ + "a.b": map[string]any{"x[y]": float64(1), "": float64(2)}, + "plain": float64(3), + }} + if !reflect.DeepEqual(got, want) { + t.Fatalf("round trip = %#v, want %#v\nTSV:\n%s", got, want, tsv) + } +} + +func containsString(values []string, target string) bool { + for _, value := range values { + if value == target { + return true + } + } + return false +} + +func TestParsePathRejectsMalformedHeaders(t *testing.T) { + tooDeep := "root" + strings.Repeat(".child", maxPathTokens) + for _, path := range []string{ + "a..b", + "a[-1]", + "a[foo]", + "a[0", + "a]", + `["unterminated]`, + fmt.Sprintf("a[%d]", maxArrayIndex+1), + tooDeep, + } { + t.Run(path, func(t *testing.T) { + if _, err := parsePath(path); err == nil { + t.Fatalf("parsePath(%q) succeeded", path) + } + }) + } +} diff --git a/cmd/dataxl/fuzz_test.go b/cmd/dataxl/fuzz_test.go new file mode 100644 index 0000000..412fef8 --- /dev/null +++ b/cmd/dataxl/fuzz_test.go @@ -0,0 +1,90 @@ +package main + +import ( + "encoding/json" + "reflect" + "strings" + "testing" + "unicode/utf8" +) + +func FuzzPathKeyRoundTrip(f *testing.F) { + for _, seed := range []string{"name", "", "a.b", "x[y]", "日本語", " leading ", "quote\"slash\\"} { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, key string) { + if !utf8.ValidString(key) { + t.Skip() + } + for _, prefix := range []string{"", "root"} { + path := appendPathKey(prefix, key) + tokens, err := parsePath(path) + if err != nil { + t.Fatalf("parse generated path %q: %v", path, err) + } + last := tokens[len(tokens)-1] + if last.isIndex || last.key != key { + t.Fatalf("path %q decoded final token %#v, want key %q", path, last, key) + } + } + }) +} + +func FuzzParseJSON(f *testing.F) { + for _, seed := range [][]byte{ + []byte(`{"name":"Alice","items":[1,2,null]}`), + []byte(`{"duplicate":1,"duplicate":2}`), + []byte(`[]`), + []byte(`{"a":1} {"b":2}`), + {0xff}, + } { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, input []byte) { + if len(input) > 64*1024 { + t.Skip() + } + value, err := parseStructured(input, "json") + if err != nil { + return + } + encoded, err := json.Marshal(value) + if err != nil { + t.Fatalf("marshal accepted value: %v", err) + } + if !json.Valid(encoded) { + t.Fatalf("parser returned value that encoded as invalid JSON") + } + }) +} + +func FuzzDelimitedTableRoundTrip(f *testing.F) { + for _, seed := range []string{ + "plain", + " surrounding spaces ", + "comma, tab\t quote\"", + "first line\nsecond line", + "日本語と絵文字🌙", + } { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, value string) { + if !utf8.ValidString(value) || strings.Contains(value, "\r\n") { + t.Skip() + } + want := table{Header: []string{"value"}, Rows: [][]string{{value}}} + for _, format := range []string{"csv", "tsv"} { + encoded, err := encodeTable(want, format, "Sheet1") + if err != nil { + t.Fatalf("encode %s: %v", format, err) + } + got, err := parseTable(encoded, format, "Sheet1") + if err != nil { + t.Fatalf("parse %s: %v", format, err) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("%s round trip = %#v, want %#v", format, got, want) + } + } + }) +} diff --git a/cmd/dataxl/main.go b/cmd/dataxl/main.go index cb46e84..d6b29c9 100644 --- a/cmd/dataxl/main.go +++ b/cmd/dataxl/main.go @@ -5,9 +5,12 @@ import ( "fmt" "io" "os" + "runtime/debug" "strings" ) +var buildVersion = "dev" + // options contains CLI concerns only. Conversion functions receive explicit // arguments so they can be reused and tested without constructing a FlagSet. type options struct { @@ -17,6 +20,7 @@ type options struct { to string sheet string pretty bool + version bool } func main() { @@ -31,6 +35,10 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) error { if err != nil { return err } + if opt.version { + _, err := fmt.Fprintln(stdout, "dataxl", resolvedVersion()) + return err + } if err := opt.resolveFormats(); err != nil { return err } @@ -56,6 +64,7 @@ func parseOptions(args []string, stderr io.Writer) (options, error) { fs.StringVar(&opt.to, "to", "", "output format: json, yaml, toml, csv, tsv, xlsx") fs.StringVar(&opt.sheet, "sheet", "Sheet1", "worksheet name for xlsx input/output") fs.BoolVar(&opt.pretty, "pretty", true, "pretty-print structured output") + fs.BoolVar(&opt.version, "version", false, "print version and exit") fs.Usage = func() { _, _ = fmt.Fprintln(stderr, `Usage: dataxl -from yaml -to tsv -i input.yaml -o output.tsv @@ -79,6 +88,17 @@ Notes: return opt, nil } +func resolvedVersion() string { + if buildVersion != "dev" { + return buildVersion + } + info, ok := debug.ReadBuildInfo() + if ok && info.Main.Version != "" && info.Main.Version != "(devel)" { + return info.Main.Version + } + return buildVersion +} + func (opt *options) resolveFormats() error { opt.from = normalizeFormat(opt.from) opt.to = normalizeFormat(opt.to) diff --git a/cmd/dataxl/main_test.go b/cmd/dataxl/main_test.go index c5733d0..7021b18 100644 --- a/cmd/dataxl/main_test.go +++ b/cmd/dataxl/main_test.go @@ -129,6 +129,21 @@ func TestResolveFormatsFromFileExtensions(t *testing.T) { } } +func TestVersionFlagDoesNotRequireFormats(t *testing.T) { + oldVersion := buildVersion + buildVersion = "v9.8.7-test" + t.Cleanup(func() { buildVersion = oldVersion }) + + var out bytes.Buffer + var errOut bytes.Buffer + if err := run([]string{"-version"}, nil, &out, &errOut); err != nil { + t.Fatalf("run failed: %v\nstderr: %s", err, errOut.String()) + } + if got, want := out.String(), "dataxl v9.8.7-test\n"; got != want { + t.Fatalf("version output = %q, want %q", got, want) + } +} + func TestConvertStructuredToStructured(t *testing.T) { got, err := convert([]byte("name: Alice\nactive: true\n"), "yaml", "json", "Sheet1", false) if err != nil { @@ -140,10 +155,13 @@ func TestConvertStructuredToStructured(t *testing.T) { } func TestRowsToTablePadsRaggedRows(t *testing.T) { - got := rowsToTable([][]string{{"a", "b"}, {"1"}, {"2", "3", "4"}}) + got, err := rowsToTable([][]string{{"a", "b"}, {"1"}, {"2", "3"}}) + if err != nil { + t.Fatal(err) + } want := table{ - Header: []string{"a", "b", ""}, - Rows: [][]string{{"1", "", ""}, {"2", "3", "4"}}, + Header: []string{"a", "b"}, + Rows: [][]string{{"1", ""}, {"2", "3"}}, } if !reflect.DeepEqual(got, want) { t.Fatalf("table = %#v, want %#v", got, want) @@ -152,13 +170,16 @@ func TestRowsToTablePadsRaggedRows(t *testing.T) { func TestParseCellConservativeInference(t *testing.T) { tests := map[string]any{ - "": "", - "true": true, - "42": int64(42), - "3.14": 3.14, - "00123": "00123", - "1e3": 1000.0, - "Alice": "Alice", + "": "", + "true": true, + "42": int64(42), + "3.14": 3.14, + "00123": "00123", + "1e3": 1000.0, + "Alice": "Alice", + " 42 ": " 42 ", + " true ": " true ", + " Alice ": " Alice ", } for input, want := range tests { if got := parseCell(input); !reflect.DeepEqual(got, want) { @@ -167,11 +188,15 @@ func TestParseCellConservativeInference(t *testing.T) { } } -func TestSetPathDoesNotOverwriteConflictingShape(t *testing.T) { +func TestSetPathRejectsConflictingShape(t *testing.T) { t.Run("parent scalar before child", func(t *testing.T) { record := map[string]any{} - setPath(record, "user", "Alice") - setPath(record, "user.name", "Bob") + if err := setPath(record, "user", "Alice"); err != nil { + t.Fatal(err) + } + if err := setPath(record, "user.name", "Bob"); err == nil { + t.Fatal("conflicting child path succeeded") + } if got := record["user"]; got != "Alice" { t.Fatalf("user = %#v, want original scalar", got) } @@ -179,8 +204,12 @@ func TestSetPathDoesNotOverwriteConflictingShape(t *testing.T) { t.Run("child before parent scalar", func(t *testing.T) { record := map[string]any{} - setPath(record, "user.name", "Bob") - setPath(record, "user", "Alice") + if err := setPath(record, "user.name", "Bob"); err != nil { + t.Fatal(err) + } + if err := setPath(record, "user", "Alice"); err == nil { + t.Fatal("conflicting parent path succeeded") + } want := map[string]any{"name": "Bob"} if got := record["user"]; !reflect.DeepEqual(got, want) { t.Fatalf("user = %#v, want original nested value %#v", got, want) @@ -190,7 +219,9 @@ func TestSetPathDoesNotOverwriteConflictingShape(t *testing.T) { func TestSetPathRestoresNestedArrays(t *testing.T) { record := map[string]any{} - setPath(record, "orders[0].items[1].sku", "B-002") + if err := setPath(record, "orders[0].items[1].sku", "B-002"); err != nil { + t.Fatal(err) + } orders := record["orders"].([]any) order := orders[0].(map[string]any) diff --git a/cmd/dataxl/path.go b/cmd/dataxl/path.go index 94dd700..95ea472 100644 --- a/cmd/dataxl/path.go +++ b/cmd/dataxl/path.go @@ -1,33 +1,70 @@ package main import ( - "regexp" + "encoding/json" + "fmt" "strconv" "strings" ) -// tableToRecords interprets non-empty headers as path expressions. Blank -// headers are intentionally ignored so spare spreadsheet columns are harmless. -func tableToRecords(t table) []map[string]any { - records := make([]map[string]any, 0, len(t.Rows)) - for _, row := range t.Rows { - record := make(map[string]any) - for i, header := range t.Header { - header = strings.TrimSpace(header) - if header == "" || i >= len(row) { - continue +type parsedColumn struct { + header string + index int + tokens []pathToken +} + +// tableToRecords interprets non-empty headers as path expressions. A blank +// header is safe only when every cell below it is also blank; otherwise there +// is no key under which the value can be preserved. +func tableToRecords(t table) ([]map[string]any, error) { + columns := make([]parsedColumn, 0, len(t.Header)) + seen := make(map[string]int) + for columnIndex, header := range t.Header { + if header == "" { + for rowIndex, row := range t.Rows { + if columnIndex < len(row) && row[columnIndex] != "" { + return nil, fmt.Errorf( + "row %d column %d contains data but its header is blank", + rowIndex+2, columnIndex+1, + ) + } + } + continue + } + if firstIndex, exists := seen[header]; exists { + return nil, fmt.Errorf( + "duplicate header %q in columns %d and %d", + header, firstIndex+1, columnIndex+1, + ) + } + seen[header] = columnIndex + tokens, err := parsePath(header) + if err != nil { + return nil, fmt.Errorf("invalid header %q in column %d: %w", header, columnIndex+1, err) + } + columns = append(columns, parsedColumn{header: header, index: columnIndex, tokens: tokens}) + } + + records := make([]map[string]any, 0, len(t.Rows)) + for rowIndex, row := range t.Rows { + record := make(map[string]any) + for _, column := range columns { + cell := "" + if column.index < len(row) { + cell = row[column.index] + } + if err := setPathTokens(record, column.tokens, parseCell(cell)); err != nil { + return nil, fmt.Errorf("row %d header %q: %w", rowIndex+2, column.header, err) } - setPath(record, header, parseCell(row[i])) } records = append(records, record) } - return records + return records, nil } // parseCell deliberately performs narrow inference. In particular, strings -// such as 00123 remain strings because converting identifiers is surprising. +// with surrounding whitespace and zero-padded identifiers remain strings. func parseCell(s string) any { - s = strings.TrimSpace(s) if s == "" { return "" } @@ -49,91 +86,232 @@ func parseCell(s string) any { return s } -var pathTokenRE = regexp.MustCompile(`([^\.\[\]]+)|\[(\d+)\]`) - type pathToken struct { key string index int isIndex bool } -func parsePath(path string) []pathToken { - matches := pathTokenRE.FindAllStringSubmatch(path, -1) - tokens := make([]pathToken, 0, len(matches)) - for _, match := range matches { - if match[1] != "" { - tokens = append(tokens, pathToken{key: match[1]}) - continue - } - index, _ := strconv.Atoi(match[2]) // regexp guarantees decimal digits. - tokens = append(tokens, pathToken{index: index, isIndex: true}) +const ( + maxPathTokens = 256 + maxArrayIndex = 10_000 +) + +// parsePath accepts dotted keys, zero-based array indices, and JSON-quoted map +// keys in brackets. Quoted keys make delimiters and empty keys unambiguous: +// user.name, items[0].sku, and settings["build.target"] are all valid. +func parsePath(path string) ([]pathToken, error) { + if path == "" { + return nil, fmt.Errorf("path is empty") } - return tokens + + var tokens []pathToken + var err error + position := 0 + if strings.HasPrefix(path, `["`) { + var key string + key, position, err = parseQuotedKey(path, position) + if err != nil { + return nil, err + } + tokens = append(tokens, pathToken{key: key}) + } else { + var key string + key, position, err = parseBareKey(path, position) + if err != nil { + return nil, err + } + tokens = append(tokens, pathToken{key: key}) + } + + for position < len(path) { + switch path[position] { + case '.': + position++ + key, next, err := parseBareKey(path, position) + if err != nil { + return nil, err + } + tokens = append(tokens, pathToken{key: key}) + position = next + case '[': + if position+1 < len(path) && path[position+1] == '"' { + key, next, err := parseQuotedKey(path, position) + if err != nil { + return nil, err + } + tokens = append(tokens, pathToken{key: key}) + position = next + continue + } + index, next, err := parseIndex(path, position) + if err != nil { + return nil, err + } + tokens = append(tokens, pathToken{index: index, isIndex: true}) + position = next + default: + return nil, fmt.Errorf("unexpected character %q at byte %d", path[position], position) + } + } + if len(tokens) > maxPathTokens { + return nil, fmt.Errorf("path has %d elements; maximum is %d", len(tokens), maxPathTokens) + } + return tokens, nil } -// setPath creates maps and slices while walking a dotted/indexed header. The -// supported grammar is a sequence of map keys with optional array indices, -// e.g. "orders[0].items[1].sku". Conflicting intermediate shapes are left -// unchanged rather than silently overwriting data from an earlier column. -func setPath(root map[string]any, path string, value any) { - tokens := parsePath(path) +func parseBareKey(path string, position int) (string, int, error) { + start := position + for position < len(path) && !strings.ContainsRune(".[]", rune(path[position])) { + position++ + } + if position == start { + return "", position, fmt.Errorf("map key is empty at byte %d", start) + } + return path[start:position], position, nil +} + +func parseQuotedKey(path string, position int) (string, int, error) { + if position+1 >= len(path) || path[position] != '[' || path[position+1] != '"' { + return "", position, fmt.Errorf("quoted key expected at byte %d", position) + } + quoteStart := position + 1 + escaped := false + quoteEnd := -1 + for i := quoteStart + 1; i < len(path); i++ { + switch { + case escaped: + escaped = false + case path[i] == '\\': + escaped = true + case path[i] == '"': + quoteEnd = i + i = len(path) + } + } + if quoteEnd == -1 { + return "", position, fmt.Errorf("unterminated quoted key at byte %d", position) + } + if quoteEnd+1 >= len(path) || path[quoteEnd+1] != ']' { + return "", position, fmt.Errorf("quoted key at byte %d is missing closing ]", position) + } + var key string + if err := json.Unmarshal([]byte(path[quoteStart:quoteEnd+1]), &key); err != nil { + return "", position, fmt.Errorf("invalid quoted key at byte %d: %w", position, err) + } + return key, quoteEnd + 2, nil +} + +func parseIndex(path string, position int) (int, int, error) { + digitsStart := position + 1 + digitsEnd := digitsStart + for digitsEnd < len(path) && path[digitsEnd] >= '0' && path[digitsEnd] <= '9' { + digitsEnd++ + } + if digitsEnd == digitsStart { + return 0, position, fmt.Errorf("array index is empty or invalid at byte %d", position) + } + if digitsEnd >= len(path) || path[digitsEnd] != ']' { + return 0, position, fmt.Errorf("array index at byte %d is missing closing ]", position) + } + index, err := strconv.Atoi(path[digitsStart:digitsEnd]) + if err != nil { + return 0, position, fmt.Errorf("array index at byte %d is out of range: %w", position, err) + } + if index > maxArrayIndex { + return 0, position, fmt.Errorf("array index %d exceeds maximum %d", index, maxArrayIndex) + } + return index, digitsEnd + 1, nil +} + +// appendPathKey adds a map key to a column path. Keys containing path +// delimiters (and the empty key) use JSON bracket notation so round trips do +// not confuse literal key content with structure. +func appendPathKey(prefix, key string) string { + segment := key + quoted := key == "" || strings.ContainsAny(key, ".[]") + if quoted { + encoded, _ := json.Marshal(key) // Go strings always have a JSON representation. + segment = "[" + string(encoded) + "]" + return prefix + segment + } + if prefix == "" { + return segment + } + return prefix + "." + segment +} + +// setPath creates maps and slices while walking a dotted/indexed header. +// Conflicting shapes return an error instead of discarding a column value. +func setPath(root map[string]any, path string, value any) error { + tokens, err := parsePath(path) + if err != nil { + return err + } + return setPathTokens(root, tokens, value) +} + +func setPathTokens(root map[string]any, tokens []pathToken, value any) error { + if len(tokens) == 0 || tokens[0].isIndex { + return fmt.Errorf("path must start with a map key") + } + _, err := setPathValue(root, tokens, value) + return err +} + +func setPathValue(current any, tokens []pathToken, value any) (any, error) { if len(tokens) == 0 { - return + return value, nil } - var current any = root - for i := 0; i < len(tokens); i++ { - token := tokens[i] - last := i == len(tokens)-1 - nextIsIndex := !last && tokens[i+1].isIndex - if token.isIndex { - continue + token := tokens[0] + if token.isIndex { + var values []any + switch typed := current.(type) { + case nil: + values = []any{} + case []any: + values = typed + default: + return nil, fmt.Errorf("expected an array at index [%d], found %T", token.index, current) } - m, ok := current.(map[string]any) - if !ok { - return + for len(values) <= token.index { + values = append(values, nil) } - if last { - // Preserve the value established by an earlier column. This also - // protects a nested map/slice when a later parent scalar conflicts - // with it (for example, user.name followed by user). - if _, exists := m[token.key]; !exists { - m[token.key] = value + if len(tokens) == 1 { + if values[token.index] != nil { + return nil, fmt.Errorf("path conflicts with an existing array value at index [%d]", token.index) } - return + values[token.index] = value + return values, nil } - if _, exists := m[token.key]; !exists { - if nextIsIndex { - m[token.key] = []any{} - } else { - m[token.key] = map[string]any{} - } - } - if !nextIsIndex { - current = m[token.key] - continue + child, err := setPathValue(values[token.index], tokens[1:], value) + if err != nil { + return nil, err } + values[token.index] = child + return values, nil + } - slice, ok := m[token.key].([]any) - if !ok { - return - } - index := tokens[i+1].index - for len(slice) <= index { - if i+2 < len(tokens) && !tokens[i+2].isIndex { - slice = append(slice, map[string]any{}) - } else { - slice = append(slice, nil) - } - } - m[token.key] = slice - if i+2 == len(tokens) { - slice[index] = value - return - } - if slice[index] == nil { - slice[index] = map[string]any{} - } - current = slice[index] - i++ // The array index was consumed together with its key. + var object map[string]any + switch typed := current.(type) { + case nil: + object = make(map[string]any) + case map[string]any: + object = typed + default: + return nil, fmt.Errorf("expected an object before key %q, found %T", token.key, current) } + if len(tokens) == 1 { + if _, exists := object[token.key]; exists { + return nil, fmt.Errorf("path conflicts with an existing value at key %q", token.key) + } + object[token.key] = value + return object, nil + } + child, err := setPathValue(object[token.key], tokens[1:], value) + if err != nil { + return nil, err + } + object[token.key] = child + return object, nil } diff --git a/cmd/dataxl/structured.go b/cmd/dataxl/structured.go index fc50c2d..cf0ca0c 100644 --- a/cmd/dataxl/structured.go +++ b/cmd/dataxl/structured.go @@ -3,7 +3,10 @@ package main import ( "bytes" "encoding/json" + "errors" "fmt" + "io" + "unicode/utf8" "github.com/BurntSushi/toml" "gopkg.in/yaml.v3" @@ -15,28 +18,118 @@ func parseStructured(input []byte, format string) (any, error) { var value any switch format { case "json": + // encoding/json replaces malformed UTF-8 inside strings with U+FFFD. + // Reject it up front so a successful conversion never changes bytes + // silently. + if !utf8.Valid(input) { + return nil, fmt.Errorf("JSON input is not valid UTF-8") + } dec := json.NewDecoder(bytes.NewReader(input)) dec.UseNumber() - if err := dec.Decode(&value); err != nil { + var err error + value, err = decodeJSONValue(dec) + if err != nil { return nil, err } + _, err = decodeJSONValue(dec) + switch { + case errors.Is(err, io.EOF): + case err == nil: + return nil, fmt.Errorf("multiple JSON values are not supported") + default: + return nil, fmt.Errorf("invalid trailing JSON data: %w", err) + } case "yaml": - if err := yaml.Unmarshal(input, &value); err != nil { - return nil, err + dec := yaml.NewDecoder(bytes.NewReader(input)) + var documents []any + for { + var document any + err := dec.Decode(&document) + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return nil, err + } + documents = append(documents, normalizeYAML(document)) + } + switch len(documents) { + case 0: + value = nil + case 1: + value = documents[0] + default: + // A YAML stream is an ordered sequence of documents. Representing it + // as a slice preserves every document for conversion to other formats. + value = documents } - value = normalizeYAML(value) case "toml": var m map[string]any if err := toml.Unmarshal(input, &m); err != nil { return nil, err } - value = m + value = normalizeTOML(m) default: return nil, fmt.Errorf("format %q is not structured", format) } return value, nil } +// decodeJSONValue builds a generic structured value directly from decoder +// tokens so duplicate object keys can be rejected before a map overwrites one. +func decodeJSONValue(dec *json.Decoder) (any, error) { + token, err := dec.Token() + if err != nil { + return nil, err + } + delimiter, isDelimiter := token.(json.Delim) + if !isDelimiter { + return token, nil + } + + switch delimiter { + case '{': + object := make(map[string]any) + for dec.More() { + keyToken, err := dec.Token() + if err != nil { + return nil, err + } + key, ok := keyToken.(string) + if !ok { + return nil, fmt.Errorf("JSON object key has unexpected type %T", keyToken) + } + if _, exists := object[key]; exists { + return nil, fmt.Errorf("duplicate JSON object key %q", key) + } + child, err := decodeJSONValue(dec) + if err != nil { + return nil, err + } + object[key] = child + } + if _, err := dec.Token(); err != nil { + return nil, err + } + return object, nil + case '[': + array := make([]any, 0) + for dec.More() { + child, err := decodeJSONValue(dec) + if err != nil { + return nil, err + } + array = append(array, child) + } + if _, err := dec.Token(); err != nil { + return nil, err + } + return array, nil + default: + return nil, fmt.Errorf("unexpected JSON delimiter %q", delimiter) + } +} + func encodeStructured(value any, format string, pretty bool) ([]byte, error) { switch format { case "json": @@ -83,3 +176,31 @@ func normalizeYAML(value any) any { } return value } + +// BurntSushi/toml represents arrays of tables as []map[string]any instead of +// []any. Normalize that concrete container so wrapper detection, flattening, +// and format-to-format conversions use the same structured representation. +func normalizeTOML(value any) any { + switch v := value.(type) { + case map[string]any: + out := make(map[string]any, len(v)) + for key, child := range v { + out[key] = normalizeTOML(child) + } + return out + case []map[string]any: + out := make([]any, len(v)) + for i, child := range v { + out[i] = normalizeTOML(child) + } + return out + case []any: + out := make([]any, len(v)) + for i, child := range v { + out[i] = normalizeTOML(child) + } + return out + default: + return value + } +} diff --git a/cmd/dataxl/table.go b/cmd/dataxl/table.go index e4aef8a..f9d9f6a 100644 --- a/cmd/dataxl/table.go +++ b/cmd/dataxl/table.go @@ -7,6 +7,7 @@ import ( "fmt" "sort" "strconv" + "unicode/utf8" "github.com/xuri/excelize/v2" ) @@ -34,38 +35,48 @@ func parseTable(input []byte, format, sheet string) (table, error) { if err != nil { return table{}, err } - return rowsToTable(rows), nil + return rowsToTable(rows) default: return table{}, fmt.Errorf("format %q is not tabular", format) } } func readDelimited(input []byte, comma rune) (table, error) { + // Spreadsheet applications commonly prefix UTF-8 CSV/TSV exports with a + // BOM. It is an encoding marker, not part of the first column name. + input = bytes.TrimPrefix(input, []byte{0xEF, 0xBB, 0xBF}) + if !utf8.Valid(input) { + return table{}, fmt.Errorf("delimited input is not valid UTF-8") + } r := csv.NewReader(bytes.NewReader(input)) r.Comma = comma r.FieldsPerRecord = -1 - r.TrimLeadingSpace = comma == ',' rows, err := r.ReadAll() if err != nil { return table{}, err } - return rowsToTable(rows), nil + return rowsToTable(rows) } -func rowsToTable(rows [][]string) table { +func rowsToTable(rows [][]string) (table, error) { if len(rows) == 0 { - return table{} + return table{}, nil } - width := 0 - for _, row := range rows { - width = max(width, len(row)) + width := len(rows[0]) + for rowIndex, row := range rows[1:] { + if len(row) > width { + return table{}, fmt.Errorf( + "row %d has %d fields but the header has %d", + rowIndex+2, len(row), width, + ) + } } header := padRow(rows[0], width) body := make([][]string, 0, len(rows)-1) for _, row := range rows[1:] { body = append(body, padRow(row, width)) } - return table{Header: header, Rows: body} + return table{Header: header, Rows: body}, nil } func padRow(row []string, width int) []string { @@ -91,11 +102,25 @@ func writeDelimited(t table, comma rune) ([]byte, error) { var b bytes.Buffer w := csv.NewWriter(&b) w.Comma = comma - if err := w.Write(t.Header); err != nil { + writeRecord := func(row []string) error { + if len(row) == 1 && row[0] == "" { + // encoding/csv writes this record as a bare empty line, which its + // Reader intentionally skips. Quote the field so the row survives a + // table-to-table round trip. + w.Flush() + if err := w.Error(); err != nil { + return err + } + b.WriteString("\"\"\n") + return nil + } + return w.Write(row) + } + if err := writeRecord(t.Header); err != nil { return nil, err } for _, row := range t.Rows { - if err := w.Write(row); err != nil { + if err := writeRecord(row); err != nil { return nil, err } } @@ -193,10 +218,14 @@ func recordsFromValue(value any) []any { case []any: return v case map[string]any: - // Wrapper keys let object-shaped formats such as TOML carry row arrays. - for _, key := range []string{"rows", "records", "items"} { - if rows, ok := v[key].([]any); ok { - return rows + // A sole wrapper key lets object-shaped formats such as TOML carry row + // arrays. If siblings exist, unwrapping would silently discard them, so + // preserve the complete object as one table record instead. + if len(v) == 1 { + for _, key := range []string{"rows", "records", "items"} { + if rows, ok := v[key].([]any); ok { + return rows + } } } return []any{v} @@ -221,10 +250,7 @@ func flatten(prefix string, value any, out map[string]string) { } sort.Strings(keys) for _, key := range keys { - childPrefix := key - if prefix != "" { - childPrefix = prefix + "." + key - } + childPrefix := appendPathKey(prefix, key) flatten(childPrefix, v[key], out) } case []any: diff --git a/cmd/dataxl/testdata/fuzz/FuzzDelimitedTableRoundTrip/5838cdfae7b16cde b/cmd/dataxl/testdata/fuzz/FuzzDelimitedTableRoundTrip/5838cdfae7b16cde new file mode 100644 index 0000000..64c3aba --- /dev/null +++ b/cmd/dataxl/testdata/fuzz/FuzzDelimitedTableRoundTrip/5838cdfae7b16cde @@ -0,0 +1,2 @@ +go test fuzz v1 +string("")