Replace io.ReadAll + json.Unmarshal with streaming JSON decoder
Refactor the internal HTTP helper from get() returning raw bytes to getAndDecode() that streams directly into the target struct via json.NewDecoder. This eliminates the intermediate []byte allocation from io.ReadAll on every API response. The new decoder also validates that responses contain exactly one JSON value by attempting a second Decode after the primary one — any content beyond the first value (e.g., concatenated objects from a misbehaving proxy) returns an error instead of silently discarding it. Changes: - api/client.go: Replace get() with getAndDecode(), update FetchStores and FetchSavings callers to use the new signature - api/client_test.go: Add TestFetchSavings_TrailingJSONIsRejected and TestFetchStores_MalformedJSONReturnsDecodeError covering the new decoder error paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,7 @@ package api
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
@@ -42,10 +43,10 @@ func NewClientWithBaseURLs(savingsURL, storeURL string) *Client {
|
||||
}
|
||||
}
|
||||
|
||||
func (c *Client) get(ctx context.Context, reqURL, storeNumber string) ([]byte, error) {
|
||||
func (c *Client) getAndDecode(ctx context.Context, reqURL, storeNumber string, out any) error {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("creating request: %w", err)
|
||||
return fmt.Errorf("creating request: %w", err)
|
||||
}
|
||||
|
||||
req.Header.Set("Accept", "application/json")
|
||||
@@ -56,19 +57,22 @@ func (c *Client) get(ctx context.Context, reqURL, storeNumber string) ([]byte, e
|
||||
|
||||
resp, err := c.httpClient.Do(req)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("executing request: %w", err)
|
||||
return fmt.Errorf("executing request: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, fmt.Errorf("unexpected status %d from %s", resp.StatusCode, reqURL)
|
||||
return fmt.Errorf("unexpected status %d from %s", resp.StatusCode, reqURL)
|
||||
}
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("reading response: %w", err)
|
||||
dec := json.NewDecoder(resp.Body)
|
||||
if err := dec.Decode(out); err != nil {
|
||||
return fmt.Errorf("decoding response: %w", err)
|
||||
}
|
||||
return body, nil
|
||||
if err := dec.Decode(new(struct{})); !errors.Is(err, io.EOF) {
|
||||
return fmt.Errorf("decoding response: trailing JSON content")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// FetchStores finds Publix stores near the given zip code.
|
||||
@@ -81,14 +85,9 @@ func (c *Client) FetchStores(ctx context.Context, zipCode string, count int) ([]
|
||||
"zipCode": {zipCode},
|
||||
}
|
||||
|
||||
body, err := c.get(ctx, c.storeURL+"?"+params.Encode(), "")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("fetching stores: %w", err)
|
||||
}
|
||||
|
||||
var resp StoreResponse
|
||||
if err := json.Unmarshal(body, &resp); err != nil {
|
||||
return nil, fmt.Errorf("decoding stores: %w", err)
|
||||
if err := c.getAndDecode(ctx, c.storeURL+"?"+params.Encode(), "", &resp); err != nil {
|
||||
return nil, fmt.Errorf("fetching stores: %w", err)
|
||||
}
|
||||
return resp.Stores, nil
|
||||
}
|
||||
@@ -96,22 +95,17 @@ func (c *Client) FetchStores(ctx context.Context, zipCode string, count int) ([]
|
||||
// FetchSavings fetches all weekly ad savings for the given store.
|
||||
func (c *Client) FetchSavings(ctx context.Context, storeNumber string) (*SavingsResponse, error) {
|
||||
params := url.Values{
|
||||
"page": {"1"},
|
||||
"pageSize": {"0"},
|
||||
"page": {"1"},
|
||||
"pageSize": {"0"},
|
||||
"includePersonalizedDeals": {"false"},
|
||||
"languageID": {"1"},
|
||||
"isWeb": {"true"},
|
||||
"getSavingType": {"WeeklyAd"},
|
||||
}
|
||||
|
||||
body, err := c.get(ctx, c.savingsURL+"?"+params.Encode(), storeNumber)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("fetching savings: %w", err)
|
||||
"languageID": {"1"},
|
||||
"isWeb": {"true"},
|
||||
"getSavingType": {"WeeklyAd"},
|
||||
}
|
||||
|
||||
var resp SavingsResponse
|
||||
if err := json.Unmarshal(body, &resp); err != nil {
|
||||
return nil, fmt.Errorf("decoding savings: %w", err)
|
||||
if err := c.getAndDecode(ctx, c.savingsURL+"?"+params.Encode(), storeNumber, &resp); err != nil {
|
||||
return nil, fmt.Errorf("fetching savings: %w", err)
|
||||
}
|
||||
return &resp, nil
|
||||
}
|
||||
|
||||
@@ -127,6 +127,34 @@ func TestFetchStores_NoResults(t *testing.T) {
|
||||
assert.Empty(t, result)
|
||||
}
|
||||
|
||||
func TestFetchSavings_TrailingJSONIsRejected(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"Savings":[],"LanguageId":1} {"extra":true}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
client := api.NewClientWithBaseURLs(srv.URL, "")
|
||||
_, err := client.FetchSavings(context.Background(), "1425")
|
||||
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "decoding")
|
||||
}
|
||||
|
||||
func TestFetchStores_MalformedJSONReturnsDecodeError(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"Stores":`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
client := api.NewClientWithBaseURLs("", srv.URL)
|
||||
_, err := client.FetchStores(context.Background(), "37042", 5)
|
||||
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "decoding")
|
||||
}
|
||||
|
||||
func TestStoreNumber(t *testing.T) {
|
||||
tests := []struct {
|
||||
input string
|
||||
|
||||
Reference in New Issue
Block a user