Skip to content

Commit 1a30418

Browse files
Merge pull request #30 from ashum9/security-pr-s1-strict-json
harden location ingest JSON parsing
2 parents f7347e7 + d72e493 commit 1a30418

2 files changed

Lines changed: 89 additions & 1 deletion

File tree

handlers.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"io"
78
"log"
89
"net/http"
910
"time"
@@ -53,7 +54,16 @@ func handlePostLocation(store LocationSaver, tracker *Tracker) http.HandlerFunc
5354
r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // 1MB
5455

5556
var loc LocationReport
56-
if err := json.NewDecoder(r.Body).Decode(&loc); err != nil {
57+
decoder := json.NewDecoder(r.Body)
58+
decoder.DisallowUnknownFields()
59+
if err := decoder.Decode(&loc); err != nil {
60+
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid JSON: " + err.Error()})
61+
return
62+
}
63+
if err := decoder.Decode(new(json.RawMessage)); err == nil {
64+
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid JSON: request body must contain a single JSON object and no trailing data"})
65+
return
66+
} else if err != io.EOF {
5767
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid JSON: " + err.Error()})
5868
return
5969
}

handlers_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,16 @@ func postLocation(handler http.HandlerFunc, loc LocationReport) *httptest.Respon
2626
return w
2727
}
2828

29+
func postLocationWithBody(handler http.HandlerFunc, body []byte, contentType string) *httptest.ResponseRecorder {
30+
req := httptest.NewRequest("POST", "/api/v1/locations", bytes.NewReader(body))
31+
if contentType != "" {
32+
req.Header.Set("Content-Type", contentType)
33+
}
34+
w := httptest.NewRecorder()
35+
handler(w, req)
36+
return w
37+
}
38+
2939
func getFeed(handler http.HandlerFunc, query string) *httptest.ResponseRecorder {
3040
url := "/gtfs-rt/vehicle-positions"
3141
if query != "" {
@@ -267,3 +277,71 @@ func TestHandlePostLocation_InvalidJSON(t *testing.T) {
267277
require.NoError(t, err)
268278
assert.Contains(t, resp["error"], "invalid JSON")
269279
}
280+
281+
func TestHandlePostLocation_UnknownFieldRejected(t *testing.T) {
282+
tracker := NewTracker(5 * time.Minute)
283+
handler := handlePostLocation(nil, tracker)
284+
285+
body := []byte(`{"vehicle_id":"bus-1","latitude":1,"longitude":2,"timestamp":100,"extra":"x"}`)
286+
w := postLocationWithBody(handler, body, "application/json")
287+
288+
assert.Equal(t, http.StatusBadRequest, w.Code)
289+
var resp map[string]string
290+
err := json.NewDecoder(w.Body).Decode(&resp)
291+
require.NoError(t, err)
292+
assert.Contains(t, resp["error"], "unknown field")
293+
}
294+
295+
func TestHandlePostLocation_TrailingJSONRejected(t *testing.T) {
296+
tracker := NewTracker(5 * time.Minute)
297+
handler := handlePostLocation(nil, tracker)
298+
299+
body := []byte(`{"vehicle_id":"bus-1","latitude":1,"longitude":2,"timestamp":100}{"x":1}`)
300+
w := postLocationWithBody(handler, body, "application/json")
301+
302+
assert.Equal(t, http.StatusBadRequest, w.Code)
303+
var resp map[string]string
304+
err := json.NewDecoder(w.Body).Decode(&resp)
305+
require.NoError(t, err)
306+
assert.Contains(t, resp["error"], "single JSON object")
307+
}
308+
309+
func TestHandlePostLocation_TrailingEmptyJSONObjectRejected(t *testing.T) {
310+
tracker := NewTracker(5 * time.Minute)
311+
handler := handlePostLocation(nil, tracker)
312+
313+
body := []byte(`{"vehicle_id":"bus-1","latitude":1,"longitude":2,"timestamp":100}{}`)
314+
w := postLocationWithBody(handler, body, "application/json")
315+
316+
assert.Equal(t, http.StatusBadRequest, w.Code)
317+
var resp map[string]string
318+
err := json.NewDecoder(w.Body).Decode(&resp)
319+
require.NoError(t, err)
320+
assert.Contains(t, resp["error"], "single JSON object")
321+
}
322+
323+
func TestHandlePostLocation_TrailingGarbageRejected(t *testing.T) {
324+
tracker := NewTracker(5 * time.Minute)
325+
handler := handlePostLocation(nil, tracker)
326+
327+
body := []byte(`{"vehicle_id":"bus-1","latitude":1,"longitude":2,"timestamp":100}GARBAGE`)
328+
w := postLocationWithBody(handler, body, "application/json")
329+
330+
assert.Equal(t, http.StatusBadRequest, w.Code)
331+
var resp map[string]string
332+
err := json.NewDecoder(w.Body).Decode(&resp)
333+
require.NoError(t, err)
334+
assert.Contains(t, resp["error"], "invalid JSON:")
335+
}
336+
337+
func TestHandlePostLocation_TrailingWhitespaceAccepted(t *testing.T) {
338+
tracker := NewTracker(5 * time.Minute)
339+
mStore := &mockStore{}
340+
handler := handlePostLocation(mStore, tracker)
341+
342+
body := []byte("{\"vehicle_id\":\"bus-1\",\"latitude\":1,\"longitude\":2,\"timestamp\":100} \n")
343+
w := postLocationWithBody(handler, body, "application/json")
344+
345+
assert.Equal(t, http.StatusCreated, w.Code)
346+
assert.True(t, mStore.saved, "location should be saved when only trailing whitespace exists")
347+
}

0 commit comments

Comments
 (0)