Skip to content

Commit b90ce24

Browse files
committed
Review fixes: key order, response limit, archive validation, HTML escaping
- Preserve JSON key order: API methods return json.RawMessage (raw bytes); PrintJSON passes them directly to tidwall/pretty without re-encoding. Pagination merged output still re-encodes (map merge loses order, accepted). - Response body limit: io.LimitReader(100MB) in doGet to prevent OOM. - Archive ID validation: reject non-alphanumeric/dash/underscore IDs before URL construction (defense-in-depth against path traversal). - Consistent HTML escaping: errors.go now uses SetEscapeHTML(false) matching output.go; & in URLs no longer escaped to \u0026 in errors. - goreleaser brews: section added for automatic tap updates on release. - jq path uses json.Decoder with UseNumber() to preserve integer fidelity.
1 parent afac1d7 commit b90ce24

File tree

8 files changed

+109
-87
lines changed

8 files changed

+109
-87
lines changed

.goreleaser.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,18 @@ changelog:
4343
exclude:
4444
- "^docs:"
4545
- "^test:"
46+
47+
brews:
48+
- name: serpapi-cli
49+
repository:
50+
owner: serpapi
51+
name: homebrew-tap
52+
token: "{{ .Env.HOMEBREW_TAP_TOKEN }}"
53+
folder: Formula
54+
homepage: https://serpapi.com
55+
description: "HTTP client for structured web search data via SerpApi"
56+
license: MIT
57+
install: |
58+
bin.install "serpapi"
59+
test: |
60+
assert_match version.to_s, shell_output("#{bin}/serpapi --version")

pkg/api/client.go

Lines changed: 23 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (c *Client) doGet(endpoint string, params map[string]string) ([]byte, error
5858
}
5959
defer resp.Body.Close()
6060

61-
body, err := io.ReadAll(resp.Body)
61+
body, err := io.ReadAll(io.LimitReader(resp.Body, 100<<20)) // 100 MB limit
6262
if err != nil {
6363
return nil, &clierrors.NetworkError{Message: "Failed to read response: " + err.Error()}
6464
}
@@ -81,24 +81,19 @@ func (c *Client) doGet(endpoint string, params map[string]string) ([]byte, error
8181
return body, nil
8282
}
8383

