From 00b692c0f911efaa4b41422e23adf3629cfc746c Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Thu, 9 Feb 2023 10:11:16 +0100 Subject: [PATCH] Chore: Validate batch query refIds (#63018) --- pkg/services/query/errors.go | 1 + pkg/services/query/models.go | 15 +++++++++++++++ pkg/services/query/query.go | 3 +-- pkg/services/query/query_test.go | 19 +++++++++++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/pkg/services/query/errors.go b/pkg/services/query/errors.go index f64d394757a..fbb3a50e1a7 100644 --- a/pkg/services/query/errors.go +++ b/pkg/services/query/errors.go @@ -9,4 +9,5 @@ var ( ErrInvalidDatasourceID = errutil.NewBase(errutil.StatusBadRequest, "query.invalidDatasourceId", errutil.WithPublicMessage("Query does not contain a valid data source identifier")).Errorf("invalid data source identifier") ErrMissingDataSourceInfo = errutil.NewBase(errutil.StatusBadRequest, "query.missingDataSourceInfo").MustTemplate("query missing datasource info: {{ .Public.RefId }}", errutil.WithPublic("Query {{ .Public.RefId }} is missing datasource information")) ErrQueryParamMismatch = errutil.NewBase(errutil.StatusBadRequest, "query.headerMismatch", errutil.WithPublicMessage("The request headers point to a different plugin than is defined in the request body")).Errorf("plugin header/body mismatch") + ErrDuplicateRefId = errutil.NewBase(errutil.StatusBadRequest, "query.duplicateRefId", errutil.WithPublicMessage("Multiple queries using the same RefId is not allowed ")).Errorf("multiple queries using the same RefId is not allowed") ) diff --git a/pkg/services/query/models.go b/pkg/services/query/models.go index bb8bf8db6f0..2dfed5ffd39 100644 --- a/pkg/services/query/models.go +++ b/pkg/services/query/models.go @@ -32,6 +32,21 @@ func (pr parsedRequest) getFlattenedQueries() []parsedQuery { } func (pr parsedRequest) validateRequest(ctx context.Context) error { + refIds := make(map[string]bool) + for _, pq := range pr.parsedQueries { + for _, q := range pq { + if refIds[q.query.RefID] { + return ErrDuplicateRefId + } + refIds[q.query.RefID] = true + } + } + + // Skip header validation. See https://github.com/grafana/grafana/pull/58871 + if true { + return nil + } + reqCtx := contexthandler.FromContext(ctx) if reqCtx == nil || reqCtx.Req == nil { diff --git a/pkg/services/query/query.go b/pkg/services/query/query.go index 5bcaaa0a1bc..bdbf99dfd79 100644 --- a/pkg/services/query/query.go +++ b/pkg/services/query/query.go @@ -295,8 +295,7 @@ func (s *Service) parseMetricRequest(ctx context.Context, user *user.SignedInUse }) } - _ = req.validateRequest(ctx) - return req, nil // TODO req.validateRequest() + return req, req.validateRequest(ctx) } func (s *Service) getDataSourceFromQuery(ctx context.Context, user *user.SignedInUser, skipCache bool, query *simplejson.Json, history map[string]*datasources.DataSource) (*datasources.DataSource, error) { diff --git a/pkg/services/query/query_test.go b/pkg/services/query/query_test.go index 53510380ead..c00d3e024fb 100644 --- a/pkg/services/query/query_test.go +++ b/pkg/services/query/query_test.go @@ -235,6 +235,25 @@ func TestParseMetricRequest(t *testing.T) { _, err = tc.queryService.parseMetricRequest(httpreq.Context(), tc.signedInUser, true, mr) require.NoError(t, err) }) + + t.Run("Test a duplicated refId", func(t *testing.T) { + tc := setup(t) + mr := metricRequestWithQueries(t, `{ + "refId": "A", + "datasource": { + "uid": "gIEkMvIVz", + "type": "postgres" + } + }`, `{ + "refId": "A", + "datasource": { + "uid": "gIEkMvIVz", + "type": "postgres" + } + }`) + _, err := tc.queryService.parseMetricRequest(context.Background(), tc.signedInUser, true, mr) + require.Error(t, err) + }) } func TestQueryDataMultipleSources(t *testing.T) {