From d94229bd456f372d03af323e9cf0f9098ba581d4 Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Tue, 19 May 2026 16:04:29 -0400 Subject: [PATCH 1/3] Add test for Unary-Get query parameter order Signed-off-by: Oliver Sun --- internal/app/referenceserver/checks.go | 49 +++++++++ internal/app/referenceserver/checks_test.go | 109 ++++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 internal/app/referenceserver/checks_test.go diff --git a/internal/app/referenceserver/checks.go b/internal/app/referenceserver/checks.go index 15bbc27a..1d326e40 100644 --- a/internal/app/referenceserver/checks.go +++ b/internal/app/referenceserver/checks.go @@ -21,6 +21,7 @@ import ( "math" "net/http" "net/url" + "slices" "strconv" "strings" "sync" @@ -49,6 +50,21 @@ const ( codecJSON = "json" ) +// connectGetQueryParamRank is the recommended ordering of Connect-defined +// query parameters in Unary-Get requests, as zero-based ranks. Clients should +// emit parameters in this order to maximize cache hit rates on shared caches; +// servers must accept any order. See the Query-Get rule in the Connect +// protocol spec. +// +//nolint:gochecknoglobals // canonical lookup table, not mutable state +var connectGetQueryParamRank = map[string]int{ + "connect": 0, + "base64": 1, + "compression": 2, + "encoding": 3, + "message": 4, +} + type int32Enum interface { ~int32 protoreflect.Enum @@ -93,6 +109,9 @@ func referenceServerChecks(handler http.Handler, errPrinter internal.Printer) ht } if protocol, ok := enumValue("X-Expect-Protocol", req.Header, conformancev1.Protocol(0), feedback); ok { checkProtocol(protocol, req, feedback) + if protocol == conformancev1.Protocol_PROTOCOL_CONNECT { + checkConnectGetQueryParamOrder(req, feedback) + } if timeout, ok := extractTimeout(req.Header, protocol, feedback); ok { // In reference mode, we *remove* the timeout in this middleware so that the server // will NOT enforce it. That way, we can test that the client is actually enforcing it. @@ -279,6 +298,36 @@ func checkCompression(expected conformancev1.Compression, req *http.Request, fee } } +// checkConnectGetQueryParamOrder verifies that a Connect Unary-Get request +// orders its Connect-defined query parameters per the protocol recommendation: +// connect, base64, compression, encoding, message. Unknown parameters and any +// non-Connect parameters are ignored for ordering purposes. This is a SHOULD +// rule for clients (servers MUST accept any order), so it only runs for the +// reference server's expected Connect protocol GET requests. +func checkConnectGetQueryParamOrder(req *http.Request, feedback *feedbackPrinter) { + if req.Method != http.MethodGet { + return + } + var observed []string + for pair := range strings.SplitSeq(req.URL.RawQuery, "&") { + if pair == "" { + continue + } + name, _, _ := strings.Cut(pair, "=") + if _, ok := connectGetQueryParamRank[name]; ok { + observed = append(observed, name) + } + } + expected := slices.Clone(observed) + slices.SortStableFunc(expected, func(a, b string) int { + return connectGetQueryParamRank[a] - connectGetQueryParamRank[b] + }) + if !slices.Equal(observed, expected) { + feedback.Printf("connect GET query parameters not in recommended order: got [%s]; expected [%s]", + strings.Join(observed, ", "), strings.Join(expected, ", ")) + } +} + func checkTLS(req *http.Request, feedback *feedbackPrinter) { tlsHeaderVal, _ := getHeader(req.Header, "X-Expect-Tls", feedback) expectTLS, err := strconv.ParseBool(tlsHeaderVal) diff --git a/internal/app/referenceserver/checks_test.go b/internal/app/referenceserver/checks_test.go new file mode 100644 index 00000000..a644a83b --- /dev/null +++ b/internal/app/referenceserver/checks_test.go @@ -0,0 +1,109 @@ +// Copyright 2023-2024 The Connect Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package referenceserver + +import ( + "net/http" + "net/url" + "testing" + + "connectrpc.com/conformance/internal" + "github.com/stretchr/testify/assert" +) + +func TestCheckConnectGetQueryParamOrder(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + method string + rawQuery string + expectedError string + }{ + { + name: "non-GET request is ignored", + method: http.MethodPost, + rawQuery: "message=foo&encoding=proto&connect=v1", + }, + { + name: "empty query", + method: http.MethodGet, + rawQuery: "", + }, + { + name: "encoding and message only, correct order", + method: http.MethodGet, + rawQuery: "encoding=proto&message=AAAA", + }, + { + name: "all five parameters in spec order", + method: http.MethodGet, + rawQuery: "connect=v1&base64=1&compression=gzip&encoding=proto&message=AAAA", + }, + { + name: "version plus encoding plus message", + method: http.MethodGet, + rawQuery: "connect=v1&encoding=proto&message=AAAA", + }, + { + name: "alphabetical order (connect-go default) flags", + method: http.MethodGet, + rawQuery: "base64=1&compression=gzip&connect=v1&encoding=proto&message=AAAA", + expectedError: "got [base64, compression, connect, encoding, message]", + }, + { + name: "message before encoding flags", + method: http.MethodGet, + rawQuery: "message=AAAA&encoding=proto", + expectedError: "got [message, encoding]", + }, + { + name: "unknown parameters are ignored", + method: http.MethodGet, + rawQuery: "x-trace=abc&connect=v1&extra=1&encoding=proto&message=AAAA", + }, + { + name: "unknown parameters do not mask misorder", + method: http.MethodGet, + rawQuery: "encoding=proto&x-trace=abc&connect=v1&message=AAAA", + expectedError: "got [encoding, connect, message]", + }, + { + name: "bare parameter name without value", + method: http.MethodGet, + rawQuery: "connect&encoding=proto&message=AAAA", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + printer := &internal.SimplePrinter{} + feedback := &feedbackPrinter{p: printer, testCaseName: testCase.name} + req := &http.Request{ + Method: testCase.method, + URL: &url.URL{RawQuery: testCase.rawQuery}, + } + checkConnectGetQueryParamOrder(req, feedback) + if testCase.expectedError == "" { + assert.Empty(t, printer.Messages) + return + } + if assert.Len(t, printer.Messages, 1) { + assert.Contains(t, printer.Messages[0], testCase.expectedError) + } + }) + } +} From c126da61e397442d9f5e2a502ac71b2a32532d73 Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Thu, 21 May 2026 11:42:17 -0400 Subject: [PATCH 2/3] update connect-go to v1.20.0 Signed-off-by: Oliver Sun --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index f9596069..0f5b29ea 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.25.0 require ( buf.build/go/protoyaml v0.7.0 - connectrpc.com/connect v1.19.2 + connectrpc.com/connect v1.20.0 github.com/andybalholm/brotli v1.2.1 github.com/golang/snappy v1.0.0 github.com/google/go-cmp v0.7.0 diff --git a/go.sum b/go.sum index 97e2b2bb..227d43bf 100644 --- a/go.sum +++ b/go.sum @@ -8,8 +8,8 @@ cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= -connectrpc.com/connect v1.19.2 h1:McQ83FGdzL+t60peksi0gXC7MQ/iLKgLduAnThbM0mo= -connectrpc.com/connect v1.19.2/go.mod h1:tN20fjdGlewnSFeZxLKb0xwIZ6ozc3OQs2hTXy4du9w= +connectrpc.com/connect v1.20.0 h1:6TNDAB+WeNd2uolWNlYczB5E0KNNaVMNUEx8JEUsPmQ= +connectrpc.com/connect v1.20.0/go.mod h1:A2ygJrukXwWy32vkCAAHNVguZrqZ+jeZ9rGRnGR4dN4= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= From 276f628c4472dff3d2524df9c909f0f9ce7e6cae Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Thu, 21 May 2026 12:56:38 -0400 Subject: [PATCH 3/3] nits Signed-off-by: Oliver Sun --- internal/app/referenceserver/checks.go | 19 +++++++++---------- internal/app/referenceserver/checks_test.go | 1 + 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/app/referenceserver/checks.go b/internal/app/referenceserver/checks.go index 1d326e40..948f144e 100644 --- a/internal/app/referenceserver/checks.go +++ b/internal/app/referenceserver/checks.go @@ -299,32 +299,31 @@ func checkCompression(expected conformancev1.Compression, req *http.Request, fee } // checkConnectGetQueryParamOrder verifies that a Connect Unary-Get request -// orders its Connect-defined query parameters per the protocol recommendation: -// connect, base64, compression, encoding, message. Unknown parameters and any -// non-Connect parameters are ignored for ordering purposes. This is a SHOULD -// rule for clients (servers MUST accept any order), so it only runs for the -// reference server's expected Connect protocol GET requests. +// orders its Connect-defined query parameters per the protocol recommendation. +// Unknown parameters are ignored for ordering purposes. func checkConnectGetQueryParamOrder(req *http.Request, feedback *feedbackPrinter) { if req.Method != http.MethodGet { return } - var observed []string + var actual []string for pair := range strings.SplitSeq(req.URL.RawQuery, "&") { if pair == "" { continue } name, _, _ := strings.Cut(pair, "=") + // We could only test the ranks are monotically increasing, but we keep + // actual as a slice so that we can also print expected as a slice. if _, ok := connectGetQueryParamRank[name]; ok { - observed = append(observed, name) + actual = append(actual, name) } } - expected := slices.Clone(observed) + expected := slices.Clone(actual) slices.SortStableFunc(expected, func(a, b string) int { return connectGetQueryParamRank[a] - connectGetQueryParamRank[b] }) - if !slices.Equal(observed, expected) { + if !slices.Equal(actual, expected) { feedback.Printf("connect GET query parameters not in recommended order: got [%s]; expected [%s]", - strings.Join(observed, ", "), strings.Join(expected, ", ")) + strings.Join(actual, ", "), strings.Join(expected, ", ")) } } diff --git a/internal/app/referenceserver/checks_test.go b/internal/app/referenceserver/checks_test.go index a644a83b..4d4f8908 100644 --- a/internal/app/referenceserver/checks_test.go +++ b/internal/app/referenceserver/checks_test.go @@ -23,6 +23,7 @@ import ( "github.com/stretchr/testify/assert" ) +// TestCheckConnectGetQueryParamOrder tests the check itself. func TestCheckConnectGetQueryParamOrder(t *testing.T) { t.Parallel()