cli/doctor: handle truncated rows gracefully in zipdir

When debug doctor is run on zipdir files which are corrupt / incomplete
it can end up crashing. This patch aims to generate a more productive
error message if any row does not have enough fields in any file format.
Currently this always hits an index out of range error.

Fixes: #147722

Release note: None

Author: fqazi

pkg/cli/doctor.go

@@ -376,6 +376,9 @@ func fromZipDir(
 	if checkIfFileExists(zipDirPath, "system.descriptor.txt") {
 		if err := slurp(zipDirPath, "system.descriptor.txt", func(row string) error {
 			fields := strings.Fields(row)
+			if len(fields) != 2 {
+				return errors.Errorf("expected 2 fields, got %d in system.descriptors.txt", len(fields))
+			}
 			last := len(fields) - 1
 			i, err := strconv.Atoi(fields[0])
 			if err != nil {
@@ -432,6 +435,9 @@ func fromZipDir(
 	if checkIfFileExists(zipDirPath, namespaceFileName) {
 		if err := slurp(zipDirPath, namespaceFileName, func(row string) error {
 			fields := strings.Fields(row)
+			if len(fields) != 4 {
+				return errors.Errorf("expected 4 fields, got %d in system.namespace.txtq", len(fields))
+			}
 			parID, err := strconv.Atoi(fields[0])
 			if err != nil {
 				return errors.Wrapf(err, "failed to parse parent id %s", fields[0])
@@ -505,6 +511,9 @@ func fromZipDir(
 	if checkIfFileExists(zipDirPath, "crdb_internal.system_jobs.txt") {
 		if err := slurp(zipDirPath, "crdb_internal.system_jobs.txt", func(row string) error {
 			fields := strings.Fields(row)
+			if len(fields) < 6 {
+				return errors.Errorf("expected at least 6 fields, got %d in crdb_internal.system_jobs.txt", len(fields))
+			}
 			md := jobs.JobMetadata{}
 			md.State = jobs.State(fields[1])
 

pkg/cli/doctor_test.go

@@ -260,6 +260,42 @@ func TestDoctorZipDir(t *testing.T) {
 		})
 	})
 
+	t.Run("examine", func(t *testing.T) {
+		out, err := c.RunWithCapture("debug doctor examine zipdir testdata/doctor/debugzip-bad-descriptors 21.1-52")
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// Using datadriven allows TESTFLAGS=-rewrite.
+		datadriven.RunTest(t, datapathutils.TestDataPath(t, "doctor", "test_examine_zipdir_bad_descriptors"), func(t *testing.T, td *datadriven.TestData) string {
+			return out
+		})
+	})
+
+	t.Run("examine", func(t *testing.T) {
+		out, err := c.RunWithCapture("debug doctor examine zipdir testdata/doctor/debugzip-bad-jobs 21.1-52")
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// Using datadriven allows TESTFLAGS=-rewrite.
+		datadriven.RunTest(t, datapathutils.TestDataPath(t, "doctor", "test_examine_zipdir_bad_jobs"), func(t *testing.T, td *datadriven.TestData) string {
+			return out
+		})
+	})
+
+	t.Run("examine", func(t *testing.T) {
+		out, err := c.RunWithCapture("debug doctor examine zipdir testdata/doctor/debugzip-bad-namespace 21.1-52")
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// Using datadriven allows TESTFLAGS=-rewrite.
+		datadriven.RunTest(t, datapathutils.TestDataPath(t, "doctor", "test_examine_zipdir_bad_namespace"), func(t *testing.T, td *datadriven.TestData) string {
+			return out
+		})
+	})
+
 	// Regression test (for #104347) to ensure that quoted table names get properly parsed in system.namespace.
 	t.Run("examine", func(t *testing.T) {
 		out, err := c.RunWithCapture("debug doctor examine zipdir testdata/doctor/debugzip-with-quotes")

pkg/cli/testdata/doctor/debugzip-bad-descriptors/crdb_internal.system_jobs.txt

@@ -0,0 +1,3 @@
+
+few
+fields

pkg/cli/testdata/doctor/debugzip-bad-descriptors/system.descriptor.txt

@@ -0,0 +1 @@
+something might have happened

pkg/cli/testdata/doctor/debugzip-bad-descriptors/system.jobs.txt.err.txt

@@ -0,0 +1,59 @@
+parentID	parentSchemaID	name	id
+0	0	defaultdb	100
+0	0	postgres	102
+0	0	system	1
+1	0	public	29
+1	29	comments	24
+1	29	database_role_settings	44
+1	29	descriptor	3
+1	29	descriptor_id_seq	7
+1	29	eventlog	12
+1	29	external_connections	53
+1	29	job_info	54
+1	29	jobs	15
+1	29	join_tokens	41
+1	29	lease	11
+1	29	locations	21
+1	29	migrations	40
+1	29	mvcc_statistics	64
+1	29	namespace	30
+1	29	privileges	52
+1	29	protected_ts_meta	31
+1	29	protected_ts_records	32
+1	29	rangelog	13
+1	29	region_liveness	9
+1	29	replication_constraint_stats	25
+1	29	replication_critical_localities	26
+1	29	replication_stats	27
+1	29	reports_meta	28
+1	29	role_id_seq	48
+1	29	role_members	23
+1	29	role_options	33
+1	29	scheduled_jobs	37
+1	29	settings	6
+1	29	span_configurations	47
+1	29	span_count	51
+1	29	span_stats_buckets	56
+1	29	span_stats_samples	57
+1	29	span_stats_tenant_boundaries	58
+1	29	span_stats_unique_keys	55
+1	29	sql_instances	46
+1	29	sqlliveness	39
+1	29	statement_activity	61
+1	29	statement_bundle_chunks	34
+1	29	statement_diagnostics	36
+1	29	statement_diagnostics_requests	35
+1	29	statement_execution_insights	66
+1	29	statement_statistics	42
+1	29	table_statistics	20
+1	29	task_payloads	59
+1	29	tenant_id_seq	63
+1	29	tenant_settings	50
+1	29	tenant_tasks	60
+1	29	tenant_usage	45
+1	29	tenants	8
+1	29	transaction_activity	62
+1	29	transaction_execution_insights	65
+1	29	transaction_statistics	43
+1	29	zones	5
+100	0	public	101

pkg/cli/testdata/doctor/debugzip-bad-descriptors/system.namespace.txt

@@ -0,0 +1,2 @@
+
+too few fields

pkg/cli/testdata/doctor/debugzip-bad-jobs/crdb_internal.system_jobs.txt

@@ -0,0 +1 @@
+something might have happened

pkg/cli/testdata/doctor/debugzip-bad-jobs/system.descriptor.txt

@@ -0,0 +1,59 @@
+parentID	parentSchemaID	name	id
+0	0	defaultdb	100
+0	0	postgres	102
+0	0	system	1
+1	0	public	29
+1	29	comments	24
+1	29	database_role_settings	44
+1	29	descriptor	3
+1	29	descriptor_id_seq	7
+1	29	eventlog	12
+1	29	external_connections	53
+1	29	job_info	54
+1	29	jobs	15
+1	29	join_tokens	41
+1	29	lease	11
+1	29	locations	21
+1	29	migrations	40
+1	29	mvcc_statistics	64
+1	29	namespace	30
+1	29	privileges	52
+1	29	protected_ts_meta	31
+1	29	protected_ts_records	32
+1	29	rangelog	13
+1	29	region_liveness	9
+1	29	replication_constraint_stats	25
+1	29	replication_critical_localities	26
+1	29	replication_stats	27
+1	29	reports_meta	28
+1	29	role_id_seq	48
+1	29	role_members	23
+1	29	role_options	33
+1	29	scheduled_jobs	37
+1	29	settings	6
+1	29	span_configurations	47
+1	29	span_count	51
+1	29	span_stats_buckets	56
+1	29	span_stats_samples	57
+1	29	span_stats_tenant_boundaries	58
+1	29	span_stats_unique_keys	55
+1	29	sql_instances	46
+1	29	sqlliveness	39
+1	29	statement_activity	61
+1	29	statement_bundle_chunks	34
+1	29	statement_diagnostics	36
+1	29	statement_diagnostics_requests	35
+1	29	statement_execution_insights	66
+1	29	statement_statistics	42
+1	29	table_statistics	20
+1	29	task_payloads	59
+1	29	tenant_id_seq	63
+1	29	tenant_settings	50
+1	29	tenant_tasks	60
+1	29	tenant_usage	45
+1	29	tenants	8
+1	29	transaction_activity	62
+1	29	transaction_execution_insights	65
+1	29	transaction_statistics	43
+1	29	zones	5
+100	0	public	101