Skip to content

Commit 2476284

Browse files
EvanNotFoundEvanNotFound
committed
fix(api): preserve counter route contracts and normalize targets
- keep `/log`, `/api/v1/log`, and `/api/v2/log` responses intact - tighten public CORS to wildcard non-credentialed `GET` and `POST` - normalize targets once across handlers and counter storage flows - log Redis rate-limit failures with explicit fail-open behavior - archive OpenSpec cleanup docs and extend API and counter tests
1 parent 07ec142 commit 2476284

13 files changed

Lines changed: 634 additions & 330 deletions

File tree

‎apps/api/internal/api/log.go‎

Lines changed: 173 additions & 187 deletions
Large diffs are not rendered by default.

‎apps/api/internal/api/public.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"path/filepath"
99
"time"
1010

11+
"github.com/go-chi/chi/v5"
1112
redis "github.com/redis/go-redis/v9"
1213
)
1314

@@ -40,7 +41,7 @@ func (h *PublicHandler) Root(w http.ResponseWriter, _ *http.Request) {
4041
"/healthz",
4142
"/js",
4243
"/bench/write",
43-
// "/log", deprecated
44+
"/log",
4445
"/api/v1/log",
4546
"/api/v2/log",
4647
},
@@ -98,13 +99,22 @@ func writeJSON(w http.ResponseWriter, status int, payload any) {
9899
}
99100

100101
func applyCORSHeaders(w http.ResponseWriter) {
101-
w.Header().Set("Access-Control-Allow-Credentials", "true")
102102
w.Header().Set("Access-Control-Allow-Origin", "*")
103-
w.Header().Set("Access-Control-Allow-Methods", "GET, HEAD, DELETE, PATCH, POST, PUT, OPTIONS")
104-
w.Header().Set("Access-Control-Allow-Headers", "X-CSRF-Token, X-Requested-With, Accept, Accept-Version, Content-Length, Content-MD5, Content-Type, Date, X-Api-Version, X-Browser-Token")
103+
w.Header().Set("Access-Control-Allow-Methods", "GET, HEAD, POST, OPTIONS")
104+
w.Header().Set("Access-Control-Allow-Headers", "Content-Type")
105105
w.Header().Set("Access-Control-Max-Age", "86400")
106106
}
107107

