Add test for Unary-Get query parameter order

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

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=

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,35 @@ 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.
+// Unknown parameters are ignored for ordering purposes.
+func checkConnectGetQueryParamOrder(req *http.Request, feedback *feedbackPrinter) {
+	if req.Method != http.MethodGet {
+		return
+	}
+	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 {
+			actual = append(actual, name)
+		}
+	}
+	expected := slices.Clone(actual)
+	slices.SortStableFunc(expected, func(a, b string) int {
+		return connectGetQueryParamRank[a] - connectGetQueryParamRank[b]
+	})
+	if !slices.Equal(actual, expected) {
+		feedback.Printf("connect GET query parameters not in recommended order: got [%s]; expected [%s]",
+			strings.Join(actual, ", "), strings.Join(expected, ", "))
+	}
+}
+
 func checkTLS(req *http.Request, feedback *feedbackPrinter) {
 	tlsHeaderVal, _ := getHeader(req.Header, "X-Expect-Tls", feedback)
 	expectTLS, err := strconv.ParseBool(tlsHeaderVal)

internal/app/referenceserver/checks_test.go

@@ -0,0 +1,110 @@
+// 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"
+)
+
+// TestCheckConnectGetQueryParamOrder tests the check itself.
+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)
+			}
+		})
+	}
+}