Fix integer parsing logic

We have various places where the types of what we parse and then use don't align. This is something CodeQL correctly warns about that it can lead to unexpected behavior with truncation. This fixes those cases that CodeQL is complaining about. Signed-off-by: Dirkjan Bussink <[email protected]>
vitessio · Jan 29, 2025 · 9e8ba03 · 9e8ba03
1 parent 9c6c380
commit 9e8ba03
Show file tree

Hide file tree

Showing 14 changed files with 53 additions and 46 deletions.
diff --git a/go/vt/vtcombo/tablet_map.go b/go/vt/vtcombo/tablet_map.go
@@ -1005,7 +1005,7 @@ func (itmc *internalTabletManagerClient) PopulateReparentJournal(context.Context
 }
 
 // ReadReparentJournalInfo is part of the tmclient.TabletManagerClient interface.
-func (itmc *internalTabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error) {
+func (itmc *internalTabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error) {
 	return 0, fmt.Errorf("not implemented in vtcombo")
 }
 

diff --git a/go/vt/vtctl/grpcvtctldserver/server.go b/go/vt/vtctl/grpcvtctldserver/server.go
@@ -22,6 +22,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"math"
 	"net/http"
 	"path/filepath"
 	"runtime/debug"
@@ -1509,11 +1510,17 @@ func (s *VtctldServer) GetBackups(ctx context.Context, req *vtctldatapb.GetBacku
 
 	totalBackups := len(bhs)
 	if req.Limit > 0 {
+		if req.Limit > math.MaxInt {
+			return nil, fmt.Errorf("limit %v exceeds maximum allowed value %v", req.DetailedLimit, math.MaxInt)
+		}
 		totalBackups = int(req.Limit)
 	}
 
 	totalDetailedBackups := len(bhs)
 	if req.DetailedLimit > 0 {
+		if req.DetailedLimit > math.MaxInt {
+			return nil, fmt.Errorf("detailed_limit %v exceeds maximum allowed value %v", req.DetailedLimit, math.MaxInt)
+		}
 		totalDetailedBackups = int(req.DetailedLimit)
 	}
 

diff --git a/go/vt/vtctl/grpcvtctldserver/testutil/test_tmclient.go b/go/vt/vtctl/grpcvtctldserver/testutil/test_tmclient.go
@@ -295,7 +295,7 @@ type TabletManagerClient struct {
 	// keyed by tablet alias
 	PopulateReparentJournalResults map[string]error
 	// keyed by tablet alias
-	ReadReparentJournalInfoResults map[string]int
+	ReadReparentJournalInfoResults map[string]int32
 	// keyed by tablet alias.
 	PromoteReplicaDelays map[string]time.Duration
 	// keyed by tablet alias. injects a sleep to the end of the function
@@ -974,7 +974,7 @@ func (fake *TabletManagerClient) PopulateReparentJournal(ctx context.Context, ta
 }
 
 // ReadReparentJournalInfo is part of the tmclient.TabletManagerClient interface.
-func (fake *TabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error) {
+func (fake *TabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error) {
 	if fake.ReadReparentJournalInfoResults == nil {
 		return 1, nil
 	}

diff --git a/go/vt/vtctl/reparentutil/emergency_reparenter.go b/go/vt/vtctl/reparentutil/emergency_reparenter.go
@@ -805,7 +805,7 @@ func (erp *EmergencyReparenter) findErrantGTIDs(
 	}
 
 	// Find the maximum length of the reparent journal among all the candidates.
-	var maxLen int
+	var maxLen int32
 	for _, length := range reparentJournalLen {
 		maxLen = max(maxLen, length)
 	}
@@ -902,8 +902,8 @@ func (erp *EmergencyReparenter) gatherReparenJournalInfo(
 	validCandidates map[string]replication.Position,
 	tabletMap map[string]*topo.TabletInfo,
 	waitReplicasTimeout time.Duration,
-) (map[string]int, error) {
-	reparentJournalLen := make(map[string]int)
+) (map[string]int32, error) {
+	reparentJournalLen := make(map[string]int32)
 	var mu sync.Mutex
 	errCh := make(chan concurrency.Error)
 	defer close(errCh)
@@ -916,7 +916,7 @@ func (erp *EmergencyReparenter) gatherReparenJournalInfo(
 	for candidate := range validCandidates {
 		go func(alias string) {
 			var err error
-			var length int
+			var length int32
 			defer func() {
 				errCh <- concurrency.Error{
 					Err: err,

diff --git a/go/vt/vtctl/reparentutil/emergency_reparenter_test.go b/go/vt/vtctl/reparentutil/emergency_reparenter_test.go
@@ -4637,7 +4637,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 		{
 			name: "Case 1a: No Errant GTIDs. This is the first reparent. A replica is the most advanced.",
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 1,
 					"zone1-0000000103": 1,
 					"zone1-0000000104": 1,
@@ -4733,7 +4733,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 2,
 					"zone1-0000000103": 2,
 					"zone1-0000000104": 2,
@@ -4797,7 +4797,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 2,
 					"zone1-0000000103": 2,
 					"zone1-0000000104": 2,
@@ -4861,7 +4861,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 3,
 					"zone1-0000000103": 2,
 					"zone1-0000000104": 1,
@@ -4925,7 +4925,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 2,
 					"zone1-0000000103": 2,
 					"zone1-0000000104": 1,
@@ -4989,7 +4989,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 2,
 					"zone1-0000000103": 3,
 					"zone1-0000000104": 3,
@@ -5052,7 +5052,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 2,
 					"zone1-0000000103": 3,
 					"zone1-0000000104": 2,
@@ -5116,7 +5116,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 2,
 					"zone1-0000000103": 3,
 					"zone1-0000000104": 2,
@@ -5179,7 +5179,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 3,
 					"zone1-0000000103": 3,
 					"zone1-0000000104": 3,
@@ -5243,7 +5243,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 3,
 					"zone1-0000000103": 3,
 					"zone1-0000000104": 2,
@@ -5297,7 +5297,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 3,
 					"zone1-0000000103": 3,
 				},
@@ -5354,7 +5354,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 3,
 					"zone1-0000000103": 3,
 					"zone1-0000000104": 3,
@@ -5418,7 +5418,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{
+				ReadReparentJournalInfoResults: map[string]int32{
 					"zone1-0000000102": 2,
 					"zone1-0000000103": 3,
 					"zone1-0000000104": 2,
@@ -5481,7 +5481,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
 				},
 			},
 			tmc: &testutil.TabletManagerClient{
-				ReadReparentJournalInfoResults: map[string]int{},
+				ReadReparentJournalInfoResults: map[string]int32{},
 			},
 			statusMap: map[string]*replicationdatapb.StopReplicationStatus{
 				"zone1-0000000103": {

diff --git a/go/vt/vttablet/common/config.go b/go/vt/vttablet/common/config.go
@@ -142,18 +142,18 @@ func NewVReplicationConfig(overrides map[string]string) (*VReplicationConfig, er
 				c.ExperimentalFlags = value
 			}
 		case "vreplication_net_read_timeout":
-			value, err := strconv.ParseInt(v, 10, 64)
+			value, err := strconv.Atoi(v)
 			if err != nil {
 				errors = append(errors, getError(k, v))
 			} else {
-				c.NetReadTimeout = int(value)
+				c.NetReadTimeout = value
 			}
 		case "vreplication_net_write_timeout":
-			value, err := strconv.ParseInt(v, 10, 64)
+			value, err := strconv.Atoi(v)
 			if err != nil {
 				errors = append(errors, getError(k, v))
 			} else {
-				c.NetWriteTimeout = int(value)
+				c.NetWriteTimeout = value
 			}
 		case "vreplication_copy_phase_duration":
 			value, err := time.ParseDuration(v)
@@ -177,18 +177,18 @@ func NewVReplicationConfig(overrides map[string]string) (*VReplicationConfig, er
 				c.MaxTimeToRetryError = value
 			}
 		case "relay_log_max_size":
-			value, err := strconv.ParseInt(v, 10, 64)
+			value, err := strconv.Atoi(v)
 			if err != nil {
 				errors = append(errors, getError(k, v))
 			} else {
-				c.RelayLogMaxSize = int(value)
+				c.RelayLogMaxSize = value
 			}
 		case "relay_log_max_items":
-			value, err := strconv.ParseInt(v, 10, 64)
+			value, err := strconv.Atoi(v)
 			if err != nil {
 				errors = append(errors, getError(k, v))
 			} else {
-				c.RelayLogMaxItems = int(value)
+				c.RelayLogMaxItems = value
 			}
 		case "vreplication_replica_lag_tolerance":
 			value, err := time.ParseDuration(v)
@@ -198,11 +198,11 @@ func NewVReplicationConfig(overrides map[string]string) (*VReplicationConfig, er
 				c.ReplicaLagTolerance = value
 			}
 		case "vreplication_heartbeat_update_interval":
-			value, err := strconv.ParseInt(v, 10, 64)
+			value, err := strconv.Atoi(v)
 			if err != nil {
 				errors = append(errors, getError(k, v))
 			} else {
-				c.HeartbeatUpdateInterval = int(value)
+				c.HeartbeatUpdateInterval = value
 			}
 		case "vreplication_store_compressed_gtid":
 			value, err := strconv.ParseBool(v)
@@ -212,19 +212,19 @@ func NewVReplicationConfig(overrides map[string]string) (*VReplicationConfig, er
 				c.StoreCompressedGTID = value
 			}
 		case "vreplication-parallel-insert-workers":
-			value, err := strconv.ParseInt(v, 10, 64)
+			value, err := strconv.Atoi(v)
 			if err != nil {
 				errors = append(errors, getError(k, v))
 			} else {
-				c.ParallelInsertWorkers = int(value)
+				c.ParallelInsertWorkers = value
 			}
 		case "vstream_packet_size":
-			value, err := strconv.ParseInt(v, 10, 64)
+			value, err := strconv.Atoi(v)
 			if err != nil {
 				errors = append(errors, getError(k, v))
 			} else {
 				c.VStreamPacketSizeOverride = true
-				c.VStreamPacketSize = int(value)
+				c.VStreamPacketSize = value
 			}
 		case "vstream_dynamic_packet_size":
 			value, err := strconv.ParseBool(v)

diff --git a/go/vt/vttablet/faketmclient/fake_client.go b/go/vt/vttablet/faketmclient/fake_client.go
@@ -358,7 +358,7 @@ func (client *FakeTabletManagerClient) PopulateReparentJournal(ctx context.Conte
 }
 
 // ReadReparentJournalInfo is part of the tmclient.TabletManagerClient interface.
-func (client *FakeTabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error) {
+func (client *FakeTabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error) {
 	return 10, nil
 }
 

diff --git a/go/vt/vttablet/grpctmclient/client.go b/go/vt/vttablet/grpctmclient/client.go
@@ -1124,7 +1124,7 @@ func (client *Client) PopulateReparentJournal(ctx context.Context, tablet *topod
 }
 
 // ReadReparentJournalInfo is part of the tmclient.TabletManagerClient interface.
-func (client *Client) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error) {
+func (client *Client) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error) {
 	c, closer, err := client.dialer.dial(ctx, tablet)
 	if err != nil {
 		return 0, err
@@ -1134,7 +1134,7 @@ func (client *Client) ReadReparentJournalInfo(ctx context.Context, tablet *topod
 	if err != nil {
 		return 0, err
 	}
-	return int(resp.Length), nil
+	return resp.Length, nil
 }
 
 // InitReplica is part of the tmclient.TabletManagerClient interface.

diff --git a/go/vt/vttablet/grpctmserver/server.go b/go/vt/vttablet/grpctmserver/server.go
@@ -570,7 +570,7 @@ func (s *server) ReadReparentJournalInfo(ctx context.Context, request *tabletman
 	response = &tabletmanagerdatapb.ReadReparentJournalInfoResponse{}
 	length, err := s.tm.ReadReparentJournalInfo(ctx)
 	if err == nil {
-		response.Length = int32(length)
+		response.Length = length
 	}
 	return response, err
 }

diff --git a/go/vt/vttablet/tabletmanager/rpc_agent.go b/go/vt/vttablet/tabletmanager/rpc_agent.go
@@ -140,7 +140,7 @@ type RPCTM interface {
 
 	PopulateReparentJournal(ctx context.Context, timeCreatedNS int64, actionName string, tabletAlias *topodatapb.TabletAlias, pos string) error
 
-	ReadReparentJournalInfo(ctx context.Context) (int, error)
+	ReadReparentJournalInfo(ctx context.Context) (int32, error)
 
 	InitReplica(ctx context.Context, parent *topodatapb.TabletAlias, replicationPosition string, timeCreatedNS int64, semiSync bool) error
 

diff --git a/go/vt/vttablet/tabletmanager/rpc_replication.go b/go/vt/vttablet/tabletmanager/rpc_replication.go
@@ -428,7 +428,7 @@ func (tm *TabletManager) PopulateReparentJournal(ctx context.Context, timeCreate
 }
 
 // ReadReparentJournalInfo reads the information from reparent journal.
-func (tm *TabletManager) ReadReparentJournalInfo(ctx context.Context) (int, error) {
+func (tm *TabletManager) ReadReparentJournalInfo(ctx context.Context) (int32, error) {
 	log.Infof("ReadReparentJournalInfo")
 	if err := tm.waitForGrantsToHaveApplied(ctx); err != nil {
 		return 0, err
@@ -442,7 +442,7 @@ func (tm *TabletManager) ReadReparentJournalInfo(ctx context.Context) (int, erro
 	if len(res.Rows) != 1 {
 		return 0, vterrors.Errorf(vtrpc.Code_INTERNAL, "unexpected rows when reading reparent journal, got %v", len(res.Rows))
 	}
-	return res.Rows[0][0].ToInt()
+	return res.Rows[0][0].ToInt32()
 }
 
 // InitReplica sets replication primary and position, and waits for the

diff --git a/go/vt/vttablet/tabletserver/twopc.go b/go/vt/vttablet/tabletserver/twopc.go
@@ -569,7 +569,7 @@ func (tpc *TwoPC) UnresolvedTransactions(ctx context.Context, abandonTime time.T
 			appendCurrentTx()
 
 			// Extract the transaction state and initialize a new TransactionMetadata
-			stateID, _ := row[1].ToInt()
+			stateID, _ := row[1].ToInt32()
 			timeCreated, _ := row[2].ToCastInt64()
 			currentTx = &querypb.TransactionMetadata{
 				Dtid:         dtid,

diff --git a/go/vt/vttablet/tmclient/rpc_client_api.go b/go/vt/vttablet/tmclient/rpc_client_api.go
@@ -239,7 +239,7 @@ type TabletManagerClient interface {
 	PopulateReparentJournal(ctx context.Context, tablet *topodatapb.Tablet, timeCreatedNS int64, actionName string, tabletAlias *topodatapb.TabletAlias, pos string) error
 
 	// ReadReparentJournalInfo reads the information from reparent journal
-	ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error)
+	ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error)
 
 	// InitReplica tells a tablet to start replicating from the
 	// passed in primary tablet alias, and wait for the row in the

diff --git a/go/vt/vttablet/tmrpctest/test_tm_rpc.go b/go/vt/vttablet/tmrpctest/test_tm_rpc.go
@@ -1179,9 +1179,9 @@ func (fra *fakeRPCTM) PopulateReparentJournal(ctx context.Context, timeCreatedNS
 	return nil
 }
 
-var testReparentJournalLen = 10
+var testReparentJournalLen int32 = 10
 
-func (fra *fakeRPCTM) ReadReparentJournalInfo(context.Context) (int, error) {
+func (fra *fakeRPCTM) ReadReparentJournalInfo(context.Context) (int32, error) {
 	if fra.panics {
 		panic(fmt.Errorf("test-triggered panic"))
 	}