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) #167

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions gnmi_server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3467,6 +3467,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
26 changes: 20 additions & 6 deletions sonic_data_client/db_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,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 @@ -688,6 +689,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 @@ -726,9 +733,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 @@ -746,7 +756,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 @@ -770,12 +780,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 @@ -806,20 +820,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 @@ -888,7 +902,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 @@ -1140,7 +1154,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
2 changes: 1 addition & 1 deletion sonic_data_client/mixed_db_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ func (c *MixedDbClient) tableData2TypedValue(tblPaths []tablePath, op *string) (
return nil, err
}
}
return msi2TypedValue(msi)
return Msi2TypedValue(msi)
}

func ConvertDbEntry(inputData map[string]interface{}) map[string]string {
Expand Down