Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recover from potential panic when doing map to JSON serialization #161

Merged
merged 13 commits into from
Oct 16, 2023
20 changes: 19 additions & 1 deletion gnmi_server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3713,6 +3713,24 @@ func TestTableData2MsiUseKey(t *testing.T) {
}
}

func TestRecoverFromJSONSerializationPanic(t *testing.T) {
panicMarshal := func(v interface{}) ([]byte, error) {
panic("json.Marshal panics and is unable to serialize JSON")
}
mock := gomonkey.ApplyFunc(json.Marshal, panicMarshal)
defer mock.Reset()

tblPath := sdc.CreateTablePath("STATE_DB", "NEIGH_STATE_TABLE", "|", "10.0.0.57")
msi := make(map[string]interface{})
sdc.TableData2Msi(&tblPath, true, nil, &msi)

typedValue, err := sdc.Msi2TypedValue(msi)
if typedValue != nil && err != nil {
t.Errorf("Test should recover from panic and have nil TypedValue/Error after attempting JSON serialization")
}

}

func TestGnmiSetBatch(t *testing.T) {
mockCode :=
`
Expand Down Expand Up @@ -4087,4 +4105,4 @@ func init() {

// Inform gNMI server to use redis tcp localhost connection
sdc.UseRedisLocalTcpPort = true
}
}
26 changes: 20 additions & 6 deletions sonic_data_client/db_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ func (c *DbClient) PollRun(q *queue.PriorityQueue, poll chan struct{}, w *sync.W
for gnmiPath, tblPaths := range c.pathG2S {
val, err := tableData2TypedValue(tblPaths, nil)
if err != nil {
log.V(2).Infof("Unable to create gnmi TypedValue due to err: %v", err)
return
}

Expand Down Expand Up @@ -721,6 +722,12 @@ func makeJSON_redis(msi *map[string]interface{}, key *string, op *string, mfv ma
// emitJSON marshalls map[string]interface{} to JSON byte stream.
func emitJSON(v *map[string]interface{}) ([]byte, error) {
//j, err := json.MarshalIndent(*v, "", indentString)
defer func() {
if r := recover(); r != nil {
log.V(2).Infof("Recovered from panic: %v", r)
log.V(2).Infof("Current state of map to be serialized is: %v", *v)
}
}()
j, err := json.Marshal(*v)
if err != nil {
return nil, fmt.Errorf("JSON marshalling error: %v", err)
Expand Down Expand Up @@ -759,9 +766,12 @@ func TableData2Msi(tblPath *tablePath, useKey bool, op *string, msi *map[string]
dbkeys = []string{tblPath.tableName + tblPath.delimitor + tblPath.tableKey}
}

log.V(4).Infof("dbkeys to be pulled from redis %v", dbkeys)

// Asked to use jsonField and jsonTableKey in the final json value
if tblPath.jsonField != "" && tblPath.jsonTableKey != "" {
val, err := redisDb.HGet(dbkeys[0], tblPath.field).Result()
log.V(4).Infof("Data pulled for key %s and field %s: %s", dbkeys[0], tblPath.field, val)
if err != nil {
log.V(3).Infof("redis HGet failed for %v %v", tblPath, err)
// ignore non-existing field which was derived from virtual path
Expand All @@ -779,7 +789,7 @@ func TableData2Msi(tblPath *tablePath, useKey bool, op *string, msi *map[string]
log.V(2).Infof("redis HGetAll failed for %v, dbkey %s", tblPath, dbkey)
return err
}

log.V(4).Infof("Data pulled for dbkey %s: %v", dbkey, fv)
if tblPath.jsonTableKey != "" { // If jsonTableKey was prepared, use it
err = makeJSON_redis(msi, &tblPath.jsonTableKey, op, fv)
} else if (tblPath.tableKey != "" && !useKey) || tblPath.tableName == dbkey {
Expand All @@ -803,12 +813,16 @@ func TableData2Msi(tblPath *tablePath, useKey bool, op *string, msi *map[string]
return nil
}

func msi2TypedValue(msi map[string]interface{}) (*gnmipb.TypedValue, error) {
func Msi2TypedValue(msi map[string]interface{}) (*gnmipb.TypedValue, error) {
log.V(4).Infof("State of map after adding redis data %v", msi)
jv, err := emitJSON(&msi)
if err != nil {
log.V(2).Infof("emitJSON err %s for %v", err, msi)
return nil, fmt.Errorf("emitJSON err %s for %v", err, msi)
}
if jv == nil { // json and err is nil because panic potentially happened
return nil, fmt.Errorf("emitJSON failed to grab json value of map due to potential panic")
}
return &gnmipb.TypedValue{
Value: &gnmipb.TypedValue_JsonIetfVal{
JsonIetfVal: jv,
Expand Down Expand Up @@ -839,20 +853,20 @@ func tableData2TypedValue(tblPaths []tablePath, op *string) (*gnmipb.TypedValue,
log.V(2).Infof("redis HGet failed for %v", tblPath)
return nil, err
}
log.V(4).Infof("Data pulled for key %s and field %s: %s", key, tblPath.field, val)
// TODO: support multiple table paths
return &gnmipb.TypedValue{
Value: &gnmipb.TypedValue_StringVal{
StringVal: val,
}}, nil
}
}

err := TableData2Msi(&tblPath, useKey, nil, &msi)
if err != nil {
return nil, err
}
}
return msi2TypedValue(msi)
return Msi2TypedValue(msi)
}

func enqueueFatalMsg(c *DbClient, msg string) {
Expand Down Expand Up @@ -921,7 +935,7 @@ func dbFieldMultiSubscribe(c *DbClient, gnmiPath *gnmipb.Path, onChange bool, in
}

sendVal := func(msi map[string]interface{}) error {
val, err := msi2TypedValue(msi)
val, err := Msi2TypedValue(msi)
if err != nil {
enqueueFatalMsg(c, err.Error())
return err
Expand Down Expand Up @@ -1178,7 +1192,7 @@ func dbTableKeySubscribe(c *DbClient, gnmiPath *gnmipb.Path, interval time.Durat
sendDeleteField = true
}
delete(msiData, "delete")
val, err := msi2TypedValue(msiData)
val, err := Msi2TypedValue(msiData)
if err != nil {
return err
}
Expand Down