Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix redis-check-aof incorrectly considering data in manifest format as MP-AOF #12958

Merged
merged 3 commits into from
Mar 12, 2024

Conversation

enjoy-binbin
Copy link
Collaborator

@enjoy-binbin enjoy-binbin commented Jan 17, 2024

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)

…s MP-AOF

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 redis#12951, if the preamble aof also contains it, it will also fail.
Fixes redis#12951.
src/redis-check-aof.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@chenyang8094 chenyang8094 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so 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)

@enjoy-binbin
Copy link
Collaborator Author

yes

@oranagra oranagra merged commit da727ad into redis:unstable Mar 12, 2024
13 checks passed
@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Mar 12, 2024
@enjoy-binbin enjoy-binbin deleted the fix_check_aof branch March 12, 2024 06:55
YaacovHazan pushed a commit to YaacovHazan/redis that referenced this pull request May 16, 2024
…s MP-AOF (redis#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 redis#12951, if the preamble aof also contains it, it will also fail.
Fixes redis#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 da727ad)
YaacovHazan pushed a commit that referenced this pull request May 19, 2024
…s 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 da727ad)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes indication that this issue needs to be mentioned in the release notes
Projects
Status: In Progress
Status: Done
Development

Successfully merging this pull request may close these issues.

[BUG] redis-check-aof fails when aof file is ok
3 participants