From 1e0e09f073405b010c470c4549425d0c08e3ce09 Mon Sep 17 00:00:00 2001 From: PetarTodorovv <31803034+PetarTodorovv@users.noreply.github.com> Date: Wed, 17 Mar 2021 16:40:38 +0200 Subject: [PATCH] [HOTFIX] Fix connectivity-adapter json marshalling issue (#1771) * [HOTFIX] Fix connectivity-adapter json marshalling issue * Update connectivity-adapter version * Use escape HTML encoder * Use custom json marshalling and unmarshalling --- chart/compass/values.yaml | 2 +- .../internal/appregistry/service/converter.go | 51 +++++--------- .../appregistry/service/converter_test.go | 64 ++++++++++++++--- .../appregistry/service/fixtures_test.go | 14 ++-- .../servicedetailsvalidation_test.go | 8 ++- .../internal/connectorservice/connector.go | 3 +- .../connectivity-adapter/pkg/model/model.go | 68 ++++++++++++++++++- .../connectivity-adapter/pkg/res/success.go | 4 +- 8 files changed, 155 insertions(+), 59 deletions(-) diff --git a/chart/compass/values.yaml b/chart/compass/values.yaml index 2380550f93..3ab0860b00 100644 --- a/chart/compass/values.yaml +++ b/chart/compass/values.yaml @@ -16,7 +16,7 @@ global: version: "PR-1762" connectivity_adapter: dir: - version: "PR-1750" + version: "PR-1771" pairing_adapter: dir: version: "PR-1750" diff --git a/components/connectivity-adapter/internal/appregistry/service/converter.go b/components/connectivity-adapter/internal/appregistry/service/converter.go index 9f54688401..f676553d6f 100644 --- a/components/connectivity-adapter/internal/appregistry/service/converter.go +++ b/components/connectivity-adapter/internal/appregistry/service/converter.go @@ -3,7 +3,6 @@ package service import ( "encoding/json" "fmt" - "strconv" "strings" "github.com/pkg/errors" @@ -132,15 +131,14 @@ func (c *converter) DetailsToGraphQLCreateInput(deprecated model.ServiceDetails) if apiDef.Spec == nil { apiDef.Spec = &graphql.APISpecInput{} } - asClob := graphql.CLOB(string(deprecated.Api.Spec)) - apiDef.Spec.Data = &asClob + apiDef.Spec.Data = ptrClob(graphql.CLOB(*deprecated.Api.Spec)) if apiDef.Spec.Type == "" { apiDef.Spec.Type = graphql.APISpecTypeOpenAPI } - if c.isXML(string(deprecated.Api.Spec)) { + if model.IsXML(string(*deprecated.Api.Spec)) { apiDef.Spec.Format = graphql.SpecFormatXML - } else if c.isJSON(deprecated.Api.Spec) { + } else if c.isJSON(string(*deprecated.Api.Spec)) { apiDef.Spec.Format = graphql.SpecFormatJSON } else { apiDef.Spec.Format = graphql.SpecFormatYaml @@ -229,9 +227,9 @@ func (c *converter) DetailsToGraphQLCreateInput(deprecated model.ServiceDetails) // TODO add tests var format graphql.SpecFormat - if c.isXML(string(deprecated.Events.Spec)) { + if model.IsXML(string(*deprecated.Events.Spec)) { format = graphql.SpecFormatXML - } else if c.isJSON(deprecated.Events.Spec) { + } else if c.isJSON(string(*deprecated.Events.Spec)) { format = graphql.SpecFormatJSON } else { format = graphql.SpecFormatYaml @@ -241,7 +239,7 @@ func (c *converter) DetailsToGraphQLCreateInput(deprecated model.ServiceDetails) &graphql.EventDefinitionInput{ Name: deprecated.Name, Spec: &graphql.EventSpecInput{ - Data: ptrClob(graphql.CLOB(deprecated.Events.Spec)), + Data: ptrClob(graphql.CLOB(*deprecated.Events.Spec)), Type: graphql.EventSpecTypeAsyncAPI, Format: format, }, @@ -350,7 +348,7 @@ func (c *converter) GraphQLToServiceDetails(in graphql.BundleExt, legacyServiceR if apiDef.Spec != nil { outDeprecated.Api.ApiType = string(apiDef.Spec.Type) if apiDef.Spec.Data != nil { - outDeprecated.Api.Spec = json.RawMessage(*apiDef.Spec.Data) + outDeprecated.Api.Spec = ptrSpecResponse(model.SpecResponse(*apiDef.Spec.Data)) } } @@ -477,7 +475,7 @@ func (c *converter) GraphQLToServiceDetails(in graphql.BundleExt, legacyServiceR if eventDef.Spec != nil && eventDef.Spec.Data != nil { outDeprecated.Events = &model.Events{ - Spec: []byte(string(*eventDef.Spec.Data)), + Spec: ptrSpecResponse(model.SpecResponse(*eventDef.Spec.Data)), } } //TODO: convert also fetchRequest @@ -510,33 +508,16 @@ func (c *converter) ServiceDetailsToService(in model.ServiceDetails, serviceID s }, nil } -func ptrClob(in graphql.CLOB) *graphql.CLOB { - return &in +func (c *converter) isJSON(content string) bool { + out := map[string]interface{}{} + err := json.Unmarshal([]byte(content), &out) + return err == nil } -func (c *converter) isXML(content string) bool { - const snippetLength = 512 - - if unquoted, err := strconv.Unquote(content); err == nil { - content = unquoted - } - - var snippet string - length := len(content) - if length < snippetLength { - snippet = content - } else { - snippet = content[:snippetLength] - } - - openingIndex := strings.Index(snippet, "<") - closingIndex := strings.Index(snippet, ">") - - return openingIndex == 0 && openingIndex < closingIndex +func ptrClob(in graphql.CLOB) *graphql.CLOB { + return &in } -func (c *converter) isJSON(content []byte) bool { - out := map[string]interface{}{} - err := json.Unmarshal(content, &out) - return err == nil +func ptrSpecResponse(in model.SpecResponse) *model.SpecResponse { + return &in } diff --git a/components/connectivity-adapter/internal/appregistry/service/converter_test.go b/components/connectivity-adapter/internal/appregistry/service/converter_test.go index 0d98a41c11..6a020e2d5e 100644 --- a/components/connectivity-adapter/internal/appregistry/service/converter_test.go +++ b/components/connectivity-adapter/internal/appregistry/service/converter_test.go @@ -114,7 +114,7 @@ func TestConverter_DetailsToGraphQLCreateInput(t *testing.T) { "API with directly spec provided in YAML": { given: model.ServiceDetails{ Api: &model.API{ - Spec: json.RawMessage(`openapi: "3.0.0"`), + Spec: ptrSpecResponse(`openapi: "3.0.0"`), }, }, expected: graphql.BundleCreateInput{ @@ -122,7 +122,7 @@ func TestConverter_DetailsToGraphQLCreateInput(t *testing.T) { APIDefinitions: []*graphql.APIDefinitionInput{ { Spec: &graphql.APISpecInput{ - Data: ptrClob(graphql.CLOB(`openapi: "3.0.0"`)), + Data: ptrClob(`openapi: "3.0.0"`), Type: graphql.APISpecTypeOpenAPI, Format: graphql.SpecFormatYaml, }, @@ -134,7 +134,7 @@ func TestConverter_DetailsToGraphQLCreateInput(t *testing.T) { "API with directly spec provided in JSON": { given: model.ServiceDetails{ Api: &model.API{ - Spec: json.RawMessage(`{"spec":"v0.0.1"}`), + Spec: ptrSpecResponse(`{"spec":"v0.0.1"}`), }, }, expected: graphql.BundleCreateInput{ @@ -142,7 +142,7 @@ func TestConverter_DetailsToGraphQLCreateInput(t *testing.T) { APIDefinitions: []*graphql.APIDefinitionInput{ { Spec: &graphql.APISpecInput{ - Data: ptrClob(graphql.CLOB(`{"spec":"v0.0.1"}`)), + Data: ptrClob(`{"spec":"v0.0.1"}`), Type: graphql.APISpecTypeOpenAPI, Format: graphql.SpecFormatJSON, }, @@ -154,7 +154,7 @@ func TestConverter_DetailsToGraphQLCreateInput(t *testing.T) { "API with directly spec provided in XML": { given: model.ServiceDetails{ Api: &model.API{ - Spec: json.RawMessage(`"`), + Spec: ptrSpecResponse(`"`), }, }, expected: graphql.BundleCreateInput{ @@ -162,7 +162,7 @@ func TestConverter_DetailsToGraphQLCreateInput(t *testing.T) { APIDefinitions: []*graphql.APIDefinitionInput{ { Spec: &graphql.APISpecInput{ - Data: ptrClob(graphql.CLOB(`"`)), + Data: ptrClob(`"`), Type: graphql.APISpecTypeOpenAPI, Format: graphql.SpecFormatXML, }, @@ -458,7 +458,7 @@ func TestConverter_DetailsToGraphQLCreateInput(t *testing.T) { given: model.ServiceDetails{ Name: "foo", Events: &model.Events{ - Spec: json.RawMessage(`asyncapi: "1.2.0"`), + Spec: ptrSpecResponse(`asyncapi: "1.2.0"`), }, }, expected: graphql.BundleCreateInput{ @@ -762,7 +762,7 @@ func TestConverter_GraphQLToServiceDetails(t *testing.T) { Data: []*graphql.EventAPIDefinitionExt{{ Spec: &graphql.EventAPISpecExt{ EventSpec: graphql.EventSpec{ - Data: ptrClob(`asyncapi: "1.2.0"`), + Data: ptrClob(`{"asyncapi": "1.2.0"}`), }, }}, }, @@ -770,7 +770,45 @@ func TestConverter_GraphQLToServiceDetails(t *testing.T) { }, expected: model.ServiceDetails{ Events: &model.Events{ - Spec: json.RawMessage(`asyncapi: "1.2.0"`), + Spec: ptrSpecResponse(`{"asyncapi": "1.2.0"}`), + }, + Labels: emptyLabels(), + }, + }, + "events with XML spec": { + given: graphql.BundleExt{ + EventDefinitions: graphql.EventAPIDefinitionPageExt{ + Data: []*graphql.EventAPIDefinitionExt{{ + Spec: &graphql.EventAPISpecExt{ + EventSpec: graphql.EventSpec{ + Data: ptrClob(``), + }, + }}, + }, + }, + }, + expected: model.ServiceDetails{ + Events: &model.Events{ + Spec: ptrSpecResponse(``), + }, + Labels: emptyLabels(), + }, + }, + "API with XML spec": { + given: graphql.BundleExt{ + APIDefinitions: graphql.APIDefinitionPageExt{ + Data: []*graphql.APIDefinitionExt{{ + Spec: &graphql.APISpecExt{ + APISpec: graphql.APISpec{ + Data: ptrClob(``), + }, + }}, + }, + }, + }, + expected: model.ServiceDetails{ + Api: &model.API{ + Spec: ptrSpecResponse(``), }, Labels: emptyLabels(), }, @@ -782,6 +820,8 @@ func TestConverter_GraphQLToServiceDetails(t *testing.T) { // THEN require.NoError(t, err) assert.Equal(t, tc.expected, actual) + _, err = json.Marshal(actual) + require.NoError(t, err) }) } @@ -823,7 +863,7 @@ func TestConverter_ServiceDetailsToService(t *testing.T) { Name: "name", Description: "description", ShortDescription: "short description", - Identifier: "identifie", + Identifier: "identifier", Labels: &map[string]string{"blalb": "blalba"}, } id := "id" @@ -964,3 +1004,7 @@ func ptrString(in string) *string { func ptrClob(in graphql.CLOB) *graphql.CLOB { return &in } + +func ptrSpecResponse(in model.SpecResponse) *model.SpecResponse { + return &in +} diff --git a/components/connectivity-adapter/internal/appregistry/service/fixtures_test.go b/components/connectivity-adapter/internal/appregistry/service/fixtures_test.go index 1d9df37b43..d64df43724 100644 --- a/components/connectivity-adapter/internal/appregistry/service/fixtures_test.go +++ b/components/connectivity-adapter/internal/appregistry/service/fixtures_test.go @@ -1,6 +1,8 @@ package service_test -import "github.com/kyma-incubator/compass/components/connectivity-adapter/pkg/model" +import ( + "github.com/kyma-incubator/compass/components/connectivity-adapter/pkg/model" +) func fixAPIOpenAPIYAML() model.API { spec := `openapi: 3.0.0 @@ -29,7 +31,7 @@ paths: type: string` return model.API{ - Spec: []byte(spec), + Spec: ptrSpecResponse(model.SpecResponse(spec)), } } @@ -104,7 +106,7 @@ func fixAPIOpenAPIJSON() model.API { }` return model.API{ - Spec: []byte(spec), + Spec: ptrSpecResponse(model.SpecResponse(spec)), } } @@ -271,7 +273,7 @@ func fixAPIODataXML() model.API { ` return model.API{ - Spec: []byte(spec), + Spec: ptrSpecResponse(model.SpecResponse(spec)), ApiType: "odata", } } @@ -310,7 +312,7 @@ components: type: string` return model.Events{ - Spec: []byte(spec), + Spec: ptrSpecResponse(model.SpecResponse(spec)), } } @@ -340,7 +342,7 @@ func fixEventsAsyncAPIJSON() model.Events { }` return model.Events{ - Spec: []byte(spec), + Spec: ptrSpecResponse(model.SpecResponse(spec)), } } diff --git a/components/connectivity-adapter/internal/appregistry/service/validation/servicedetailsvalidation_test.go b/components/connectivity-adapter/internal/appregistry/service/validation/servicedetailsvalidation_test.go index a1bef3b450..c3acbc12cb 100644 --- a/components/connectivity-adapter/internal/appregistry/service/validation/servicedetailsvalidation_test.go +++ b/components/connectivity-adapter/internal/appregistry/service/validation/servicedetailsvalidation_test.go @@ -55,7 +55,7 @@ func TestServiceDetailsValidator(t *testing.T) { Provider: "provider", Description: "description", Events: &model.Events{ - Spec: eventsRawSpec, + Spec: ptrSpecResponse(model.SpecResponse(eventsRawSpec)), }, } @@ -78,7 +78,7 @@ func TestServiceDetailsValidator(t *testing.T) { TargetUrl: "http://target.com", }, Events: &model.Events{ - Spec: eventsRawSpec, + Spec: ptrSpecResponse(model.SpecResponse(eventsRawSpec)), }, } @@ -633,3 +633,7 @@ func TestServiceDetailsValidator_Specification_Basic(t *testing.T) { assert.Equal(t, apperrors.CodeWrongInput, err.Code()) }) } + +func ptrSpecResponse(in model.SpecResponse) *model.SpecResponse { + return &in +} diff --git a/components/connectivity-adapter/internal/connectorservice/connector.go b/components/connectivity-adapter/internal/connectorservice/connector.go index fefcbf6bc2..65d0f0dd69 100644 --- a/components/connectivity-adapter/internal/connectorservice/connector.go +++ b/components/connectivity-adapter/internal/connectorservice/connector.go @@ -4,9 +4,10 @@ import ( "net/http" "time" + "github.com/kyma-incubator/compass/components/connectivity-adapter/internal/connectorservice/api/middlewares" + "github.com/gorilla/mux" "github.com/kyma-incubator/compass/components/connectivity-adapter/internal/connectorservice/api" - "github.com/kyma-incubator/compass/components/connectivity-adapter/internal/connectorservice/api/middlewares" "github.com/kyma-incubator/compass/components/connectivity-adapter/internal/connectorservice/connector" "github.com/kyma-incubator/compass/components/connectivity-adapter/internal/connectorservice/director" "github.com/kyma-incubator/compass/components/connectivity-adapter/pkg/model" diff --git a/components/connectivity-adapter/pkg/model/model.go b/components/connectivity-adapter/pkg/model/model.go index 7b2e8953b3..d94c7fa2e8 100644 --- a/components/connectivity-adapter/pkg/model/model.go +++ b/components/connectivity-adapter/pkg/model/model.go @@ -1,6 +1,10 @@ package model -import "encoding/json" +import ( + "encoding/json" + "strconv" + "strings" +) const ( TokenFormat = "?token=%s" @@ -12,6 +16,43 @@ const ( RevocationCertURLFormat = "%s/v1/applications/certificates/revocations" ) +type SpecResponse string + +func (sp *SpecResponse) UnmarshalJSON(bytes []byte) error { + if sp == nil { + sp = new(SpecResponse) + } + + if IsXML(string(bytes)) { + *sp = SpecResponse(bytes) + return nil + } + + jsonRawMessage := json.RawMessage{} + if err := json.Unmarshal(bytes, &jsonRawMessage); err != nil { + return err + } + + *sp = SpecResponse(jsonRawMessage) + + return nil +} + +func (sp *SpecResponse) MarshalJSON() ([]byte, error) { + if sp == nil { + return nil, nil + } + if IsXML(string(*sp)) { + unquote, err := strconv.Unquote(string(*sp)) + if err == nil { + *sp = SpecResponse(unquote) + } + + return []byte(strconv.Quote(string(*sp))), nil + } + return json.Marshal(json.RawMessage(*sp)) +} + type Service struct { ID string `json:"id"` Provider string `json:"provider"` @@ -40,7 +81,7 @@ type CreateServiceResponse struct { type API struct { TargetUrl string `json:"targetUrl" valid:"url,required~targetUrl field cannot be empty."` Credentials *CredentialsWithCSRF `json:"credentials,omitempty"` - Spec json.RawMessage `json:"spec,omitempty"` + Spec *SpecResponse `json:"spec,omitempty"` SpecificationUrl string `json:"specificationUrl,omitempty"` ApiType string `json:"apiType,omitempty"` RequestParameters *RequestParameters `json:"requestParameters,omitempty"` @@ -103,7 +144,7 @@ type CertificateGenWithCSRF struct { } type Events struct { - Spec json.RawMessage `json:"spec" valid:"required~spec cannot be empty"` + Spec *SpecResponse `json:"spec" valid:"required~spec cannot be empty"` } type Documentation struct { @@ -176,3 +217,24 @@ type TokenResponse struct { URL string `json:"url"` Token string `json:"token"` } + +func IsXML(content string) bool { + const snippetLength = 512 + + if unquoted, err := strconv.Unquote(content); err == nil { + content = unquoted + } + + var snippet string + length := len(content) + if length < snippetLength { + snippet = content + } else { + snippet = content[:snippetLength] + } + + openingIndex := strings.Index(snippet, "<") + closingIndex := strings.Index(snippet, ">") + + return openingIndex == 0 && openingIndex < closingIndex +} diff --git a/components/connectivity-adapter/pkg/res/success.go b/components/connectivity-adapter/pkg/res/success.go index fd948e71a1..212007f18b 100644 --- a/components/connectivity-adapter/pkg/res/success.go +++ b/components/connectivity-adapter/pkg/res/success.go @@ -10,5 +10,7 @@ import ( func WriteJSONResponse(writer http.ResponseWriter, res interface{}) error { log.Infoln("returning response...") writer.Header().Set(HeaderContentTypeKey, HeaderContentTypeValue) - return json.NewEncoder(writer).Encode(&res) + enc := json.NewEncoder(writer) + enc.SetEscapeHTML(false) + return enc.Encode(&res) }