Repository: deepmap/oapi-codegen
Pull request #280 (2021-01-14 01:35:30)

Fix invalid query string due to reserved char in query value issue 
### Changes
- Escape primitive string when style param 
- Add unit tests 


Closes https://github.com/deepmap/oapi-codegen/issues/279

deepmap-marcinr: I'm happy to merge this, thanks for the fix, could you resolve the conflicts?

Patch:
diff --git a/pkg/runtime/styleparam.go b/pkg/runtime/styleparam.go
index 30c285a..638e834 100644
--- a/pkg/runtime/styleparam.go
+++ b/pkg/runtime/styleparam.go
@@ -158,8 +158,9 @@ func marshalDateTimeValue(value interface{}) (string, bool) {
 }
 
 func styleStruct(style string, explode bool, paramName string, value interface{}) (string, error) {
+
 	if timeVal, ok := marshalDateTimeValue(value); ok {
-		styledVal, err := stylePrimitive(style, explode, paramName, url.QueryEscape(timeVal))
+		styledVal, err := stylePrimitive(style, explode, paramName, timeVal)
 		if err != nil {
 			return "", errors.Wrap(err, "failed to style time")
 		}
@@ -318,7 +319,7 @@ func stylePrimitive(style string, explode bool, paramName string, value interfac
 	default:
 		return "", fmt.Errorf("unsupported style '%s'", style)
 	}
-	return prefix + strVal, nil
+	return prefix + url.QueryEscape(strVal), nil
 }
 
 // Converts a primitive value to a string. We need to do this based on the


Test patch:
diff --git a/internal/test/parameters/parameters.gen.go b/internal/test/parameters/parameters.gen.go
index 9f3f096..a8aab48 100644
--- a/internal/test/parameters/parameters.gen.go
+++ b/internal/test/parameters/parameters.gen.go
@@ -111,6 +111,9 @@ type GetQueryFormParams struct {
 	// primitive
 	P *int32 `json:"p,omitempty"`
 
+	// primitive string
+	Ps *string `json:"ps,omitempty"`
+
 	// complex object
 	Co *ComplexObject `json:"co,omitempty"`
 }
@@ -1270,6 +1273,22 @@ func NewGetQueryFormRequest(server string, params *GetQueryFormParams) (*http.Re
 
 	}
 
+	if params.Ps != nil {
+
+		if queryFrag, err := runtime.StyleParam("form", true, "ps", *params.Ps); err != nil {
+			return nil, err
+		} else if parsed, err := url.ParseQuery(queryFrag); err != nil {
+			return nil, err
+		} else {
+			for k, v := range parsed {
+				for _, v2 := range v {
+					queryValues.Add(k, v2)
+				}
+			}
+		}
+
+	}
+
 	if params.Co != nil {
 
 		if queryParamBuf, err := json.Marshal(*params.Co); err != nil {
@@ -2980,6 +2999,13 @@ func (w *ServerInterfaceWrapper) GetQueryForm(ctx echo.Context) error {
 		return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Invalid format for parameter p: %s", err))
 	}
 
+	// ------------- Optional query parameter "ps" -------------
+
+	err = runtime.BindQueryParameter("form", true, false, "ps", ctx.QueryParams(), &params.Ps)
+	if err != nil {
+		return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Invalid format for parameter ps: %s", err))
+	}
+
 	// ------------- Optional query parameter "co" -------------
 
 	if paramValue := ctx.QueryParam("co"); paramValue != "" {
@@ -3131,23 +3157,24 @@ func RegisterHandlersWithBaseURL(router EchoRouter, si ServerInterface, baseURL
 // Base64 encoded, gzipped, json marshaled Swagger object
 var swaggerSpec = []string{
 
-	"H4sIAAAAAAAC/9xZS2/jNhD+K8a0p0JreXdvugXbV4Duo3UOBRY5MNI45lYSuSQdJDD03wuSelIPS7aV",
-	"OHuzpeF8832cGQypPYQs4SzFVEkI9iBQcpZKNH/WNOEx/pM/0k9ClipMlf6p8FH5PCY01f9kuMWEmOdP",
-	"HCEAqQRN7yHLMg8ilKGgXFGWQgBXC2n8LgqsBbv7hqECbWr9GPQPTFs9frYvgz1wwTgKRW1w11ENjaYK",
-	"71FA5sG1vIoSG1T+8o6xGEmqX1bOfha4gQB+8iv+fg7uf67iEfh9RwVGEHwtFnsausK5bbhtxrihQqpP",
-	"JMEOYTwQLO564aAaK6/m6tZoStMN04tjGmK+OakBgo/XN9q7okq7hxuUarFG8YACPHhAIe02vF2ulitt",
-	"yDimhFMI4P1ytXwLHnCitiZ+P99vy8/fcyJIkuk392joarJE76veDfgD1Yf6AuNKkAQVCgnB10b+EM5j",
-	"GprF/jfJnCwa2p5mYuRqQGDCBq+QwSBDXUsldpjdes0cf7da9eGVdr5TCJnB9EPG/qM4rIaxaMnQLAgu",
-	"aEIVfdCG+MhjFiEEGxJLzImFhZuCGng1qTZMJETZInj/DrxWTWTeKEQtTw8gnoyYo0QLIgR5GgtLGrBU",
-	"YSJH4ZdPLFpHPK0whvSeL4xSFlYUzChdWCOgca3MhW4jDklwHOJc5d5kElqDSsNOBiEDp/jTXRybQt4i",
-	"iVAMFfKf1uLUQt4WbvKY/n3zpbZk1pIegH7zW56Fz1Lk7UCutHV3EM9W8j1RvXDht6OyVdAt1hx9oC+C",
-	"V9cO2kRyRwWhvuYQkzuMc71NTvj7pekCvwwOQn+5y9rNo2vHx8ww58lJD6R6MhOiYQjnnIzqmhWz41TR",
-	"+kbIc6g2JmFn1+cT68qqw/o01w0IVK/jHyivSv7NzJog3MHUOkW5l86thChBH53UotFw4X1sLTqm8Gg0",
-	"e05ZdvMJVubUJMWO71UHJJuWTLOJ02pVNBohzhka1WvOqHafmqbaCV3q0rOKEylvtoLt7rdjLpW+VOaD",
-	"V0oTriRf5MLo+w7F06+IvLov7KNcszpw6IwQ+fApwsBWPCPr+ugMcQbwKlGiKua+YdqE8jsTyRD3v0uj",
-	"A9RHnTcd9me7U6p466Uw8bzpRPVsQY07d7qazX/f5CCeA7CkeuhqxGU7z/XqANsjAF/yKO1E375YO6lJ",
-	"2o9DzXljxNFx3Vp2uQduS3E+1RqfaybIdjlH7tkUcifZw8PHumPdBR+651du/MfAddfCizh2z6ZSeak+",
-	"Xp/6JwBHmaOUGJE8c8mgXZsvzTb8nYghgK1SPPD9/DOzQqmWelJMCF8SCtlt9n8AAAD//xeCHE2EIAAA",
+	"H4sIAAAAAAAC/9xZS2/jOAz+KwF3TwtPnJm5+VbMvgrsPHbTwwKDHlSbaTRrWxpJKVoE/u8LSX7Kj9hJ",
+	"3KZzSyySH/mJJChpDyFLOEsxVRKCPQiUnKUSzZ81TXiM/+Sf9JeQpQpTpX8qfFQ+jwlN9T8ZbjEh5vsT",
+	"RwhAKkHTe8iyzIMIZSgoV5SlEMDVQhq7iwJrwe6+YahAi1o7Bv0D01KPn+1isAcuGEehqHXuOqqh0VTh",
+	"PQrIPLiWV1FincoX7xiLkaR6sTL2s8ANBPCTX8Xv5+D+58ofgd93VGAEwddC2dPQFc5tw2zTxw0VUn0i",
+	"CXYQ44FgcdeCg2qkvJqpW8MpTTdMK8c0xHxzUgMEH69vtHVFlTYPNyjVYo3iAQV48IBC2m14u1wtV1qQ",
+	"cUwJpxDA++Vq+RY84ERtjf9+vt82Pn/PiSBJplfu0YSrgyV6X/VuwB+oPtQVjClBElQoJARfG/lDOI9p",
+	"aJT9b5I5WTS0Pc3EyNmAwLgNXkGDQYY6l0rsMLv1mjn+brXqwyvlfKcQMoPph4z9R3GYDSPRoqFZEFzQ",
+	"hCr6oAXxkccsQgg2JJaYBxYWZorQwKtRtWEiIcoWwft34LVqIvNGIWp6egDxZMQcJVoQIcjTWFjSgKUK",
+	"EzkKv/xi0Tr8abkxxPd8bpS0sKJgRvHCGg6Na2UudBtxiILjEOcq92YkoRWoOOyMIGTgFH+6i2NTyFsk",
+	"EYqhQv7TSpxayNvCTO7Tv2++1FRmLekB6De/5Vn4LEXeduRKS3c78Wwl3+PVCxd+2ytbBd1kzdEH+jx4",
+	"de2gHUhuqAiorznE5A7jnG+TE/5+abrAL4OD0F+uWrt5dO34mBnmPDnpgVRPZkI0EcI5J6M6Z8XsOJW0",
+	"vhHyHKyNSdjZ+fnEurLqMD9NvQGC6nX8A+VVGX8zsyYQdzC1TmHupXMrIUrQRye1aDRceB9bSscUHo1m",
+	"zykb3XyElTk1ibHje9UByqYl02zktFoVjUaQc4ZG9Zozqt2nprF2Qpe69KziRMqbrWC7++2YS6Uvlfjg",
+	"ldKEK8kXuTD6vkPx9Csir+4L+0KuSR04dEaIfPgUYWCrOCNr+ugMcQbwKlGiyue+Ydq48jsTyVDsf5dC",
+	"B0Ifdd50oj/bnVIVt1aFiedNx6tnc2rcudPlbP77JgfxHIBlqIeuRtxo57leHYj2fICLvMf14MgGUOvd",
+	"4WVP6I6z7fu6k3qvfXNqjjEjTqTrltrlnuNtiPOx1ngFmkDb5ZzkZ2PIHZAPzzTrDr0LPsvPz9z4N8Z1",
+	"l+JFnOZnY6m8qx/PT/1lwWHmKCZGJM9cNGjT5gHbur8TMQSwVYoHvp+/XiuUaqkH0ITwJaGQ3Wb/BwAA",
+	"//8HeZ4w2yAAAA==",
 }
 
 // GetSwagger returns the Swagger specification corresponding to the generated code
diff --git a/internal/test/parameters/parameters.yaml b/internal/test/parameters/parameters.yaml
index 7472fd3..c71778b 100644
--- a/internal/test/parameters/parameters.yaml
+++ b/internal/test/parameters/parameters.yaml
@@ -303,6 +303,12 @@ paths:
           schema:
             type: integer
             format: int32
+        - name: ps
+          description: primitive string
+          in: query
+          required: false
+          schema:
+            type: string
         - name: co
           description: complex object
           in: query
diff --git a/internal/test/parameters/parameters_test.go b/internal/test/parameters/parameters_test.go
index 734e09f..53920e2 100644
--- a/internal/test/parameters/parameters_test.go
+++ b/internal/test/parameters/parameters_test.go
@@ -16,14 +16,15 @@ import (
 )
 
 type testServer struct {
-	array         []int32
-	object        *Object
-	complexObject *ComplexObject
-	passThrough   *string
-	primitive     *int32
-	cookieParams  *GetCookieParams
-	queryParams   *GetQueryFormParams
-	headerParams  *GetHeaderParams
+	array           []int32
+	object          *Object
+	complexObject   *ComplexObject
+	passThrough     *string
+	primitive       *int32
+	primitiveString *string
+	cookieParams    *GetCookieParams
+	queryParams     *GetQueryFormParams
+	headerParams    *GetHeaderParams
 }
 
 func (t *testServer) reset() {
@@ -32,6 +33,7 @@ func (t *testServer) reset() {
 	t.complexObject = nil
 	t.passThrough = nil
 	t.primitive = nil
+	t.primitiveString = nil
 	t.cookieParams = nil
 	t.queryParams = nil
 	t.headerParams = nil
@@ -151,6 +153,9 @@ func (t *testServer) GetQueryForm(ctx echo.Context, params GetQueryFormParams) e
 	if params.P != nil {
 		t.primitive = params.P
 	}
+	if params.Ps != nil {
+		t.primitiveString = params.Ps
+	}
 	if params.Ep != nil {
 		t.primitive = params.Ep
 	}
@@ -235,6 +240,8 @@ func TestParameterBinding(t *testing.T) {
 
 	var expectedPrimitive int32 = 5
 
+	var expectedPrimitiveString string = "123;456"
+
 	// Check the passthrough case
 	//  (GET /passThrough/{param})
 	result := testutil.NewRequest().Get("/passThrough/some%20string").Go(t, e)
@@ -372,6 +379,12 @@ func TestParameterBinding(t *testing.T) {
 	assert.EqualValues(t, &expectedPrimitive, ts.primitive)
 	ts.reset()
 
+	// primitive string within reserved char, i.e., ';' escaped to '%3B'
+	result = testutil.NewRequest().Get("/queryForm?ps=123%3B456").Go(t, e)
+	assert.Equal(t, http.StatusOK, result.Code())
+	assert.EqualValues(t, &expectedPrimitiveString, ts.primitiveString)
+	ts.reset()
+
 	// complex object
 	q = fmt.Sprintf("/queryForm?co=%s", string(marshaledComplexObject))
 	result = testutil.NewRequest().Get(q).Go(t, e)
@@ -608,6 +621,7 @@ func TestClientQueryParams(t *testing.T) {
 
 	var expectedPrimitive1 int32 = 5
 	var expectedPrimitive2 int32 = 100
+	var expectedPrimitiveString string = "123;456"
 
 	// Check query params
 	qParams := GetQueryFormParams{
@@ -617,6 +631,7 @@ func TestClientQueryParams(t *testing.T) {
 		O:  &expectedObject2,
 		Ep: &expectedPrimitive1,
 		P:  &expectedPrimitive2,
+		Ps: &expectedPrimitiveString,
 		Co: &expectedComplexObject,
 	}
 
diff --git a/pkg/runtime/styleparam_test.go b/pkg/runtime/styleparam_test.go
index de3791a..0120cc9 100644
--- a/pkg/runtime/styleparam_test.go
+++ b/pkg/runtime/styleparam_test.go
@@ -24,6 +24,8 @@ import (
 
 func TestStyleParam(t *testing.T) {
 	primitive := 5
+	primitiveString := "123"
+	primitiveStringWithReservedChar := "123;456"
 	array := []int{3, 4, 5}
 	type TestObject struct {
 		FirstName string `json:"firstName"`
@@ -251,6 +253,22 @@ func TestStyleParam(t *testing.T) {
 	assert.NoError(t, err)
 	assert.EqualValues(t, "id=5", result)
 
+	result, err = StyleParam("form", false, "id", primitiveString)
+	assert.NoError(t, err)
+	assert.EqualValues(t, "id=123", result)
+
+	result, err = StyleParam("form", true, "id", primitiveString)
+	assert.NoError(t, err)
+	assert.EqualValues(t, "id=123", result)
+
+	result, err = StyleParam("form", false, "id", primitiveStringWithReservedChar)
+	assert.NoError(t, err)
+	assert.EqualValues(t, "id=123%3B456", result)
+
+	result, err = StyleParam("form", true, "id", primitiveStringWithReservedChar)
+	assert.NoError(t, err)
+	assert.EqualValues(t, "id=123%3B456", result)
+
 	result, err = StyleParam("form", false, "id", array)
 	assert.NoError(t, err)
 	assert.EqualValues(t, "id=3,4,5", result)