Skip to content

Commit bc750a5

Browse files
[orchagent] Add value validation for RouteOrch, NhgOrch and Srv6Orch (#3596)
Add null checks for variable assignments in RouteOrch, NhgOrch and Srv6Orch. The assignments will only proceed if the string is not empty. What I did Add value validity checks to the assignment part of SET_COMMAND in the doTask methods for RouteOrch, NhgOrch and Srv6Orch, ensuring that the values assigned to fields are not empty. Why I did it For the value obtained by orchagent, assigning an empty value without validation may cause an issue. So add a validity check before assigning the value.
1 parent d1fb9f7 commit bc750a5

File tree

3 files changed

+21
-22
lines changed

3 files changed

+21
-22
lines changed

orchagent/nhgorch.cpp

+7-8
Original file line numberDiff line numberDiff line change
@@ -70,29 +70,28 @@ void NhgOrch::doTask(Consumer& consumer)
7070
/* Get group's next hop IPs and aliases */
7171
for (auto i : kfvFieldsValues(t))
7272
{
73-
if (fvField(i) == "nexthop")
73+
if (fvField(i) == "nexthop" && fvValue(i) != "")
7474
ips = fvValue(i);
7575

76-
if (fvField(i) == "ifname")
76+
if (fvField(i) == "ifname" && fvValue(i) != "")
7777
aliases = fvValue(i);
7878

79-
if (fvField(i) == "weight")
79+
if (fvField(i) == "weight" && fvValue(i) != "")
8080
weights = fvValue(i);
8181

82-
if (fvField(i) == "mpls_nh")
82+
if (fvField(i) == "mpls_nh" && fvValue(i) != "")
8383
mpls_nhs = fvValue(i);
8484

85-
if (fvField(i) == "seg_src")
85+
if (fvField(i) == "seg_src" && fvValue(i) != "")
8686
{
8787
srv6_source = fvValue(i);
8888
srv6_nh = true;
8989
}
9090

91-
if (fvField(i) == "nexthop_group")
91+
if (fvField(i) == "nexthop_group" && fvValue(i) != "")
9292
{
9393
nhgs = fvValue(i);
94-
if (!nhgs.empty())
95-
is_recursive = true;
94+
is_recursive = true;
9695
}
9796
}
9897
/* A NHG should not have both regular(ip/alias) and recursive fields */

orchagent/routeorch.cpp

+12-12
Original file line numberDiff line numberDiff line change
@@ -742,58 +742,58 @@ void RouteOrch::doTask(Consumer& consumer)
742742

743743
for (auto i : kfvFieldsValues(t))
744744
{
745-
if (fvField(i) == "nexthop")
745+
if (fvField(i) == "nexthop" && fvValue(i) != "")
746746
ips = fvValue(i);
747747

748-
if (fvField(i) == "ifname")
748+
if (fvField(i) == "ifname" && fvValue(i) != "")
749749
aliases = fvValue(i);
750750

751-
if (fvField(i) == "mpls_nh")
751+
if (fvField(i) == "mpls_nh" && fvValue(i) != "")
752752
mpls_nhs = fvValue(i);
753753

754-
if (fvField(i) == "vni_label") {
754+
if (fvField(i) == "vni_label" && fvValue(i) != "") {
755755
vni_labels = fvValue(i);
756756
overlay_nh = true;
757757
}
758758

759-
if (fvField(i) == "router_mac")
759+
if (fvField(i) == "router_mac" && fvValue(i) != "")
760760
remote_macs = fvValue(i);
761761

762762
if (fvField(i) == "blackhole")
763763
blackhole = fvValue(i) == "true";
764764

765-
if (fvField(i) == "weight")
765+
if (fvField(i) == "weight" && fvValue(i) != "")
766766
weights = fvValue(i);
767767

768-
if (fvField(i) == "nexthop_group")
768+
if (fvField(i) == "nexthop_group" && fvValue(i) != "")
769769
nhg_index = fvValue(i);
770770

771-
if (fvField(i) == "segment") {
771+
if (fvField(i) == "segment" && fvValue(i) != "") {
772772
srv6_segments = fvValue(i);
773773
srv6_seg = true;
774774
srv6_nh = true;
775775
}
776776

777-
if (fvField(i) == "seg_src") {
777+
if (fvField(i) == "seg_src" && fvValue(i) != "") {
778778
srv6_source = fvValue(i);
779779
srv6_nh = true;
780780
}
781781

782-
if (fvField(i) == "protocol")
782+
if (fvField(i) == "protocol" && fvValue(i) != "")
783783
{
784784
ctx.protocol = fvValue(i);
785785
}
786786

787787
if (fvField(i) == "fallback_to_default_route")
788788
fallback_to_default_route = fvValue(i) == "true";
789789

790-
if (fvField(i) == "vpn_sid") {
790+
if (fvField(i) == "vpn_sid" && fvValue(i) != "") {
791791
srv6_vpn_sids = fvValue(i);
792792
srv6_nh = true;
793793
srv6_vpn = true;
794794
}
795795

796-
if (fvField(i) == "pic_context_id")
796+
if (fvField(i) == "pic_context_id" && fvValue(i) != "")
797797
{
798798
context_index = fvValue(i);
799799
srv6_vpn = true;

orchagent/srv6orch.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -2032,11 +2032,11 @@ task_process_status Srv6Orch::doTaskPicContextTable(const KeyOpFieldsValuesTuple
20322032
pci.ref_count = 0;
20332033
for (auto i : kfvFieldsValues(tuple))
20342034
{
2035-
if (fvField(i) == "nexthop")
2035+
if (fvField(i) == "nexthop" && fvValue(i) != "")
20362036
{
20372037
pci.nexthops = tokenize(fvValue(i), ',');
20382038
}
2039-
else if (fvField(i) == "vpn_sid")
2039+
else if (fvField(i) == "vpn_sid" && fvValue(i) != "")
20402040
{
20412041
pci.sids = tokenize(fvValue(i), ',');
20422042
}

0 commit comments

Comments
 (0)