all: consistently use stringsutil.JSONString() for formatting JSON strings with fmt.* functions instead of using "%q" formatter

The %q formatter may result in incorrectly formatted JSON string if the original string
contains special chars such as \x1b . They must be encoded as \u001b , otherwise the resulting JSON string
cannot be parsed by JSON parsers.

This is a follow-up for c0caa69939

See https://github.com/VictoriaMetrics/victorialogs-datasource/issues/24
This commit is contained in:
Aliaksandr Valialkin 2024-07-17 13:52:10 +02:00
parent f2362812c3
commit f8aa445945
No known key found for this signature in database
GPG key ID: 52C003EE2BCDB9EB
12 changed files with 70 additions and 29 deletions

View file

@ -230,7 +230,7 @@ func generateLogsAtTimestamp(bw *bufio.Writer, workerID int, ts int64, firstStre
for i := 0; i < activeStreams; i++ { for i := 0; i < activeStreams; i++ {
ip := toIPv4(rand.Uint32()) ip := toIPv4(rand.Uint32())
uuid := toUUID(rand.Uint64(), rand.Uint64()) uuid := toUUID(rand.Uint64(), rand.Uint64())
fmt.Fprintf(bw, `{"_time":%q,"_msg":"message for the stream %d and worker %d; ip=%s; uuid=%s; u64=%d","host":"host_%d","worker_id":"%d"`, fmt.Fprintf(bw, `{"_time":"%s","_msg":"message for the stream %d and worker %d; ip=%s; uuid=%s; u64=%d","host":"host_%d","worker_id":"%d"`,
timeStr, streamID, workerID, ip, uuid, rand.Uint64(), streamID, workerID) timeStr, streamID, workerID, ip, uuid, rand.Uint64(), streamID, workerID)
fmt.Fprintf(bw, `,"run_id":"%s"`, runID) fmt.Fprintf(bw, `,"run_id":"%s"`, runID)
for j := 0; j < *constFieldsPerLog; j++ { for j := 0; j < *constFieldsPerLog; j++ {

View file

@ -10,6 +10,8 @@ import (
"strings" "strings"
"time" "time"
"github.com/VictoriaMetrics/metrics"
"github.com/VictoriaMetrics/VictoriaMetrics/app/vmagent/csvimport" "github.com/VictoriaMetrics/VictoriaMetrics/app/vmagent/csvimport"
"github.com/VictoriaMetrics/VictoriaMetrics/app/vmagent/datadogsketches" "github.com/VictoriaMetrics/VictoriaMetrics/app/vmagent/datadogsketches"
"github.com/VictoriaMetrics/VictoriaMetrics/app/vmagent/datadogv1" "github.com/VictoriaMetrics/VictoriaMetrics/app/vmagent/datadogv1"
@ -42,7 +44,7 @@ import (
"github.com/VictoriaMetrics/VictoriaMetrics/lib/protoparser/common" "github.com/VictoriaMetrics/VictoriaMetrics/lib/protoparser/common"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/protoparser/opentelemetry/firehose" "github.com/VictoriaMetrics/VictoriaMetrics/lib/protoparser/opentelemetry/firehose"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/pushmetrics" "github.com/VictoriaMetrics/VictoriaMetrics/lib/pushmetrics"
"github.com/VictoriaMetrics/metrics" "github.com/VictoriaMetrics/VictoriaMetrics/lib/stringsutil"
) )
var ( var (
@ -450,7 +452,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
w.Header().Set("Content-Type", "application/json") w.Header().Set("Content-Type", "application/json")
var bb bytesutil.ByteBuffer var bb bytesutil.ByteBuffer
promscrape.WriteConfigData(&bb) promscrape.WriteConfigData(&bb)
fmt.Fprintf(w, `{"status":"success","data":{"yaml":%q}}`, bb.B) fmt.Fprintf(w, `{"status":"success","data":{"yaml":%s}}`, stringsutil.JSONString(string(bb.B)))
return true return true
case "/prometheus/-/reload", "/-/reload": case "/prometheus/-/reload", "/-/reload":
if !httpserver.CheckAuthFlag(w, r, reloadAuthKey) { if !httpserver.CheckAuthFlag(w, r, reloadAuthKey) {

View file

@ -9,6 +9,7 @@ import (
"time" "time"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/auth" "github.com/VictoriaMetrics/VictoriaMetrics/lib/auth"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/stringsutil"
) )
// ActiveQueriesHandler returns response to /api/v1/status/active_queries // ActiveQueriesHandler returns response to /api/v1/status/active_queries
@ -41,8 +42,8 @@ func writeActiveQueries(w http.ResponseWriter, aqes []activeQueryEntry) {
fmt.Fprintf(w, `{"status":"ok","data":[`) fmt.Fprintf(w, `{"status":"ok","data":[`)
for i, aqe := range aqes { for i, aqe := range aqes {
d := now.Sub(aqe.startTime) d := now.Sub(aqe.startTime)
fmt.Fprintf(w, `{"duration":"%.3fs","id":"%016X","remote_addr":%s,"account_id":"%d","project_id":"%d","query":%q,"start":%d,"end":%d,"step":%d}`, fmt.Fprintf(w, `{"duration":"%.3fs","id":"%016X","remote_addr":%s,"account_id":"%d","project_id":"%d","query":%s,"start":%d,"end":%d,"step":%d}`,
d.Seconds(), aqe.qid, aqe.quotedRemoteAddr, aqe.accountID, aqe.projectID, aqe.q, aqe.start, aqe.end, aqe.step) d.Seconds(), aqe.qid, aqe.quotedRemoteAddr, aqe.accountID, aqe.projectID, stringsutil.JSONString(aqe.q), aqe.start, aqe.end, aqe.step)
if i+1 < len(aqes) { if i+1 < len(aqes) {
fmt.Fprintf(w, `,`) fmt.Fprintf(w, `,`)
} }

View file

@ -9,6 +9,7 @@ import (
"time" "time"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/stringsutil"
) )
var ( var (
@ -93,13 +94,13 @@ func initQueryStats() {
} }
func (qst *queryStatsTracker) writeJSONQueryStats(w io.Writer, topN int, apFilter *accountProjectFilter, maxLifetime time.Duration) { func (qst *queryStatsTracker) writeJSONQueryStats(w io.Writer, topN int, apFilter *accountProjectFilter, maxLifetime time.Duration) {
fmt.Fprintf(w, `{"topN":"%d","maxLifetime":%q,`, topN, maxLifetime) fmt.Fprintf(w, `{"topN":"%d","maxLifetime":"%s",`, topN, maxLifetime)
fmt.Fprintf(w, `"search.queryStats.lastQueriesCount":%d,`, *lastQueriesCount) fmt.Fprintf(w, `"search.queryStats.lastQueriesCount":%d,`, *lastQueriesCount)
fmt.Fprintf(w, `"search.queryStats.minQueryDuration":%q,`, *minQueryDuration) fmt.Fprintf(w, `"search.queryStats.minQueryDuration":"%s",`, *minQueryDuration)
fmt.Fprintf(w, `"topByCount":[`) fmt.Fprintf(w, `"topByCount":[`)
topByCount := qst.getTopByCount(topN, apFilter, maxLifetime) topByCount := qst.getTopByCount(topN, apFilter, maxLifetime)
for i, r := range topByCount { for i, r := range topByCount {
fmt.Fprintf(w, `{"accountID":%d,"projectID":%d,"query":%q,"timeRangeSeconds":%d,"count":%d}`, r.accountID, r.projectID, r.query, r.timeRangeSecs, r.count) fmt.Fprintf(w, `{"accountID":%d,"projectID":%d,"query":%s,"timeRangeSeconds":%d,"count":%d}`, r.accountID, r.projectID, stringsutil.JSONString(r.query), r.timeRangeSecs, r.count)
if i+1 < len(topByCount) { if i+1 < len(topByCount) {
fmt.Fprintf(w, `,`) fmt.Fprintf(w, `,`)
} }
@ -107,8 +108,8 @@ func (qst *queryStatsTracker) writeJSONQueryStats(w io.Writer, topN int, apFilte
fmt.Fprintf(w, `],"topByAvgDuration":[`) fmt.Fprintf(w, `],"topByAvgDuration":[`)
topByAvgDuration := qst.getTopByAvgDuration(topN, apFilter, maxLifetime) topByAvgDuration := qst.getTopByAvgDuration(topN, apFilter, maxLifetime)
for i, r := range topByAvgDuration { for i, r := range topByAvgDuration {
fmt.Fprintf(w, `{"accountID":%d,"projectID":%d,"query":%q,"timeRangeSeconds":%d,"avgDurationSeconds":%.3f,"count":%d}`, fmt.Fprintf(w, `{"accountID":%d,"projectID":%d,"query":%s,"timeRangeSeconds":%d,"avgDurationSeconds":%.3f,"count":%d}`,
r.accountID, r.projectID, r.query, r.timeRangeSecs, r.duration.Seconds(), r.count) r.accountID, r.projectID, stringsutil.JSONString(r.query), r.timeRangeSecs, r.duration.Seconds(), r.count)
if i+1 < len(topByAvgDuration) { if i+1 < len(topByAvgDuration) {
fmt.Fprintf(w, `,`) fmt.Fprintf(w, `,`)
} }
@ -116,8 +117,8 @@ func (qst *queryStatsTracker) writeJSONQueryStats(w io.Writer, topN int, apFilte
fmt.Fprintf(w, `],"topBySumDuration":[`) fmt.Fprintf(w, `],"topBySumDuration":[`)
topBySumDuration := qst.getTopBySumDuration(topN, apFilter, maxLifetime) topBySumDuration := qst.getTopBySumDuration(topN, apFilter, maxLifetime)
for i, r := range topBySumDuration { for i, r := range topBySumDuration {
fmt.Fprintf(w, `{"accountID":%d,"projectID":%d,"query":%q,"timeRangeSeconds":%d,"sumDurationSeconds":%.3f,"count":%d}`, fmt.Fprintf(w, `{"accountID":%d,"projectID":%d,"query":%s,"timeRangeSeconds":%d,"sumDurationSeconds":%.3f,"count":%d}`,
r.accountID, r.projectID, r.query, r.timeRangeSecs, r.duration.Seconds(), r.count) r.accountID, r.projectID, stringsutil.JSONString(r.query), r.timeRangeSecs, r.duration.Seconds(), r.count)
if i+1 < len(topBySumDuration) { if i+1 < len(topBySumDuration) {
fmt.Fprintf(w, `,`) fmt.Fprintf(w, `,`)
} }

View file

@ -24,6 +24,7 @@ import (
"github.com/VictoriaMetrics/VictoriaMetrics/lib/protoparser/common" "github.com/VictoriaMetrics/VictoriaMetrics/lib/protoparser/common"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/pushmetrics" "github.com/VictoriaMetrics/VictoriaMetrics/lib/pushmetrics"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/storage" "github.com/VictoriaMetrics/VictoriaMetrics/lib/storage"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/stringsutil"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/timeutil" "github.com/VictoriaMetrics/VictoriaMetrics/lib/timeutil"
) )
@ -236,7 +237,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request, strg *storage.Storag
snapshotsCreateErrorsTotal.Inc() snapshotsCreateErrorsTotal.Inc()
return true return true
} }
fmt.Fprintf(w, `{"status":"ok","snapshot":%q}`, snapshotPath) fmt.Fprintf(w, `{"status":"ok","snapshot":%s}`, stringsutil.JSONString(snapshotPath))
return true return true
case "/list": case "/list":
snapshotsListTotal.Inc() snapshotsListTotal.Inc()
@ -561,7 +562,8 @@ func writeStorageMetrics(w io.Writer, strg *storage.Storage) {
func jsonResponseError(w http.ResponseWriter, err error) { func jsonResponseError(w http.ResponseWriter, err error) {
logger.Errorf("%s", err) logger.Errorf("%s", err)
w.WriteHeader(http.StatusInternalServerError) w.WriteHeader(http.StatusInternalServerError)
fmt.Fprintf(w, `{"status":"error","msg":%q}`, err) errStr := err.Error()
fmt.Fprintf(w, `{"status":"error","msg":%s}`, stringsutil.JSONString(errStr))
} }
func usage() { func usage() {

View file

@ -20,14 +20,16 @@ import (
"sync/atomic" "sync/atomic"
"time" "time"
"github.com/VictoriaMetrics/metrics"
"github.com/klauspost/compress/gzhttp"
"github.com/valyala/fastrand"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/appmetrics" "github.com/VictoriaMetrics/VictoriaMetrics/lib/appmetrics"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime" "github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/flagutil" "github.com/VictoriaMetrics/VictoriaMetrics/lib/flagutil"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/netutil" "github.com/VictoriaMetrics/VictoriaMetrics/lib/netutil"
"github.com/VictoriaMetrics/metrics" "github.com/VictoriaMetrics/VictoriaMetrics/lib/stringsutil"
"github.com/klauspost/compress/gzhttp"
"github.com/valyala/fastrand"
) )
var ( var (
@ -533,7 +535,7 @@ func GetQuotedRemoteAddr(r *http.Request) string {
remoteAddr += ", X-Forwarded-For: " + addr remoteAddr += ", X-Forwarded-For: " + addr
} }
// quote remoteAddr and X-Forwarded-For, since they may contain untrusted input // quote remoteAddr and X-Forwarded-For, since they may contain untrusted input
return strconv.Quote(remoteAddr) return stringsutil.JSONString(remoteAddr)
} }
type responseWriterWithAbort struct { type responseWriterWithAbort struct {

View file

@ -3,7 +3,6 @@ package promscrape
import ( import (
"fmt" "fmt"
"reflect" "reflect"
"strconv"
"strings" "strings"
"testing" "testing"
"time" "time"
@ -13,6 +12,7 @@ import (
"github.com/VictoriaMetrics/VictoriaMetrics/lib/promscrape/discovery/gce" "github.com/VictoriaMetrics/VictoriaMetrics/lib/promscrape/discovery/gce"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/promutils" "github.com/VictoriaMetrics/VictoriaMetrics/lib/promutils"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/proxy" "github.com/VictoriaMetrics/VictoriaMetrics/lib/proxy"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/stringsutil"
) )
func TestMergeLabels(t *testing.T) { func TestMergeLabels(t *testing.T) {
@ -432,7 +432,7 @@ scrape_configs:
// String returns human-readable representation for sw. // String returns human-readable representation for sw.
func (sw *ScrapeWork) String() string { func (sw *ScrapeWork) String() string {
return strconv.Quote(sw.key()) return stringsutil.JSONString(sw.key())
} }
func TestGetFileSDScrapeWorkSuccess(t *testing.T) { func TestGetFileSDScrapeWorkSuccess(t *testing.T) {

View file

@ -19,6 +19,7 @@ import (
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/promrelabel" "github.com/VictoriaMetrics/VictoriaMetrics/lib/promrelabel"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/promutils" "github.com/VictoriaMetrics/VictoriaMetrics/lib/promutils"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/stringsutil"
) )
var maxDroppedTargets = flag.Int("promscrape.maxDroppedTargets", 10000, "The maximum number of droppedTargets to show at /api/v1/targets page. "+ var maxDroppedTargets = flag.Int("promscrape.maxDroppedTargets", 10000, "The maximum number of droppedTargets to show at /api/v1/targets page. "+
@ -261,21 +262,21 @@ func (tsm *targetStatusMap) WriteActiveTargetsJSON(w io.Writer) {
writeLabelsJSON(w, ts.sw.Config.OriginalLabels) writeLabelsJSON(w, ts.sw.Config.OriginalLabels)
fmt.Fprintf(w, `,"labels":`) fmt.Fprintf(w, `,"labels":`)
writeLabelsJSON(w, ts.sw.Config.Labels) writeLabelsJSON(w, ts.sw.Config.Labels)
fmt.Fprintf(w, `,"scrapePool":%q`, ts.sw.Config.Job()) fmt.Fprintf(w, `,"scrapePool":%s`, stringsutil.JSONString(ts.sw.Config.Job()))
fmt.Fprintf(w, `,"scrapeUrl":%q`, ts.sw.Config.ScrapeURL) fmt.Fprintf(w, `,"scrapeUrl":%s`, stringsutil.JSONString(ts.sw.Config.ScrapeURL))
errMsg := "" errMsg := ""
if ts.err != nil { if ts.err != nil {
errMsg = ts.err.Error() errMsg = ts.err.Error()
} }
fmt.Fprintf(w, `,"lastError":%q`, errMsg) fmt.Fprintf(w, `,"lastError":%s`, stringsutil.JSONString(errMsg))
fmt.Fprintf(w, `,"lastScrape":%q`, time.Unix(ts.scrapeTime/1000, (ts.scrapeTime%1000)*1e6).Format(time.RFC3339Nano)) fmt.Fprintf(w, `,"lastScrape":"%s"`, time.Unix(ts.scrapeTime/1000, (ts.scrapeTime%1000)*1e6).Format(time.RFC3339Nano))
fmt.Fprintf(w, `,"lastScrapeDuration":%g`, (time.Millisecond * time.Duration(ts.scrapeDuration)).Seconds()) fmt.Fprintf(w, `,"lastScrapeDuration":%g`, (time.Millisecond * time.Duration(ts.scrapeDuration)).Seconds())
fmt.Fprintf(w, `,"lastSamplesScraped":%d`, ts.samplesScraped) fmt.Fprintf(w, `,"lastSamplesScraped":%d`, ts.samplesScraped)
state := "up" state := "up"
if !ts.up { if !ts.up {
state = "down" state = "down"
} }
fmt.Fprintf(w, `,"health":%q}`, state) fmt.Fprintf(w, `,"health":%s}`, stringsutil.JSONString(state))
if i+1 < len(tss) { if i+1 < len(tss) {
fmt.Fprintf(w, `,`) fmt.Fprintf(w, `,`)
} }
@ -287,7 +288,7 @@ func writeLabelsJSON(w io.Writer, labels *promutils.Labels) {
fmt.Fprintf(w, `{`) fmt.Fprintf(w, `{`)
labelsList := labels.GetLabels() labelsList := labels.GetLabels()
for i, label := range labelsList { for i, label := range labelsList {
fmt.Fprintf(w, "%q:%q", label.Name, label.Value) fmt.Fprintf(w, "%s:%s", stringsutil.JSONString(label.Name), stringsutil.JSONString(label.Value))
if i+1 < len(labelsList) { if i+1 < len(labelsList) {
fmt.Fprintf(w, `,`) fmt.Fprintf(w, `,`)
} }

View file

@ -4,6 +4,8 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"time" "time"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/stringsutil"
) )
// WriteSuccessResponse writes success response for AWS Firehose request. // WriteSuccessResponse writes success response for AWS Firehose request.
@ -17,7 +19,7 @@ func WriteSuccessResponse(w http.ResponseWriter, r *http.Request) {
return return
} }
body := fmt.Sprintf(`{"requestId":%q,"timestamp":%d}`, requestID, time.Now().UnixMilli()) body := fmt.Sprintf(`{"requestId":%s,"timestamp":%d}`, stringsutil.JSONString(requestID), time.Now().UnixMilli())
h := w.Header() h := w.Header()
h.Set("Content-Type", "application/json") h.Set("Content-Type", "application/json")

View file

@ -2,12 +2,12 @@ package streamaggr
import ( import (
"fmt" "fmt"
"strconv"
"strings" "strings"
"testing" "testing"
"time" "time"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/prompbmarshal" "github.com/VictoriaMetrics/VictoriaMetrics/lib/prompbmarshal"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/stringsutil"
) )
var benchOutputs = []string{ var benchOutputs = []string{
@ -78,7 +78,7 @@ func benchmarkAggregatorsPush(b *testing.B, output string) {
func newBenchAggregators(outputs []string, pushFunc PushFunc) *Aggregators { func newBenchAggregators(outputs []string, pushFunc PushFunc) *Aggregators {
outputsQuoted := make([]string, len(outputs)) outputsQuoted := make([]string, len(outputs))
for i := range outputs { for i := range outputs {
outputsQuoted[i] = strconv.Quote(outputs[i]) outputsQuoted[i] = stringsutil.JSONString(outputs[i])
} }
config := fmt.Sprintf(` config := fmt.Sprintf(`
- match: http_requests_total - match: http_requests_total

10
lib/stringsutil/json.go Normal file
View file

@ -0,0 +1,10 @@
package stringsutil
import (
"github.com/valyala/quicktemplate"
)
// JSONString returns JSON-quoted s.
func JSONString(s string) string {
return string(quicktemplate.AppendJSONString(nil, s, true))
}

View file

@ -0,0 +1,20 @@
package stringsutil
import (
"testing"
)
func TestJSONString(t *testing.T) {
f := func(s, resultExpected string) {
t.Helper()
result := JSONString(s)
if result != resultExpected {
t.Fatalf("unexpected result\ngot\n%s\nwant\n%s", result, resultExpected)
}
}
f(``, `""`)
f(`foo`, `"foo"`)
f("\n\b\f\t\"acЫВА'\\", `"\n\b\f\t\"acЫВА\u0027\\"`)
}