mirror of https://github.com/redis/redis.git
				
				
				
			Fix redis-check-aof incorrectly considering data in manifest format as MP-AOF (#12958)
The check in fileIsManifest misjudged the manifest file. For example,
if resp aof contains "file", it will be considered a manifest file and
the check will fail:
```
*3
$3
set
$4
file
$4
file
```
In #12951, if the preamble aof also contains it, it will also fail.
Fixes #12951.
the bug was happening if the the word "file" is mentioned
in the first 1024 lines of the AOF. and now as soon as it finds
a non-comment line it'll break (if it contains "file" or doesn't)
(cherry picked from commit da727ad445)
			
			
This commit is contained in:
		
							parent
							
								
									1caaf581f1
								
							
						
					
					
						commit
						439b8da475
					
				|  | @ -117,7 +117,9 @@ aofInfo *aofInfoDup(aofInfo *orig) { | ||||||
|     return ai; |     return ai; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /* Format aofInfo as a string and it will be a line in the manifest. */ | /* Format aofInfo as a string and it will be a line in the manifest.
 | ||||||
|  |  * | ||||||
|  |  * When update this format, make sure to update redis-check-aof as well. */ | ||||||
| sds aofInfoFormat(sds buf, aofInfo *ai) { | sds aofInfoFormat(sds buf, aofInfo *ai) { | ||||||
|     sds filename_repr = NULL; |     sds filename_repr = NULL; | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -233,6 +233,7 @@ int checkSingleAof(char *aof_filename, char *aof_filepath, int last_file, int fi | ||||||
|     struct redis_stat sb; |     struct redis_stat sb; | ||||||
|     if (redis_fstat(fileno(fp),&sb) == -1) { |     if (redis_fstat(fileno(fp),&sb) == -1) { | ||||||
|         printf("Cannot stat file: %s, aborting...\n", aof_filename); |         printf("Cannot stat file: %s, aborting...\n", aof_filename); | ||||||
|  |         fclose(fp); | ||||||
|         exit(1); |         exit(1); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  | @ -343,6 +344,7 @@ int fileIsRDB(char *filepath) { | ||||||
|     struct redis_stat sb; |     struct redis_stat sb; | ||||||
|     if (redis_fstat(fileno(fp), &sb) == -1) { |     if (redis_fstat(fileno(fp), &sb) == -1) { | ||||||
|         printf("Cannot stat file: %s\n", filepath); |         printf("Cannot stat file: %s\n", filepath); | ||||||
|  |         fclose(fp); | ||||||
|         exit(1); |         exit(1); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  | @ -379,6 +381,7 @@ int fileIsManifest(char *filepath) { | ||||||
|     struct redis_stat sb; |     struct redis_stat sb; | ||||||
|     if (redis_fstat(fileno(fp), &sb) == -1) { |     if (redis_fstat(fileno(fp), &sb) == -1) { | ||||||
|         printf("Cannot stat file: %s\n", filepath); |         printf("Cannot stat file: %s\n", filepath); | ||||||
|  |         fclose(fp); | ||||||
|         exit(1); |         exit(1); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  | @ -395,15 +398,20 @@ int fileIsManifest(char *filepath) { | ||||||
|                 break; |                 break; | ||||||
|             } else { |             } else { | ||||||
|                 printf("Cannot read file: %s\n", filepath); |                 printf("Cannot read file: %s\n", filepath); | ||||||
|  |                 fclose(fp); | ||||||
|                 exit(1); |                 exit(1); | ||||||
|             } |             } | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         /* Skip comments lines */ |         /* We will skip comments lines.
 | ||||||
|  |          * At present, the manifest format is fixed, see aofInfoFormat. | ||||||
|  |          * We will break directly as long as it encounters other items. */ | ||||||
|         if (buf[0] == '#') { |         if (buf[0] == '#') { | ||||||
|             continue; |             continue; | ||||||
|         } else if (!memcmp(buf, "file", strlen("file"))) { |         } else if (!memcmp(buf, "file", strlen("file"))) { | ||||||
|             is_manifest = 1; |             is_manifest = 1; | ||||||
|  |         } else { | ||||||
|  |             break; | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -529,6 +529,18 @@ tags {"aof external:skip"} { | ||||||
|         assert_match "*Start checking Old-Style AOF*is valid*" $result |         assert_match "*Start checking Old-Style AOF*is valid*" $result | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |     test {Test redis-check-aof for old style resp AOF - has data in the same format as manifest} { | ||||||
|  |         create_aof $aof_dirpath $aof_file { | ||||||
|  |             append_to_aof [formatCommand set file file] | ||||||
|  |             append_to_aof [formatCommand set "file appendonly.aof.2.base.rdb seq 2 type b" "file appendonly.aof.2.base.rdb seq 2 type b"] | ||||||
|  |         } | ||||||
|  | 
 | ||||||
|  |         catch { | ||||||
|  |             exec src/redis-check-aof $aof_file | ||||||
|  |         } result | ||||||
|  |         assert_match "*Start checking Old-Style AOF*is valid*" $result | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|     test {Test redis-check-aof for old style rdb-preamble AOF} { |     test {Test redis-check-aof for old style rdb-preamble AOF} { | ||||||
|         catch { |         catch { | ||||||
|             exec src/redis-check-aof tests/assets/rdb-preamble.aof |             exec src/redis-check-aof tests/assets/rdb-preamble.aof | ||||||
|  | @ -577,6 +589,19 @@ tags {"aof external:skip"} { | ||||||
|         assert_match "*Start checking Multi Part AOF*Start to check BASE AOF (RDB format)*DB preamble is OK, proceeding with AOF tail*BASE AOF*is valid*Start to check INCR files*INCR AOF*is valid*All AOF files and manifest are valid*" $result |         assert_match "*Start checking Multi Part AOF*Start to check BASE AOF (RDB format)*DB preamble is OK, proceeding with AOF tail*BASE AOF*is valid*Start to check INCR files*INCR AOF*is valid*All AOF files and manifest are valid*" $result | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |     test {Test redis-check-aof for Multi Part AOF contains a format error} { | ||||||
|  |         create_aof_manifest $aof_dirpath $aof_manifest_file { | ||||||
|  |             append_to_manifest "file appendonly.aof.1.base.aof seq 1 type b\n" | ||||||
|  |             append_to_manifest "file appendonly.aof.1.incr.aof seq 1 type i\n" | ||||||
|  |             append_to_manifest "!!!\n" | ||||||
|  |         } | ||||||
|  | 
 | ||||||
|  |         catch { | ||||||
|  |             exec src/redis-check-aof $aof_manifest_file | ||||||
|  |         } result | ||||||
|  |         assert_match "*Invalid AOF manifest file format*" $result | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|     test {Test redis-check-aof only truncates the last file for Multi Part AOF in fix mode} { |     test {Test redis-check-aof only truncates the last file for Multi Part AOF in fix mode} { | ||||||
|         create_aof $aof_dirpath $aof_base_file { |         create_aof $aof_dirpath $aof_base_file { | ||||||
|             append_to_aof [formatCommand set foo hello] |             append_to_aof [formatCommand set foo hello] | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue