Skip to content

Commit

Permalink
Recover from potential panic when doing map to JSON serialization (#161
Browse files Browse the repository at this point in the history
…) (#169)
  • Loading branch information
zbud-msft authored Oct 27, 2023
1 parent 537898d commit 67f5e31
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 6 deletions.
18 changes: 18 additions & 0 deletions gnmi_server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2536,6 +2536,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 init() {
// Enable logs at UT setup
flag.Lookup("v").Value.Set("10")
Expand Down
26 changes: 20 additions & 6 deletions sonic_data_client/db_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,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 @@ -592,6 +593,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 @@ -630,9 +637,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 @@ -650,7 +660,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 @@ -671,12 +681,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 @@ -707,20 +721,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 @@ -789,7 +803,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 @@ -1037,7 +1051,7 @@ func dbTableKeySubscribe(c *DbClient, gnmiPath *gnmipb.Path, interval time.Durat

// Helper to send hash data over the stream
sendMsiData := func(msiData map[string]interface{}) error {
val, err := msi2TypedValue(msiData)
val, err := Msi2TypedValue(msiData)
if err != nil {
return err
}
Expand Down

0 comments on commit 67f5e31

Please sign in to comment.