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)

Signed-off-by: Ping Xie <pingxie@google.com>
This commit is contained in:
Binbin 2024-03-12 14:47:43 +08:00 committed by Ping Xie
parent d9e20f2964
commit 306ce81c10
3 changed files with 37 additions and 2 deletions

View File

@ -117,7 +117,9 @@ aofInfo *aofInfoDup(aofInfo *orig) {
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 valkey-check-aof as well. */
sds aofInfoFormat(sds buf, aofInfo *ai) {
sds filename_repr = NULL;

View File

@ -233,6 +233,7 @@ int checkSingleAof(char *aof_filename, char *aof_filepath, int last_file, int fi
struct redis_stat sb;
if (redis_fstat(fileno(fp),&sb) == -1) {
printf("Cannot stat file: %s, aborting...\n", aof_filename);
fclose(fp);
exit(1);
}
@ -343,6 +344,7 @@ int fileIsRDB(char *filepath) {
struct redis_stat sb;
if (redis_fstat(fileno(fp), &sb) == -1) {
printf("Cannot stat file: %s\n", filepath);
fclose(fp);
exit(1);
}
@ -379,6 +381,7 @@ int fileIsManifest(char *filepath) {
struct redis_stat sb;
if (redis_fstat(fileno(fp), &sb) == -1) {
printf("Cannot stat file: %s\n", filepath);
fclose(fp);
exit(1);
}
@ -395,15 +398,20 @@ int fileIsManifest(char *filepath) {
break;
} else {
printf("Cannot read file: %s\n", filepath);
fclose(fp);
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] == '#') {
continue;
} else if (!memcmp(buf, "file", strlen("file"))) {
is_manifest = 1;
} else {
break;
}
}

View File

@ -529,6 +529,18 @@ tags {"aof external:skip"} {
assert_match "*Start checking Old-Style AOF*is valid*" $result
}
test {Test valkey-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/valkey-check-aof $aof_file
} result
assert_match "*Start checking Old-Style AOF*is valid*" $result
}
test {Test valkey-check-aof for old style rdb-preamble AOF} {
catch {
exec src/valkey-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
}
test {Test valkey-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/valkey-check-aof $aof_manifest_file
} result
assert_match "*Invalid AOF manifest file format*" $result
}
test {Test valkey-check-aof only truncates the last file for Multi Part AOF in fix mode} {
create_aof $aof_dirpath $aof_base_file {
append_to_aof [formatCommand set foo hello]