84-
func checkAPIError(data []byte, target any) error {
85-
if err := json.Unmarshal(data, target); err != nil {
86-
return &clierrors.ApiError{Message: "Failed to parse JSON response: " + err.Error()}
84+
// checkAPIError returns an ApiError if the JSON body contains a top-level "error" key.
85+
func checkAPIError(body []byte) error {
86+
var envelope struct {
87+
Error string `json:"error"`
8788
}
88-
// Check for top-level "error" key in map results
89-
if m, ok := target.(*map[string]any); ok {
90-
if errVal, exists := (*m)["error"]; exists {
91-
if msg, ok := errVal.(string); ok {
92-
return &clierrors.ApiError{Message: msg}
93-
}
94-
return &clierrors.ApiError{Message: "Unknown API error"}
95-
}
89+
if json.Unmarshal(body, &envelope) == nil && envelope.Error != "" {
90+
return &clierrors.ApiError{Message: envelope.Error}
9691
}
9792
return nil
9893
}
9994

10095
// Search performs a search request. If the client has an API key it is added to params.
101-
func (c *Client) Search(params map[string]string) (map[string]any, error) {
96+
func (c *Client) Search(params map[string]string) (json.RawMessage, error) {
10297
p := make(map[string]string, len(params)+1)
10398
for k, v := range params {
10499
p[k] = v
@@ -111,65 +106,52 @@ func (c *Client) Search(params map[string]string) (map[string]any, error) {
111106
if err != nil {
112107
return nil, err
113108
}
114-
115-
var result map[string]any
116-
if err := checkAPIError(body, &result); err != nil {
109+
if err := checkAPIError(body); err != nil {
117110
return nil, err
118111
}
119-
return result, nil
112+
return json.RawMessage(body), nil
120113
}
121114

122115
// Account retrieves account information.
123-
func (c *Client) Account() (map[string]any, error) {
116+
func (c *Client) Account() (json.RawMessage, error) {
124117
params := map[string]string{"api_key": c.apiKey}
125118

126119
body, err := c.doGet("/account.json", params)
127120
if err != nil {
128121
return nil, err
129122
}
130-
131-
var result map[string]any
132-
if err := checkAPIError(body, &result); err != nil {
123+
if err := checkAPIError(body); err != nil {
133124
return nil, err
134125
}
135-
return result, nil
126+
return json.RawMessage(body), nil
136127
}
137128

138129
// Locations queries the locations endpoint. No API key is added.
139-
func (c *Client) Locations(params map[string]string) ([]any, error) {
130+
func (c *Client) Locations(params map[string]string) (json.RawMessage, error) {
140131
body, err := c.doGet("/locations.json", params)
141132
if err != nil {
142133
return nil, err
143134
}
144-
145-
var result []any
146-
if err := json.Unmarshal(body, &result); err != nil {
147-
// Maybe it's an object with an error key
148-
var obj map[string]any
149-
if json.Unmarshal(body, &obj) == nil {
150-
if errVal, ok := obj["error"]; ok {
151-
if msg, ok := errVal.(string); ok {
152-
return nil, &clierrors.ApiError{Message: msg}
153-
}
154-
}
135+
// Locations returns a JSON array; check for an error object.
136+
if len(body) > 0 && body[0] == '{' {
137+
if err := checkAPIError(body); err != nil {
138+
return nil, err
155139
}
156-
return nil, &clierrors.ApiError{Message: "Failed to parse JSON response: " + err.Error()}
140+
return nil, &clierrors.ApiError{Message: "Unexpected response from locations endpoint"}
157141
}
158-
return result, nil
142+
return json.RawMessage(body), nil
159143
}
160144

161145
// Archive retrieves a previously cached search result by ID.
162-
func (c *Client) Archive(id string) (map[string]any, error) {
146+
func (c *Client) Archive(id string) (json.RawMessage, error) {
163147
params := map[string]string{"api_key": c.apiKey}
164148

165149
body, err := c.doGet("/archive/"+id+".json", params)
166150
if err != nil {
167151
return nil, err
168152
}
169-
170-
var result map[string]any
171-
if err := checkAPIError(body, &result); err != nil {
153+
if err := checkAPIError(body); err != nil {
172154
return nil, err
173155
}
174-
return result, nil
156+
return json.RawMessage(body), nil
175157
}

pkg/cmd/archive.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
package cmd
22

33
import (
4+
"regexp"
5+
46
"github.com/spf13/cobra"
57

68
"github.com/serpapi/serpapi-cli/pkg/api"
9+
clierrors "github.com/serpapi/serpapi-cli/pkg/errors"
710
)
811

12+
var reArchiveID = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)
13+
914
var archiveCmd = &cobra.Command{
1015
Use: "archive <id>",
1116
Short: "Retrieve a previously cached search by ID",
@@ -18,6 +23,11 @@ func init() {
1823
}
1924

2025
func runArchive(cmd *cobra.Command, args []string) error {
26+
id := args[0]
27+
if !reArchiveID.MatchString(id) {
28+
return &clierrors.UsageError{Message: "Invalid archive ID: must contain only alphanumeric characters, dashes, and underscores"}
29+
}
30+
2131
apiKey, err := resolveAPIKey()
2232
if err != nil {
2333
return err

pkg/cmd/login.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cmd
22

33
import (
44
"bufio"
5+
"encoding/json"
56
"fmt"
67
"io"
78
"os"
@@ -52,16 +53,17 @@ func runLogin(cmd *cobra.Command, args []string) error {
5253
}
5354

5455
client := api.New(apiKey)
55-
result, err := client.Account()
56+
raw, err := client.Account()
5657
if err != nil {
5758
return err
5859
}
5960

6061
email := "unknown"
61-
if e, ok := result["account_email"]; ok {
62-
if s, ok := e.(string); ok {
63-
email = s
64-
}
62+
var account struct {
63+
AccountEmail string `json:"account_email"`
64+
}
65+
if json.Unmarshal(raw, &account) == nil && account.AccountEmail != "" {
66+
email = account.AccountEmail
6567
}
6668

6769
if err := config.SaveConfig(apiKey); err != nil {

pkg/cmd/root.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package cmd
22

33
import (
4+
"bytes"
5+
"encoding/json"
46
"fmt"
57
"os"
68

@@ -72,20 +74,27 @@ func resolveAPIKeyOptional() string {
7274
}
7375

7476
// handleOutput applies --jq filtering and prints result.
75-
func handleOutput(result any) error {
77+
func handleOutput(raw json.RawMessage) error {
7678
if jqFlag != "" {
77-
results, err := jq.Apply(jqFlag, result)
79+
// Unmarshal for jq processing; use decoder with UseNumber to preserve integer fidelity.
80+
var v any
81+
dec := json.NewDecoder(bytes.NewReader(raw))
82+
dec.UseNumber()
83+
if err := dec.Decode(&v); err != nil {
84+
return &clierrors.ApiError{Message: "Failed to parse response: " + err.Error()}
85+
}
86+
results, err := jq.Apply(jqFlag, v)
7887
if err != nil {
7988
return &clierrors.UsageError{Message: err.Error()}
8089
}
81-
for _, v := range results {
82-
if err := output.PrintJQValue(v, os.Stdout); err != nil {
90+
for _, val := range results {
91+
if err := output.PrintJQValue(val, os.Stdout); err != nil {
8392
return &clierrors.ApiError{Message: fmt.Sprintf("Output error: %s", err)}
8493
}
8594
}
8695
return nil
8796
}
88-
if err := output.PrintJSON(result); err != nil {
97+
if err := output.PrintJSON(raw); err != nil {
8998
return &clierrors.ApiError{Message: fmt.Sprintf("Output error: %s", err)}
9099
}
91100
return nil

pkg/cmd/search.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package cmd
22

33
import (
4+
"bytes"
5+
"encoding/json"
46
"fmt"
57
"net/url"
68
"sort"
@@ -50,12 +52,12 @@ func runSearch(cmd *cobra.Command, args []string) error {
5052
sp := newSpinner("Searching...")
5153
sp.Start()
5254
client := api.New(apiKey)
53-
result, err := client.Search(paramsMap)
55+
raw, err := client.Search(paramsMap)
5456
sp.Stop()
5557
if err != nil {
5658
return err
5759
}
58-
return handleOutput(result)
60+
return handleOutput(raw)
5961
}
6062

6163
// Pagination mode
@@ -73,13 +75,21 @@ func runSearch(cmd *cobra.Command, args []string) error {
7375
for {
7476
sp := newSpinner(fmt.Sprintf("Fetching page %d...", pagesFetched+1))
7577
sp.Start()
76-
result, err := client.Search(currentParams)
78+
raw, err := client.Search(currentParams)
7779
sp.Stop()
7880
if err != nil {
7981
return err
8082
}
8183
pagesFetched++
8284

85+
// Unmarshal for pagination logic (merge requires map access).
86+
var result map[string]any
87+
dec := json.NewDecoder(bytes.NewReader(raw))
88+
dec.UseNumber()
89+
if err := dec.Decode(&result); err != nil {
90+
return &clierrors.ApiError{Message: "Failed to parse response: " + err.Error()}
91+
}
92+
8393
// Extract next pagination URL
8494
var nextURL string
8595
if pag, ok := result["serpapi_pagination"]; ok {
@@ -95,7 +105,6 @@ func runSearch(cmd *cobra.Command, args []string) error {
95105
if accumulated == nil {
96106
accumulated = result
97107
} else {
98-
// Merge: arrays are extended, scalars kept from page 1
99108
for key, val := range result {
100109
if newItems, ok := val.([]any); ok {
101110
if existing, ok := accumulated[key]; ok {
@@ -139,10 +148,16 @@ func runSearch(cmd *cobra.Command, args []string) error {
139148
return &clierrors.ApiError{Message: "No results returned"}
140149
}
141150

142-
// Strip pagination metadata from merged result
143151
delete(accumulated, "serpapi_pagination")
144152

145-
return handleOutput(accumulated)
153+
// Re-encode merged result. Key order is lost (map merge), but & < > are not escaped.
154+
var buf bytes.Buffer
155+
enc := json.NewEncoder(&buf)
156+
enc.SetEscapeHTML(false)
157+
if err := enc.Encode(accumulated); err != nil {
158+
return &clierrors.ApiError{Message: "Failed to encode result: " + err.Error()}
159+
}
160+
return handleOutput(json.RawMessage(bytes.TrimRight(buf.Bytes(), "\n")))
146161
}
147162

148163
// parseNextParams extracts query parameters from a full URL.

pkg/errors/errors.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package errors
22

33
import (
4+
"bytes"
45
"encoding/json"
56
"fmt"
67
"os"
@@ -72,14 +73,16 @@ func PrintError(err error) {
7273
},
7374
}
7475

75-
data, jsonErr := json.MarshalIndent(payload, "", " ")
76-
if jsonErr != nil {
77-
// Fallback if JSON marshalling fails
76+
var buf bytes.Buffer
77+
enc := json.NewEncoder(&buf)
78+
enc.SetEscapeHTML(false)
79+
if jsonErr := enc.Encode(payload); jsonErr != nil {
80+
// Fallback if JSON encoding fails
7881
fmt.Fprintf(os.Stderr, "{\"error\":{\"code\":\"%s\",\"message\":\"%s\"}}\n",
7982
code, strings.ReplaceAll(strings.ReplaceAll(message, "\\", "\\\\"), "\"", "\\\""))
8083
return
8184
}
82-
fmt.Fprintln(os.Stderr, string(data))
85+
fmt.Fprint(os.Stderr, buf.String())
8386
}
8487

8588
// RedactAPIKey replaces api_key values in s with [REDACTED].

0 commit comments

Comments
 (0)