108+
func RoutePattern(r *http.Request) string {
109+
if routeContext := chi.RouteContext(r.Context()); routeContext != nil {
110+
if pattern := routeContext.RoutePattern(); pattern != "" {
111+
return pattern
112+
}
113+
}
114+
115+
return r.URL.Path
116+
}
117+
108118
func applyNoStoreHeaders(w http.ResponseWriter) {
109119
w.Header().Set("Cache-Control", "no-store, max-age=0")
110120
w.Header().Set("Pragma", "no-cache")

‎apps/api/internal/app/server.go‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,15 @@ func (s *Server) Routes() http.Handler {
3636
r.Head("/js", s.public.Script)
3737
r.Get("/bench/write", s.logAPI.BenchmarkWrite)
3838

39-
r.Options("/log", s.logAPI.V1Options)
40-
r.Get("/log", s.logAPI.V1Get)
41-
r.Post("/log", s.logAPI.V1Post)
39+
registerCounterRoutes(r, "/log", s.logAPI.V1Options, s.logAPI.V1Get, s.logAPI.V1Post)
4240

4341
r.Route("/api", func(r chi.Router) {
4442
r.Route("/v1", func(r chi.Router) {
45-
r.Options("/log", s.logAPI.V1Options)
46-
r.Get("/log", s.logAPI.V1Get)
47-
r.Post("/log", s.logAPI.V1Post)
43+
registerCounterRoutes(r, "/log", s.logAPI.V1Options, s.logAPI.V1Get, s.logAPI.V1Post)
4844
})
4945

5046
r.Route("/v2", func(r chi.Router) {
51-
r.Options("/log", s.logAPI.V2Options)
52-
r.Get("/log", s.logAPI.V2Get)
53-
r.Post("/log", s.logAPI.V2Post)
47+
registerCounterRoutes(r, "/log", s.logAPI.V2Options, s.logAPI.V2Get, s.logAPI.V2Post)
5448
})
5549
})
5650

@@ -66,7 +60,7 @@ func requestLoggingMiddleware(log *Logger) func(http.Handler) http.Handler {
6660
next.ServeHTTP(wrapped, r)
6761

6862
status := wrapped.Status()
69-
route := routePattern(r)
63+
route := api.RoutePattern(r)
7064
fields := map[string]any{
7165
"request_id": middleware.GetReqID(r.Context()),
7266
"method": r.Method,
@@ -96,12 +90,8 @@ func requestLoggingMiddleware(log *Logger) func(http.Handler) http.Handler {
9690
}
9791
}
9892

99-
func routePattern(r *http.Request) string {
100-
if routeContext := chi.RouteContext(r.Context()); routeContext != nil {
101-
if pattern := routeContext.RoutePattern(); pattern != "" {
102-
return pattern
103-
}
104-
}
105-
106-
return r.URL.Path
93+
func registerCounterRoutes(r chi.Router, path string, options http.HandlerFunc, get http.HandlerFunc, post http.HandlerFunc) {
94+
r.Options(path, options)
95+
r.Get(path, get)
96+
r.Post(path, post)
10797
}

‎apps/api/internal/app/server_test.go‎

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ func TestRootEndpointReturnsServiceMetadata(t *testing.T) {
4343
if !containsRoute(routes, "/bench/write") {
4444
t.Fatalf("expected benchmark route in metadata, got %#v", routes)
4545
}
46+
for _, route := range []string{"/log", "/api/v1/log", "/api/v2/log"} {
47+
if !containsRoute(routes, route) {
48+
t.Fatalf("expected route %q in metadata, got %#v", route, routes)
49+
}
50+
}
4651
}
4752

4853
func TestHealthzReportsReadyWhenRedisReachable(t *testing.T) {
@@ -108,6 +113,10 @@ func TestScriptRouteServesJavaScriptAsset(t *testing.T) {
108113
t.Fatalf("expected CORS header, got %q", recorder.Header().Get("Access-Control-Allow-Origin"))
109114
}
110115

116+
if recorder.Header().Get("Access-Control-Allow-Credentials") != "" {
117+
t.Fatalf("expected credentialless CORS, got %q", recorder.Header().Get("Access-Control-Allow-Credentials"))
118+
}
119+
111120
if recorder.Body.String() != scriptContents {
112121
t.Fatalf("expected script contents %q, got %q", scriptContents, recorder.Body.String())
113122
}
@@ -141,6 +150,36 @@ func TestV1RoutesKeepPlainJSONResponseShape(t *testing.T) {
141150
}
142151
}
143152

153+
func TestV1PostRoutesKeepPlainJSONResponseShape(t *testing.T) {
154+
handler := newTestHandler(t, mustStartMiniRedis(t).Addr())
155+
156+
for _, path := range []string{"/log", "/api/v1/log"} {
157+
t.Run(path, func(t *testing.T) {
158+
recorder := httptest.NewRecorder()
159+
request := httptest.NewRequest(http.MethodPost, path, strings.NewReader(`{"url":"file:///bad","isNewUv":true}`))
160+
request.Header.Set("Content-Type", "application/json")
161+
handler.ServeHTTP(recorder, request)
162+
163+
if recorder.Code != http.StatusOK {
164+
t.Fatalf("expected 200, got %d", recorder.Code)
165+
}
166+
167+
var payload map[string]any
168+
if err := json.Unmarshal(recorder.Body.Bytes(), &payload); err != nil {
169+
t.Fatalf("unmarshal v1 response: %v", err)
170+
}
171+
172+
if _, hasStatus := payload["status"]; hasStatus {
173+
t.Fatalf("expected plain JSON payload, got envelope %#v", payload)
174+
}
175+
176+
if payload["error"] == nil || payload["site_uv"] == nil {
177+
t.Fatalf("expected legacy v1 fields, got %#v", payload)
178+
}
179+
})
180+
}
181+
}
182+
144183
func TestV2RouteKeepsEnvelopeResponseShape(t *testing.T) {
145184
handler := newTestHandler(t, mustStartMiniRedis(t).Addr())
146185
recorder := httptest.NewRecorder()
@@ -166,6 +205,33 @@ func TestV2RouteKeepsEnvelopeResponseShape(t *testing.T) {
166205
}
167206
}
168207

208+
func TestV2PostRouteKeepsEnvelopeResponseShape(t *testing.T) {
209+
handler := newTestHandler(t, mustStartMiniRedis(t).Addr())
210+
recorder := httptest.NewRecorder()
211+
request := httptest.NewRequest(http.MethodPost, "/api/v2/log", strings.NewReader(`{"url":"file:///bad","isNewUv":true}`))
212+
request.Header.Set("Content-Type", "application/json")
213+
214+
handler.ServeHTTP(recorder, request)
215+
216+
if recorder.Code != http.StatusOK {
217+
t.Fatalf("expected 200, got %d", recorder.Code)
218+
}
219+
220+
var payload map[string]any
221+
if err := json.Unmarshal(recorder.Body.Bytes(), &payload); err != nil {
222+
t.Fatalf("unmarshal v2 response: %v", err)
223+
}
224+
225+
if payload["status"] != "success" {
226+
t.Fatalf("expected success envelope, got %#v", payload)
227+
}
228+
229+
data, ok := payload["data"].(map[string]any)
230+
if !ok || data["site_uv"] == nil {
231+
t.Fatalf("expected data envelope, got %#v", payload)
232+
}
233+
}
234+
169235
func TestOptionsRoutesStayAvailable(t *testing.T) {
170236
handler := newTestHandler(t, mustStartMiniRedis(t).Addr())
171237

@@ -174,7 +240,8 @@ func TestOptionsRoutesStayAvailable(t *testing.T) {
174240
path string
175241
expectStatus any
176242
}{
177-
{name: "v1 options", path: "/log", expectStatus: nil},
243+
{name: "legacy options", path: "/log", expectStatus: nil},
244+
{name: "v1 options", path: "/api/v1/log", expectStatus: nil},
178245
{name: "v2 options", path: "/api/v2/log", expectStatus: "success"},
179246
} {
180247
t.Run(tc.name, func(t *testing.T) {
@@ -189,6 +256,10 @@ func TestOptionsRoutesStayAvailable(t *testing.T) {
189256
t.Fatalf("expected CORS header, got %q", recorder.Header().Get("Access-Control-Allow-Origin"))
190257
}
191258

259+
if recorder.Header().Get("Access-Control-Allow-Credentials") != "" {
260+
t.Fatalf("expected credentialless CORS, got %q", recorder.Header().Get("Access-Control-Allow-Credentials"))
261+
}
262+
192263
var payload map[string]any
193264
if err := json.Unmarshal(recorder.Body.Bytes(), &payload); err != nil {
194265
t.Fatalf("unmarshal options response: %v", err)
@@ -219,6 +290,10 @@ func TestBenchmarkWriteRouteUsesFixedTargetAndNoStoreHeaders(t *testing.T) {
219290
t.Fatalf("expected CORS header, got %q", recorder.Header().Get("Access-Control-Allow-Origin"))
220291
}
221292

293+
if recorder.Header().Get("Access-Control-Allow-Credentials") != "" {
294+
t.Fatalf("expected credentialless CORS, got %q", recorder.Header().Get("Access-Control-Allow-Credentials"))
295+
}
296+
222297
if !strings.Contains(recorder.Header().Get("Cache-Control"), "no-store") {
223298
t.Fatalf("expected no-store cache header, got %q", recorder.Header().Get("Cache-Control"))
224299
}

‎apps/api/internal/counter/busuanzi.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,32 +70,32 @@ func (s *Service) fetchBusuanziData(ctx context.Context, referer string) *busuan
7070
return data
7171
}
7272

73-
func (s *Service) fetchBusuanziSiteUV(ctx context.Context, hostSanitized string, hostOriginal string) int64 {
74-
data := s.fetchBusuanziData(ctx, "https://"+hostOriginal+"/")
73+
func (s *Service) fetchBusuanziSiteUV(ctx context.Context, target Target) int64 {
74+
data := s.fetchBusuanziData(ctx, "https://"+target.Host+"/")
7575
if data == nil {
7676
return 0
7777
}
7878

79-
s.log.Debug("Busuanzi site UV retrieved", counterLogFields("busuanzi.site_uv.retrieved", map[string]any{"host": hostSanitized, "site_uv": data.SiteUV}))
79+
s.log.Debug("Busuanzi site UV retrieved", counterLogFields("busuanzi.site_uv.retrieved", map[string]any{"host": target.Host, "site_uv": data.SiteUV}))
8080
return data.SiteUV
8181
}
8282

83-
func (s *Service) fetchBusuanziSitePV(ctx context.Context, hostSanitized string, hostOriginal string) int64 {
84-
data := s.fetchBusuanziData(ctx, "https://"+hostOriginal+"/")
83+
func (s *Service) fetchBusuanziSitePV(ctx context.Context, target Target) int64 {
84+
data := s.fetchBusuanziData(ctx, "https://"+target.Host+"/")
8585
if data == nil {
8686
return 0
8787
}
8888

89-
s.log.Debug("Busuanzi site PV retrieved", counterLogFields("busuanzi.site_pv.retrieved", map[string]any{"host": hostSanitized, "site_pv": data.SitePV}))
89+
s.log.Debug("Busuanzi site PV retrieved", counterLogFields("busuanzi.site_pv.retrieved", map[string]any{"host": target.Host, "site_pv": data.SitePV}))
9090
return data.SitePV
9191
}
9292

93-
func (s *Service) fetchBusuanziPagePV(ctx context.Context, hostSanitized string, pathSanitized string, hostOriginal string) int64 {
94-
data := s.fetchBusuanziData(ctx, "https://"+hostOriginal+pathSanitized)
93+
func (s *Service) fetchBusuanziPagePV(ctx context.Context, target Target) int64 {
94+
data := s.fetchBusuanziData(ctx, "https://"+target.Host+target.Path)
9595
if data == nil {
9696
return 0
9797
}
9898

99-
s.log.Debug("Busuanzi page PV retrieved", counterLogFields("busuanzi.page_pv.retrieved", map[string]any{"host": hostSanitized, "target_path": pathSanitized, "page_pv": data.PagePV}))
99+
s.log.Debug("Busuanzi page PV retrieved", counterLogFields("busuanzi.page_pv.retrieved", map[string]any{"host": target.Host, "target_path": target.Path, "page_pv": data.PagePV}))
100100
return data.PagePV
101101
}

0 commit comments

Comments
 (0)