diff --git a/app/vmselect/searchutil/searchutil.go b/app/vmselect/searchutil/searchutil.go index 97a1122ce0..fd69c6c2f4 100644 --- a/app/vmselect/searchutil/searchutil.go +++ b/app/vmselect/searchutil/searchutil.go @@ -132,9 +132,20 @@ func (d *Deadline) String() string { // // {env="prod",team="devops",t1="v1",t2="v2"} // {env=~"dev|staging",team!="devops",t1="v1",t2="v2"} +// +// Query args from URL path have precedence over post form args. func GetExtraTagFilters(r *http.Request) ([][]storage.TagFilter, error) { var tagFilters []storage.TagFilter - for _, match := range r.Form["extra_label"] { + urlQueryValues := r.URL.Query() + getRequestParam := func(key string) []string { + // query request param must always take precedence over form values + // in order to simplify security enforcement policy for extra_label and extra_filters + if uv, ok := urlQueryValues[key]; ok { + return uv + } + return r.Form[key] + } + for _, match := range getRequestParam("extra_label") { tmp := strings.SplitN(match, "=", 2) if len(tmp) != 2 { return nil, fmt.Errorf("`extra_label` query arg must have the format `name=value`; got %q", match) @@ -148,8 +159,8 @@ func GetExtraTagFilters(r *http.Request) ([][]storage.TagFilter, error) { Value: []byte(tmp[1]), }) } - extraFilters := append([]string{}, r.Form["extra_filters"]...) - extraFilters = append(extraFilters, r.Form["extra_filters[]"]...) + extraFilters := append([]string{}, getRequestParam("extra_filters")...) + extraFilters = append(extraFilters, getRequestParam("extra_filters[]")...) if len(extraFilters) == 0 { if len(tagFilters) == 0 { return nil, nil diff --git a/app/vmselect/searchutil/searchutil_test.go b/app/vmselect/searchutil/searchutil_test.go index 640e557d9c..00f061a7a4 100644 --- a/app/vmselect/searchutil/searchutil_test.go +++ b/app/vmselect/searchutil/searchutil_test.go @@ -20,6 +20,7 @@ func TestGetExtraTagFilters(t *testing.T) { } return &http.Request{ Form: q, + URL: &url.URL{RawQuery: q.Encode()}, } } f := func(t *testing.T, r *http.Request, want []string, wantErr bool) { @@ -79,6 +80,24 @@ func TestGetExtraTagFilters(t *testing.T) { nil, false, ) + + formValues, err := url.ParseQuery(`extra_label=env=prod&extra_label=job=vmsingle&extra_label=tenant=prod&extra_filters[]={foo="bar"}&extra_filters[]={tenant="prod"}`) + if err != nil { + t.Fatalf("BUG: cannot parse query: %s", err) + } + urlValues, err := url.ParseQuery(`extra_label=job=vmagent&extra_label=env=dev&extra_filters[]={tenant="dev"}`) + if err != nil { + t.Fatalf("BUG: cannot parse query: %s", err) + } + httpReqWithBothFormAndURLParams := &http.Request{ + Form: formValues, + URL: &url.URL{ + RawQuery: urlValues.Encode(), + }, + } + f(t, httpReqWithBothFormAndURLParams, + []string{`{tenant="dev",job="vmagent",env="dev"}`}, + false) } func TestParseMetricSelectorSuccess(t *testing.T) { diff --git a/docs/victoriametrics/changelog/CHANGELOG.md b/docs/victoriametrics/changelog/CHANGELOG.md index 3285a632c7..c684db8ded 100644 --- a/docs/victoriametrics/changelog/CHANGELOG.md +++ b/docs/victoriametrics/changelog/CHANGELOG.md @@ -41,6 +41,7 @@ See also [LTS releases](https://docs.victoriametrics.com/victoriametrics/lts-rel * BUGFIX: [vmauth](https://docs.victoriametrics.com/victoriametrics/vmauth/): now correctly respects disabling retries via `-maxRequestBodySizeToRetry=0`. Previously, disabling retries required setting both `-maxRequestBodySizeToRetry` and `-requestBufferSize` to `0`. See [#10857](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10857). Thanks to @andriibeee for the contribution. * BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/victoriametrics/vmbackupmanager/): explicitly set the MD5 checksum for `DeleteObjects` requests. Starting with [aws-sdk-go-v2/service/s3 v1.73.0](https://github.com/aws/aws-sdk-go-v2/blob/release-2025-01-15/service/s3/CHANGELOG.md#v1730-2025-01-15), the SDK switched the checksum algorithm from MD5 to CRC32, which [can break compatibility](https://github.com/aws/aws-sdk-go-v2/discussions/2960) with some third-party S3 implementations, including [Dell ECS](https://www.dell.com/en-us/lp/dt/elastic-cloud-storage). See [#10907](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10907). * BUGFIX: [vmauth](https://docs.victoriametrics.com/victoriametrics/vmauth/): attempt to route requests to the first backend when all backends are marked as broken. Previously, `vmauth` returned an error immediately without attempting any backend. This change may improve success rate in rare edge cases. See [#10837](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10837). +* BUGFIX: [vmsingle](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/): and `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/): give priority to `extra_label` and `extra_filters[]` params defined in URL query string over those defined in request body form, to properly respect constraints imposed by `vmauth`. See [#10908](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10908). * BUGFIX: [vmsingle](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/) and `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/): set CORS headers on `/api/v1/export`, `/api/v1/export/csv` and `/api/v1/export/native`. See [#10899](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10899). Thanks to @andriibeee for the contribution. ## [v1.142.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.142.0)