fix: incorrect urlencode encoding for JSON scalar, argument preset in…

…it (#44) * Fix incorrectly encoding Arbitrary JSON scalar for application/x-www-form-urlencoded content type. * Fix argument preset initialization in server settings. * Disable HTML escape when encoding JSON. * Reroute HTTP 4xx response from the remote request to HTTP 422 so the detail can appear in the graphql error. * Use JSON for the result type if the response is unknown.
hasura · Dec 15, 2024 · 367d03a · 367d03a
1 parent 2ec5ce8
commit 367d03a
Show file tree

Hide file tree

Showing 35 changed files with 1,084 additions and 268 deletions.
diff --git a/connector/connector_test.go b/connector/connector_test.go
@@ -202,7 +202,7 @@ func TestHTTPConnector_authentication(t *testing.T) {
 			Details: schema.ExplainResponseDetails{
 				"url":     server.URL + "/pet",
 				"headers": `{"Accept":["application/json"],"Api_key":["ran*******(14)"],"Content-Type":["application/json"]}`,
-				"body":    `{"name":"pet"}`,
+				"body":    "{\"name\":\"pet\"}\n",
 			},
 		})
 	})
@@ -380,7 +380,7 @@ func TestHTTPConnector_authentication(t *testing.T) {
 
 		res, err := http.Post(fmt.Sprintf("%s/query", testServer.URL), "application/json", bytes.NewBuffer(reqBody))
 		assert.NilError(t, err)
-		assert.Equal(t, http.StatusTooManyRequests, res.StatusCode)
+		assert.Equal(t, http.StatusUnprocessableEntity, res.StatusCode)
 	})
 
 	t.Run("encoding-ndjson", func(t *testing.T) {

diff --git a/connector/internal/argument/preset.go b/connector/internal/argument/preset.go
@@ -18,8 +18,8 @@ type ArgumentPreset struct {
 }
 
 // NewArgumentPreset create a new ArgumentPreset instance.
-func NewArgumentPreset(httpSchema *rest.NDCHttpSchema, preset rest.ArgumentPresetConfig) (*ArgumentPreset, error) {
-	jsonPath, targets, err := configuration.ValidateArgumentPreset(httpSchema, preset)
+func NewArgumentPreset(httpSchema *rest.NDCHttpSchema, preset rest.ArgumentPresetConfig, isGlobal bool) (*ArgumentPreset, error) {
+	jsonPath, targets, err := configuration.ValidateArgumentPreset(httpSchema, preset, isGlobal)
 	if err != nil {
 		return nil, err
 	}

diff --git a/connector/internal/argument/presets.go b/connector/internal/argument/presets.go
@@ -13,13 +13,13 @@ type ArgumentPresets struct {
 }
 
 // NewArgumentPresets create a new ArgumentPresets instance.
-func NewArgumentPresets(httpSchema *rest.NDCHttpSchema, presets []rest.ArgumentPresetConfig) (*ArgumentPresets, error) {
+func NewArgumentPresets(httpSchema *rest.NDCHttpSchema, presets []rest.ArgumentPresetConfig, isGlobal bool) (*ArgumentPresets, error) {
 	result := &ArgumentPresets{
 		httpSchema: httpSchema,
 		presets:    nil,
 	}
 	for i, item := range presets {
-		preset, err := NewArgumentPreset(httpSchema, item)
+		preset, err := NewArgumentPreset(httpSchema, item, isGlobal)
 		if err != nil {
 			return nil, fmt.Errorf("%d: %w", i, err)
 		}

diff --git a/connector/internal/client.go b/connector/internal/client.go
@@ -248,7 +248,12 @@ func (client *HTTPClient) sendSingle(ctx context.Context, request *RetryableRequ
 
 		span.SetStatus(codes.Error, "received error from remote server")
 
-		return nil, nil, schema.NewConnectorError(resp.StatusCode, resp.Status, details)
+		statusCode := resp.StatusCode
+		if statusCode < 500 {
+			statusCode = http.StatusUnprocessableEntity
+		}
+
+		return nil, nil, schema.NewConnectorError(statusCode, resp.Status, details)
 	}
 
 	result, headers, evalErr := client.evalHTTPResponse(ctx, span, resp, contentType, selection, logger)

diff --git a/connector/internal/contenttype/multipart.go b/connector/internal/contenttype/multipart.go
@@ -23,7 +23,7 @@ type MultipartFormEncoder struct {
 func NewMultipartFormEncoder(schema *rest.NDCHttpSchema, operation *rest.OperationInfo, arguments map[string]any) *MultipartFormEncoder {
 	return &MultipartFormEncoder{
 		schema:       schema,
-		paramEncoder: NewURLParameterEncoder(schema),
+		paramEncoder: NewURLParameterEncoder(schema, rest.ContentTypeMultipartFormData),
 		operation:    operation,
 		arguments:    arguments,
 	}

diff --git a/connector/internal/contenttype/url_encode.go b/connector/internal/contenttype/url_encode.go
@@ -21,12 +21,16 @@ import (
 
 // URLParameterEncoder represents a URL parameter encoder.
 type URLParameterEncoder struct {
-	schema *rest.NDCHttpSchema
+	schema      *rest.NDCHttpSchema
+	contentType string
 }
 
 // NewURLParameterEncoder creates a URLParameterEncoder instance.
-func NewURLParameterEncoder(schema *rest.NDCHttpSchema) *URLParameterEncoder {
-	return &URLParameterEncoder{schema: schema}
+func NewURLParameterEncoder(schema *rest.NDCHttpSchema, contentType string) *URLParameterEncoder {
+	return &URLParameterEncoder{
+		schema:      schema,
+		contentType: contentType,
+	}
 }
 
 func (c *URLParameterEncoder) Encode(bodyInfo *rest.ArgumentInfo, bodyData any) (io.ReadSeeker, error) {
@@ -104,7 +108,7 @@ func (c *URLParameterEncoder) EncodeParameterValues(objectField *rest.ObjectFiel
 		}
 		iScalar, ok := c.schema.ScalarTypes[ty.Name]
 		if ok {
-			return encodeScalarParameterReflectionValues(reflectValue, &iScalar, fieldPaths)
+			return c.encodeScalarParameterReflectionValues(reflectValue, &iScalar, fieldPaths)
 		}
 		kind := reflectValue.Kind()
 		objectInfo, ok := c.schema.ObjectTypes[ty.Name]
@@ -119,6 +123,7 @@ func (c *URLParameterEncoder) EncodeParameterValues(objectField *rest.ObjectFiel
 			if !ok {
 				return nil, fmt.Errorf("%s: failed to evaluate object, got <%s> %v", strings.Join(fieldPaths, ""), kind, anyValue)
 			}
+
 			for key, fieldInfo := range objectInfo.Fields {
 				fieldVal := object[key]
 				output, err := c.EncodeParameterValues(&fieldInfo, reflect.ValueOf(fieldVal), append(fieldPaths, "."+key))
@@ -159,7 +164,7 @@ func (c *URLParameterEncoder) EncodeParameterValues(objectField *rest.ObjectFiel
 	return nil, fmt.Errorf("%s: invalid type %v", strings.Join(fieldPaths, ""), objectField.Type)
 }
 
-func encodeScalarParameterReflectionValues(reflectValue reflect.Value, scalar *schema.ScalarType, fieldPaths []string) (ParameterItems, error) {
+func (c *URLParameterEncoder) encodeScalarParameterReflectionValues(reflectValue reflect.Value, scalar *schema.ScalarType, fieldPaths []string) (ParameterItems, error) {
 	switch sl := scalar.Representation.Interface().(type) {
 	case *schema.TypeRepresentationBoolean:
 		value, err := utils.DecodeBooleanReflection(reflectValue)
@@ -237,41 +242,49 @@ func encodeScalarParameterReflectionValues(reflectValue reflect.Value, scalar *s
 
 		return []ParameterItem{NewParameterItem([]Key{}, []string{rawValue})}, nil
 	default:
-		return encodeParameterReflectionValues(reflectValue, fieldPaths)
+		return c.encodeParameterReflectionValues(reflectValue, fieldPaths)
 	}
 }
 
-func encodeParameterReflectionValues(reflectValue reflect.Value, fieldPaths []string) (ParameterItems, error) {
+func (c *URLParameterEncoder) encodeParameterReflectionValues(reflectValue reflect.Value, fieldPaths []string) (ParameterItems, error) {
 	reflectValue, ok := utils.UnwrapPointerFromReflectValue(reflectValue)
 	if !ok {
 		return ParameterItems{}, nil
 	}
 
 	kind := reflectValue.Kind()
-	if result, err := StringifySimpleScalar(reflectValue, kind); err == nil {
-		return []ParameterItem{
-			NewParameterItem([]Key{}, []string{result}),
-		}, nil
+	if c.contentType == rest.ContentTypeMultipartFormData {
+		if result, err := StringifySimpleScalar(reflectValue, kind); err == nil {
+			return []ParameterItem{
+				NewParameterItem([]Key{}, []string{result}),
+			}, nil
+		}
 	}
 
 	switch kind {
 	case reflect.Slice, reflect.Array:
-		return encodeParameterReflectionSlice(reflectValue, fieldPaths)
+		return c.encodeParameterReflectionSlice(reflectValue, fieldPaths)
 	case reflect.Map, reflect.Interface:
-		return encodeParameterReflectionMap(reflectValue, fieldPaths)
+		return c.encodeParameterReflectionMap(reflectValue, fieldPaths)
 	case reflect.Struct:
-		return encodeParameterReflectionStruct(reflectValue, fieldPaths)
+		return c.encodeParameterReflectionStruct(reflectValue, fieldPaths)
 	default:
+		if result, err := StringifySimpleScalar(reflectValue, kind); err == nil {
+			return []ParameterItem{
+				NewParameterItem([]Key{}, []string{result}),
+			}, nil
+		}
+
 		return nil, fmt.Errorf("%s: failed to encode parameter, got %s", strings.Join(fieldPaths, ""), kind)
 	}
 }
 
-func encodeParameterReflectionSlice(reflectValue reflect.Value, fieldPaths []string) (ParameterItems, error) {
+func (c *URLParameterEncoder) encodeParameterReflectionSlice(reflectValue reflect.Value, fieldPaths []string) (ParameterItems, error) {
 	results := ParameterItems{}
 	valueLen := reflectValue.Len()
 	for i := range valueLen {
 		elem := reflectValue.Index(i)
-		outputs, err := encodeParameterReflectionValues(elem, append(fieldPaths, fmt.Sprintf("[%d]", i)))
+		outputs, err := c.encodeParameterReflectionValues(elem, append(fieldPaths, fmt.Sprintf("[%d]", i)))
 		if err != nil {
 			return nil, err
 		}
@@ -284,13 +297,13 @@ func encodeParameterReflectionSlice(reflectValue reflect.Value, fieldPaths []str
 	return results, nil
 }
 
-func encodeParameterReflectionStruct(reflectValue reflect.Value, fieldPaths []string) (ParameterItems, error) {
+func (c *URLParameterEncoder) encodeParameterReflectionStruct(reflectValue reflect.Value, fieldPaths []string) (ParameterItems, error) {
 	results := ParameterItems{}
 	reflectType := reflectValue.Type()
 	for fieldIndex := range reflectValue.NumField() {
 		fieldVal := reflectValue.Field(fieldIndex)
 		fieldType := reflectType.Field(fieldIndex)
-		output, err := encodeParameterReflectionValues(fieldVal, append(fieldPaths, "."+fieldType.Name))
+		output, err := c.encodeParameterReflectionValues(fieldVal, append(fieldPaths, "."+fieldType.Name))
 		if err != nil {
 			return nil, err
 		}
@@ -303,7 +316,7 @@ func encodeParameterReflectionStruct(reflectValue reflect.Value, fieldPaths []st
 	return results, nil
 }
 
-func encodeParameterReflectionMap(reflectValue reflect.Value, fieldPaths []string) (ParameterItems, error) {
+func (c *URLParameterEncoder) encodeParameterReflectionMap(reflectValue reflect.Value, fieldPaths []string) (ParameterItems, error) {
 	results := ParameterItems{}
 	anyValue := reflectValue.Interface()
 	object, ok := anyValue.(map[string]any)
@@ -318,7 +331,7 @@ func encodeParameterReflectionMap(reflectValue reflect.Value, fieldPaths []strin
 	}
 
 	for key, fieldValue := range object {
-		output, err := encodeParameterReflectionValues(reflect.ValueOf(fieldValue), append(fieldPaths, "."+key))
+		output, err := c.encodeParameterReflectionValues(reflect.ValueOf(fieldValue), append(fieldPaths, "."+key))
 		if err != nil {
 			return nil, err
 		}

diff --git a/connector/internal/contenttype/url_encode_test.go b/connector/internal/contenttype/url_encode_test.go
@@ -691,7 +691,7 @@ func TestCreateFormURLEncoded(t *testing.T) {
 			var arguments map[string]any
 			assert.NilError(t, json.Unmarshal([]byte(tc.RawArguments), &arguments))
 			argumentInfo := info.Arguments["body"]
-			builder := NewURLParameterEncoder(ndcSchema)
+			builder := NewURLParameterEncoder(ndcSchema, rest.ContentTypeFormURLEncoded)
 			buf, err := builder.Encode(&argumentInfo, arguments["body"])
 			assert.NilError(t, err)
 			result, err := io.ReadAll(buf)

diff --git a/connector/internal/request_builder.go b/connector/internal/request_builder.go
@@ -129,22 +129,24 @@ func (c *RequestBuilder) buildRequestBody(request *RetryableRequest, rawRequest
 
 			return nil
 		case contentType == rest.ContentTypeFormURLEncoded:
-			r, err := contenttype.NewURLParameterEncoder(c.Schema).Encode(&bodyInfo, bodyData)
+			r, err := contenttype.NewURLParameterEncoder(c.Schema, rest.ContentTypeFormURLEncoded).Encode(&bodyInfo, bodyData)
 			if err != nil {
 				return err
 			}
 			request.Body = r
 
 			return nil
 		case contentType == rest.ContentTypeJSON || contentType == "":
+			var buf bytes.Buffer
+			enc := json.NewEncoder(&buf)
+			enc.SetEscapeHTML(false)
 
-			bodyBytes, err := json.Marshal(bodyData)
-			if err != nil {
+			if err := enc.Encode(bodyData); err != nil {
 				return err
 			}
 
-			request.ContentLength = int64(len(bodyBytes))
-			request.Body = bytes.NewReader(bodyBytes)
+			request.ContentLength = int64(buf.Len())
+			request.Body = bytes.NewReader(buf.Bytes())
 
 			return nil
 		case contentType == rest.ContentTypeXML:
@@ -237,7 +239,7 @@ func (c *RequestBuilder) evalURLAndHeaderParameterBySchema(endpoint *url.URL, he
 	if argumentInfo.HTTP.Name != "" {
 		argumentKey = argumentInfo.HTTP.Name
 	}
-	queryParams, err := contenttype.NewURLParameterEncoder(c.Schema).EncodeParameterValues(&rest.ObjectField{
+	queryParams, err := contenttype.NewURLParameterEncoder(c.Schema, rest.ContentTypeFormURLEncoded).EncodeParameterValues(&rest.ObjectField{
 		ObjectField: schema.ObjectField{
 			Type: argumentInfo.Type,
 		},

diff --git a/connector/internal/request_builder_test.go b/connector/internal/request_builder_test.go
@@ -2,6 +2,7 @@ package internal
 
 import (
 	"encoding/json"
+	"io"
 	"net/url"
 	"os"
 	"testing"
@@ -10,7 +11,7 @@ import (
 	"gotest.tools/v3/assert"
 )
 
-func TestEvalURLAndHeaderParameters(t *testing.T) {
+func TestQueryEvalURLAndHeaderParameters(t *testing.T) {
 	testCases := []struct {
 		name         string
 		rawArguments string
@@ -77,6 +78,80 @@ func TestEvalURLAndHeaderParameters(t *testing.T) {
 	}
 }
 
+func TestMutationEvalURLAndHeaderParameters(t *testing.T) {
+	testCases := []struct {
+		name         string
+		rawArguments string
+		expectedURL  string
+		expectedBody string
+		errorMsg     string
+		headers      map[string]string
+	}{
+		{
+			name: "PostBillingMeterEvents",
+			rawArguments: `{
+        "body": {
+          "event_name": "k8hAOi2B52",
+          "identifier": "identifier_123",
+          "payload": {
+            "value": "25",
+            "stripe_customer_id": "cus_NciAYcXfLnqBoz"
+          },
+          "timestamp": 931468280
+        }
+			}`,
+			expectedURL:  "/v1/billing/meter_events",
+			expectedBody: "event_name=k8hAOi2B52&identifier=identifier_123&payload[value]=25&payload[stripe_customer_id]=cus_NciAYcXfLnqBoz&timestamp=931468280",
+		},
+	}
+
+	ndcSchema := createMockSchema(t)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var info *rest.OperationInfo
+			for key, f := range ndcSchema.Procedures {
+				if key == tc.name {
+					info = &f
+					break
+				}
+			}
+			var arguments map[string]any
+			assert.NilError(t, json.Unmarshal([]byte(tc.rawArguments), &arguments))
+
+			builder := RequestBuilder{
+				Schema:    ndcSchema,
+				Operation: info,
+				Arguments: arguments,
+			}
+
+			result, err := builder.Build()
+			if tc.errorMsg != "" {
+				assert.ErrorContains(t, err, tc.errorMsg)
+
+				return
+			}
+
+			assert.NilError(t, err)
+			decodedValue, err := url.QueryUnescape(result.URL.String())
+			assert.NilError(t, err)
+			assert.Equal(t, tc.expectedURL, decodedValue)
+
+			for k, v := range tc.headers {
+				assert.Equal(t, v, result.Headers.Get(k))
+			}
+
+			bodyBytes, err := io.ReadAll(result.Body)
+			assert.NilError(t, err)
+			expected, err := url.ParseQuery(tc.expectedBody)
+			assert.NilError(t, err)
+			body, err := url.ParseQuery(string(bodyBytes))
+			assert.NilError(t, err)
+
+			assert.DeepEqual(t, expected, body)
+		})
+	}
+}
+
 func TestEvalURLAndHeaderParametersOAS2(t *testing.T) {
 	testCases := []struct {
 		name         string

diff --git a/connector/internal/security/auth.go b/connector/internal/security/auth.go
@@ -28,15 +28,15 @@ func NewCredential(ctx context.Context, httpClient *http.Client, security schema
 	case *schema.APIKeyAuthConfig:
 		cred, err := NewApiKeyCredential(httpClient, ss)
 
-		return cred, err == nil, err
+		return cred, err != nil, err
 	case *schema.BasicAuthConfig:
 		cred, err := NewBasicCredential(httpClient, ss)
 
-		return cred, err == nil, err
+		return cred, err != nil, err
 	case *schema.HTTPAuthConfig:
 		cred, err := NewHTTPCredential(httpClient, ss)
 
-		return cred, err == nil, err
+		return cred, err != nil, err
 	case *schema.OAuth2Config:
 		var headerForwardingRequired bool
 		for flowType, flow := range ss.Flows {

diff --git a/connector/internal/upstream.go b/connector/internal/upstream.go
@@ -58,7 +58,7 @@ func (um *UpstreamManager) Register(ctx context.Context, runtimeSchema *configur
 	}
 
 	if len(runtimeSchema.Settings.ArgumentPresets) > 0 {
-		argumentPresets, err := argument.NewArgumentPresets(ndcSchema, runtimeSchema.Settings.ArgumentPresets)
+		argumentPresets, err := argument.NewArgumentPresets(ndcSchema, runtimeSchema.Settings.ArgumentPresets, true)
 		if err != nil {
 			return fmt.Errorf("%s: %w", namespace, err)
 		}
@@ -101,7 +101,7 @@ func (um *UpstreamManager) Register(ctx context.Context, runtimeSchema *configur
 		}
 
 		if len(server.ArgumentPresets) > 0 {
-			argumentPresets, err := argument.NewArgumentPresets(ndcSchema, server.ArgumentPresets)
+			argumentPresets, err := argument.NewArgumentPresets(ndcSchema, server.ArgumentPresets, false)
 			if err != nil {
 				return fmt.Errorf("%s.server[%s]: %w", namespace, serverID, err)
 			}