search

optimizely / go-sdk

Go SDK for Optimizely Feature Experimentation and Optimizely Full Stack (legacy)
https://docs.developers.optimizely.com/experimentation/v4.0-full-stack/docs/go-sdk
Apache License 2.0
18 stars 19 forks source link

Valid non-object JSON features cause panics #351

Closed djha-skin closed 2 years ago

djha-skin commented 2 years ago

Cc @simone-coelho .

This bug is in relation to Optimizely support ticket #1001007469 .

The bug described in this ticket is in these locations: https://github.com/optimizely/go-sdk/blob/master/pkg/client/client.go#L935

https://github.com/optimizely/go-sdk/blob/master/pkg/client/client.go#L500

The code always assumes that valid JSON will be returned as a map. It does not check first whether the JSON is a map, array, or scalar value. However, the optimizely web interface allows any valid json value.

We put in a JSON array into a feature in optimizely. This was passed to the optimizely agent, which in turn passed it to this go-sdk. The sdk assumed that because the value was JSON it was necessarily a map and tried to convert it to a map in the linked line of code. It wasn't a map in our case, but an array. This causes a panic, which causes the optimizely agent to sever the TCP connection with the client without so much as a 500 error.

The code herein linked should be changed so that it checks to make sure the JSON is indeed a map before it attempts conversion and issue a polite error if it isn't as expected so that the optimizely agent can give back a polite http code error in turn. Or, change the code to allow for arrays, as is allowed by the optimizely UI.

We appreciate your time.

mikechu-optimizely commented 2 years ago

Hi @djhaskin987,

Thanks for opening this issue. I'll put this in for review.

djha-skin commented 2 years ago

Thanks @mikechu-optimizely . @simone-coelho said the behavior should be that it should also accept JSON arrays. He said he'd put in a Jira ticket in for it so I'd ask him about that :)

mikechu-optimizely commented 2 years ago

I've coordinated with and taken Simone's finding, opening an active Jira issue.

mikechu-optimizely commented 2 years ago

Hi @djhaskin987,

We are addressing the Go panic directly in the SDK during a future release.

The team responsible for the Web UI and API are ensuring that both of these avenues only accept JSON object types.

Do you have any blockers for this particular GH Issue now that you've configured to use a JSON object?

cc @simone-coelho

djha-skin commented 2 years ago

No blockers, thanks!