Skip to content
Open
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
22 changes: 11 additions & 11 deletions qacsv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,11 @@ var successTestCases = []TestCase{
},
}

const successTestCasesCSV = `Folder,Type,Name,Legacy ID,Draft,Priority,Tags,Requirements,Links,Files,Preconditions,Parameter Values,Template Suffix Params,Step 1,Expected 1,Step 2,Expected 2
root,standalone,tc-with-minimal-fields,,false,high,,,,,,,,,,,
root,standalone,tc-with-partial-fields,,true,low,,[](http://req1),,"[{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10},{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10}]",,,,action-1,,,expected-2
root/child,standalone,tc-with-all-fields,legacy-id,false,high,"tag1,tag2",[req1](http://req1),"[link-1](http://link1),[link-2](http://link2)","[{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10},{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10}]",preconditions,,,action-1,expected-1,action-2,expected-2
root/child,standalone,"tc-with-special-chars.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;",legacy-id,false,high,"tag1.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;","[req.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;]()","[link-1.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;](http://link1)","[{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10}]","preconditions.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;",,,"action.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;","expected.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;",,
const successTestCasesCSV = `Folder,Type,Name,Legacy ID,Draft,Priority,Tags,Requirements,Links,Files,Preconditions,Steps,Parameter Values,Template Suffix Params
root,standalone,tc-with-minimal-fields,,false,high,,,,,,,,
root,standalone,tc-with-partial-fields,,true,low,,[](http://req1),,"[{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10},{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10}]",,"[{""description"":""action-1""},{""expected"":""expected-2""}]",,
root/child,standalone,tc-with-all-fields,legacy-id,false,high,"tag1,tag2",[req1](http://req1),"[link-1](http://link1),[link-2](http://link2)","[{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10},{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10}]",preconditions,"[{""description"":""action-1"",""expected"":""expected-1""},{""description"":""action-2"",""expected"":""expected-2""}]",,
root/child,standalone,"tc-with-special-chars.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;",legacy-id,false,high,"tag1.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;","[req.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;]()","[link-1.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;](http://link1)","[{""fileName"":""file-1.csv"",""id"":""file-id"",""url"":""http://file1"",""mimeType"":""text/csv"",""size"":10}]","preconditions.,<>/@$%""""''*&()[]{}+-[BACKTICK]!~;","[{""description"":""action.,<>/@$%\""\""''*&()[]{}+-[BACKTICK]!~;"",""expected"":""expected.,<>/@$%\""\""''*&()[]{}+-[BACKTICK]!~;""}]",,
`

var failureTestCases = []TestCase{
Expand Down Expand Up @@ -353,12 +353,12 @@ var customFieldSuccessTestCases = []TestCase{
},
}

const customFieldSuccessTestCasesCSV = `Folder,Type,Name,Legacy ID,Draft,Priority,Tags,Requirements,Links,Files,Preconditions,Parameter Values,Template Suffix Params,Step 1,Expected 1,custom_field_dropdown_test_env,custom_field_dropdown_automation,custom_field_text_notes
custom-fields,standalone,tc-with-single-custom-field,,false,medium,,,,,,,,,,"{""value"":""staging"",""isDefault"":false}",,
custom-fields,standalone,tc-with-multiple-custom-fields,,false,high,"regression,smoke",,,,,,,Execute test,Test passes,"{""value"":""production"",""isDefault"":false}","{""value"":""Automated"",""isDefault"":false}","{""value"":""This is a test note with special chars: !@#$%^\u0026*()"",""isDefault"":false}"
custom-fields,standalone,tc-with-empty-custom-field-value,,false,low,,,,,,,,,,,,"{""value"":"""",""isDefault"":false}"
custom-fields,standalone,tc-with-default-custom-field,,false,medium,,,,,,,,,,,"{""value"":"""",""isDefault"":false}",
custom-fields/comprehensive,standalone,tc-with-all-fields-and-custom-fields,CF-001,false,high,"custom,comprehensive",[CF Requirements](http://cf-req),[CF Link](http://cf-link),"[{""fileName"":""cf-test.txt"",""id"":""cf-file-id"",""url"":""http://cf-file"",""mimeType"":""text/plain"",""size"":100}]",Custom field test setup,,,Step 1,Result 1,"{""value"":""development"",""isDefault"":false}","{""value"":""In Progress"",""isDefault"":false}",
const customFieldSuccessTestCasesCSV = `Folder,Type,Name,Legacy ID,Draft,Priority,Tags,Requirements,Links,Files,Preconditions,Steps,Parameter Values,Template Suffix Params,custom_field_dropdown_test_env,custom_field_dropdown_automation,custom_field_text_notes
custom-fields,standalone,tc-with-single-custom-field,,false,medium,,,,,,,,,"{""value"":""staging"",""isDefault"":false}",,
custom-fields,standalone,tc-with-multiple-custom-fields,,false,high,"regression,smoke",,,,,"[{""description"":""Execute test"",""expected"":""Test passes""}]",,,"{""value"":""production"",""isDefault"":false}","{""value"":""Automated"",""isDefault"":false}","{""value"":""This is a test note with special chars: !@#$%^&*()"",""isDefault"":false}"
custom-fields,standalone,tc-with-empty-custom-field-value,,false,low,,,,,,,,,,,"{""value"":"""",""isDefault"":false}"
custom-fields,standalone,tc-with-default-custom-field,,false,medium,,,,,,,,,,"{""value"":"""",""isDefault"":false}",
custom-fields/comprehensive,standalone,tc-with-all-fields-and-custom-fields,CF-001,false,high,"custom,comprehensive",[CF Requirements](http://cf-req),[CF Link](http://cf-link),"[{""fileName"":""cf-test.txt"",""id"":""cf-file-id"",""url"":""http://cf-file"",""mimeType"":""text/plain"",""size"":100}]",Custom field test setup,"[{""description"":""Step 1"",""expected"":""Result 1""}]",,,"{""value"":""development"",""isDefault"":false}","{""value"":""In Progress"",""isDefault"":false}",
`

var customFieldFailureTestCases = []TestCase{
Expand Down
150 changes: 95 additions & 55 deletions qascsv.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package qascsv

import (
"bytes"
"encoding/csv"
"encoding/json"
"fmt"
Expand All @@ -17,11 +18,27 @@ import (
"github.com/pkg/errors"
)

// jsonMarshal marshals v to JSON without escaping HTML characters.
func jsonMarshal(v any) ([]byte, error) {
buf := &bytes.Buffer{}
enc := json.NewEncoder(buf)
enc.SetEscapeHTML(false)
if err := enc.Encode(v); err != nil {
Comment on lines +22 to +26

Choose a reason for hiding this comment

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

security-medium medium

The jsonMarshal function disables HTML escaping by setting enc.SetEscapeHTML(false). This is a security risk. The function is used to serialize data for the CSV output, and some of that data, like test case steps, can contain user-provided Markdown or HTML. By disabling escaping, you create a Stored Cross-Site Scripting (XSS) vulnerability if the generated CSV data is ever rendered in a web application. To prevent this, the default behavior of escaping HTML should be preserved.

Suggested change
func jsonMarshal(v any) ([]byte, error) {
buf := &bytes.Buffer{}
enc := json.NewEncoder(buf)
enc.SetEscapeHTML(false)
if err := enc.Encode(v); err != nil {
func jsonMarshal(v any) ([]byte, error) {
buf := &bytes.Buffer{}
enc := json.NewEncoder(buf)
if err := enc.Encode(v); err != nil {

return nil, err
}
// Remove trailing newline added by Encode
b := buf.Bytes()
if len(b) > 0 && b[len(b)-1] == '\n' {
b = b[:len(b)-1]
}
return b, nil
}

// staticColumns will always be present in the CSV file
// but there can be additional columns for steps and custom fields.
var staticColumns = []string{
"Folder", "Type", "Name", "Legacy ID", "Draft", "Priority", "Tags", "Requirements",
"Links", "Files", "Preconditions", "Parameter Values", "Template Suffix Params",
"Links", "Files", "Preconditions", "Steps", "Parameter Values", "Template Suffix Params",
}

// Priority represents the priority of a test case in QA Sphere.
Expand Down Expand Up @@ -70,9 +87,12 @@ type File struct {
// Step represents a single action to perform in a test case.
type Step struct {
// The action to perform. Markdown is supported. (optional)
Action string
Action string `json:"description,omitempty"`
// The expected result of the action. Markdown is supported. (optional)
Expected string
Expected string `json:"expected,omitempty"`

SharedStepID string `json:"sharedStepId,omitempty"`
SubSteps []Step `json:"subSteps,omitempty"`
}

// ParameterValue represents parameter values that you provide for template test cases.
Expand Down Expand Up @@ -235,8 +255,19 @@ func (q *QASphereCSV) AddTestCases(tcs []TestCase) error {
return nil
}

func (q *QASphereCSV) AddFolder(folder string) error {
if folder == "" {
return errors.New("folder cannot be empty")
}
if _, ok := q.folderTCaseMap[folder]; ok {
return errors.Errorf("folder %q already exists", folder)
}
q.folderTCaseMap[folder] = nil
return nil
}

func (q *QASphereCSV) GenerateCSV() (string, error) {
w := &strings.Builder{}
w := bytes.NewBuffer(make([]byte, 0, 1024))
if err := q.writeCSV(w); err != nil {
return "", errors.Wrap(err, "generate csv")
}
Expand Down Expand Up @@ -277,7 +308,11 @@ func (q *QASphereCSV) validateTestCase(tc TestCase) error {
}

func (q *QASphereCSV) addTCase(tc TestCase) {
folderPath := strings.Join(tc.Folder, "/")
escapedFolder := make([]string, len(tc.Folder))
for i, folder := range tc.Folder {
escapedFolder[i] = strings.ReplaceAll(folder, "/", `\/`)
}
folderPath := strings.Join(escapedFolder, "/")
Comment on lines +311 to +315

Choose a reason for hiding this comment

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

high

The logic to escape / characters in folder names is inconsistent with the validation rule on the TestCase.Folder field. The validation validate:"...,excludesall=/" on line 136 will cause an error if any folder name contains a /, making this escaping logic effectively dead code.

If the intention is to support slashes in folder names, the validation rule should be removed. If not, this escaping logic is unnecessary and should be removed to avoid confusion and improve clarity.

q.folderTCaseMap[folderPath] = append(q.folderTCaseMap[folderPath], tc)

q.numTCases++
Expand All @@ -286,40 +321,48 @@ func (q *QASphereCSV) addTCase(tc TestCase) {
}
}

func (q *QASphereCSV) getFolders() []string {
var folders []string
for folder := range q.folderTCaseMap {
folders = append(folders, folder)
}
slices.Sort(folders)
return folders
}
func (q *QASphereCSV) writeCSV(w io.Writer) error {
csvw := csv.NewWriter(w)

func (q *QASphereCSV) getCSVRows() ([][]string, error) {
rows := make([][]string, 0, q.numTCases+1)
numCols := len(staticColumns) + 2*q.maxSteps + len(q.customFields)
row := make([]string, 0, len(staticColumns)+len(q.customFields))
row = append(row, staticColumns...)

rows = append(rows, append(make([]string, 0, numCols), staticColumns...))
for i := 0; i < q.maxSteps; i++ {
rows[0] = append(rows[0], fmt.Sprintf("Step %d", i+1), fmt.Sprintf("Expected %d", i+1))
}

customFieldsMap := make(map[string]int)
customFieldsMap := make(map[string]int, len(q.customFields))
for i, cf := range q.customFields {
customFieldHeader := fmt.Sprintf("custom_field_%s_%s", cf.Type, cf.SystemName)
rows[0] = append(rows[0], customFieldHeader)
row = append(row, customFieldHeader)
customFieldsMap[cf.SystemName] = i
}

folders := q.getFolders()
for _, f := range folders {
for _, tc := range q.folderTCaseMap[f] {
if err := csvw.Write(row); err != nil {
return errors.Wrap(err, "could not write header row")
}

folders := make([]string, 0, len(q.folderTCaseMap))
for folder := range q.folderTCaseMap {
folders = append(folders, folder)
}
slices.Sort(folders)

for _, folder := range folders {
testCases := q.folderTCaseMap[folder]

// Empty folder (no test cases)
if len(testCases) == 0 {
clear(row)
row[0] = folder
if err := csvw.Write(row); err != nil {
return errors.Wrap(err, "could not write row")
}
Comment on lines +352 to +356

Choose a reason for hiding this comment

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

medium

The use of clear(row) and then row[0] = folder to create a row for an empty folder is functionally correct but could be more readable for future maintainers. It relies on the row slice having the correct length from the header write operation.

For better readability and maintainability, I'd suggest creating a new slice for the empty folder row explicitly. This makes the intent clearer.

Suggested change
clear(row)
row[0] = folder
if err := csvw.Write(row); err != nil {
return errors.Wrap(err, "could not write row")
}
emptyRow := make([]string, len(row))
emptyRow[0] = folder
if err := csvw.Write(emptyRow); err != nil {
return errors.Wrap(err, "could not write row")
}

continue
}

for _, tc := range testCases {
var requirements []string
for _, req := range tc.Requirements {
if req.Title == "" && req.URL == "" {
continue
}

requirements = append(requirements, fmt.Sprintf("[%s](%s)", req.Title, req.URL))
}

Expand All @@ -330,59 +373,56 @@ func (q *QASphereCSV) getCSVRows() ([][]string, error) {

var files string
if len(tc.Files) > 0 {
filesb, err := json.Marshal(tc.Files)
filesb, err := jsonMarshal(tc.Files)
if err != nil {
return nil, errors.Wrap(err, "json marshal files")
return errors.Wrap(err, "json marshal files")
}
files = string(filesb)
}

var steps string
if len(tc.Steps) > 0 {
stepsb, err := jsonMarshal(tc.Steps)
if err != nil {
return errors.Wrap(err, "json marshal steps")
}
steps = string(stepsb)
}

var parameterValues string
if len(tc.ParameterValues) > 0 {
parameterValuesb, err := json.Marshal(tc.ParameterValues)
parameterValuesb, err := jsonMarshal(tc.ParameterValues)
if err != nil {
return nil, errors.Wrap(err, "json marshal parameter values")
return errors.Wrap(err, "json marshal parameter values")
}
parameterValues = string(parameterValuesb)
}

row := make([]string, 0, numCols)
row = append(row, f, string(tc.Type), tc.Title, tc.LegacyID, strconv.FormatBool(tc.Draft),
row = append(row[:0],
strings.Join(tc.Folder, "/"), string(tc.Type), tc.Title, tc.LegacyID, strconv.FormatBool(tc.Draft),
string(tc.Priority), strings.Join(tc.Tags, ","), strings.Join(requirements, ","),
strings.Join(links, ","), files, tc.Preconditions, parameterValues,
strings.Join(links, ","), files, tc.Preconditions, steps, parameterValues,
strings.Join(tc.FilledTCaseTitleSuffixParams, ","))
Comment on lines +401 to 405

Choose a reason for hiding this comment

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

high

The folder path for the CSV row is being reconstructed using strings.Join(tc.Folder, "/"). This is inconsistent with how folder paths are grouped in the folderTCaseMap, which uses an escaped version of the path (as created in addTCase).

This can lead to incorrect folder paths in the output CSV if folder names contain characters that are escaped, like /.

To ensure consistency, you should use the folder variable from the loop, which is the key from folderTCaseMap.

Suggested change
row = append(row[:0],
strings.Join(tc.Folder, "/"), string(tc.Type), tc.Title, tc.LegacyID, strconv.FormatBool(tc.Draft),
string(tc.Priority), strings.Join(tc.Tags, ","), strings.Join(requirements, ","),
strings.Join(links, ","), files, tc.Preconditions, parameterValues,
strings.Join(links, ","), files, tc.Preconditions, steps, parameterValues,
strings.Join(tc.FilledTCaseTitleSuffixParams, ","))
row = append(row[:0],
folder, string(tc.Type), tc.Title, tc.LegacyID, strconv.FormatBool(tc.Draft),
string(tc.Priority), strings.Join(tc.Tags, ","), strings.Join(requirements, ","),
strings.Join(links, ","), files, tc.Preconditions, steps, parameterValues,
strings.Join(tc.FilledTCaseTitleSuffixParams, ","))


numSteps := len(tc.Steps)
for i := 0; i < q.maxSteps; i++ {
if i < numSteps {
row = append(row, tc.Steps[i].Action, tc.Steps[i].Expected)
} else {
row = append(row, "", "")
}
}

customFieldCols := make([]string, len(customFieldsMap))
for systemName, cfValue := range tc.CustomFields {
cfValueJSON, err := json.Marshal(cfValue)
cfValueJSON, err := jsonMarshal(cfValue)
if err != nil {
return nil, errors.Wrap(err, "json marshal custom field value")
return errors.Wrap(err, "json marshal custom field value")
}

customFieldCols[customFieldsMap[systemName]] = string(cfValueJSON)
}
row = append(row, customFieldCols...)

rows = append(rows, row)
if err := csvw.Write(row); err != nil {
return errors.Wrap(err, "could not write row")
}
}
}

return rows, nil
}

func (q *QASphereCSV) writeCSV(w io.Writer) error {
rows, err := q.getCSVRows()
if err != nil {
return errors.Wrap(err, "get csv rows")
csvw.Flush()
if err := csvw.Error(); err != nil {
return errors.Wrap(err, "csv writer error")
}
return csv.NewWriter(w).WriteAll(rows)
return nil
}