-
Notifications
You must be signed in to change notification settings - Fork 782
FIX: origClientOrderIdList format in BatchCancelOrders API #794
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -791,7 +791,9 @@ func (s *CancelMultiplesOrdersService) Do(ctx context.Context, opts ...RequestOp | |||||||||||||||||||||||||||||||||||
| r.setFormParam("orderIdList", orderIDListString) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if s.origClientOrderIDList != nil { | ||||||||||||||||||||||||||||||||||||
| r.setFormParam("origClientOrderIdList", s.origClientOrderIDList) | ||||||||||||||||||||||||||||||||||||
| // convert a slice of strings to a string e.g. ["my_id_1","my_id_2"], encode the double quotes. No space after comma. | ||||||||||||||||||||||||||||||||||||
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | |
| list := fmt.Sprint(s.origClientOrderIDList) // e.g. "[my_id_1 my_id_2]" | |
| list = strings.Trim(list, "[]") | |
| origClientOrderIDListString := "[]" | |
| if list != "" { | |
| items := strings.Split(list, " ") | |
| for i, v := range items { | |
| items[i] = fmt.Sprintf("%q", v) | |
| } | |
| origClientOrderIDListString = "[" + strings.Join(items, ",") + "]" | |
| } |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The string formatting logic doesn't handle empty slices correctly. When origClientOrderIDList is an empty slice (not nil, but length 0), this code will produce the string [""] instead of []. This happens because strings.Join on an empty slice returns an empty string, and the code then wraps it with [" + "" + "].
Consider checking the length of the slice before formatting, similar to how the nil check is done.
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | |
| r.setFormParam("origClientOrderIdList", origClientOrderIDListString) | |
| if len(s.origClientOrderIDList) == 0 { | |
| r.setFormParam("origClientOrderIdList", "[]") | |
| } else { | |
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | |
| r.setFormParam("origClientOrderIdList", origClientOrderIDListString) | |
| } |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The string concatenation doesn't handle special characters that could appear in client order IDs. If an order ID contains a double quote character, the resulting string will be malformed. Consider using json.Marshal for proper escaping of special characters in the string values, or add validation/escaping for characters like quotes and backslashes.
| // convert a slice of strings to a string e.g. ["my_id_1","my_id_2"], encode the double quotes. No space after comma. | |
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | |
| r.setFormParam("origClientOrderIdList", origClientOrderIDListString) | |
| // convert a slice of strings to a JSON array string, ensuring proper escaping of special characters | |
| origClientOrderIDListJSON, err := json.Marshal(s.origClientOrderIDList) | |
| if err != nil { | |
| return nil, err | |
| } | |
| r.setFormParam("origClientOrderIdList", string(origClientOrderIDListJSON)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states "encode the double quotes," but the implementation doesn't actually encode them - it embeds literal escaped quotes in the string using backslash notation. If the API requires URL-encoded double quotes, consider using url.QueryEscape or clarifying the comment to accurately describe what the code does (e.g., "escape the double quotes with backslashes").