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

locator: correctly reject patches adding a file for non-empty files #67

Merged
merged 2 commits into from
Feb 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/patch/locator.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ struct Location {
LineNumber offset { -1 };
};

LineNumber expected_line_number(const Hunk& hunk);

Location locate_hunk(const std::vector<Line>& content, const Hunk& hunk, bool ignore_whitespace = false, LineNumber offset = 0, LineNumber max_fuzz = 2);

bool matches_ignoring_whitespace(const std::string& as, const std::string& bs);
Expand Down
2 changes: 1 addition & 1 deletion src/applier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ static void print_hunk_statistics(std::ostream& out, size_t hunk_num, bool skipp
}
out << ".\n";
} else {
out << hunk.old_file_range.start_line + offset_old_lines_to_new << ".\n";
out << expected_line_number(hunk) + offset_old_lines_to_new << ".\n";
}
}

Expand Down
25 changes: 16 additions & 9 deletions src/locator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ bool matches(const Line& line1, const Line& line2, bool ignore_whitespace)
return matches_ignoring_whitespace(line1.content, line2.content);
}

static LineNumber expected_line_number(const Hunk& hunk)
LineNumber expected_line_number(const Hunk& hunk)
{
auto line = hunk.old_file_range.start_line;
if (hunk.old_file_range.number_of_lines == 0)
Expand All @@ -93,6 +93,21 @@ Location locate_hunk(const std::vector<Line>& content, const Hunk& hunk, bool ig
// Make a first best guess at where the from-file range is telling us where the hunk should be.
LineNumber offset_guess = expected_line_number(hunk) - 1 + offset;

// If there's no lines surrounding this hunk - it will always succeed,
// so there is no point in checking any further. Note that this check is
// also what makes matching against an empty 'from file' work (with no lines),
// as in that case there is no content for us to even match against in the
// first place!
//
// Furthermore, we also should reject patches being added when the hunk is
// claiming the file is completely empty - but there are actually lines in
// that file.
if (hunk.old_file_range.number_of_lines == 0) {
if (hunk.old_file_range.start_line == 0 && !content.empty())
return {};
return { offset_guess, 0, 0 };
}

LineNumber patch_prefix_content = 0;
for (const auto& line : hunk.lines) {
if (line.operation != ' ')
Expand All @@ -109,14 +124,6 @@ Location locate_hunk(const std::vector<Line>& content, const Hunk& hunk, bool ig

LineNumber context = std::max(patch_prefix_content, patch_suffix_content);

// If there's no lines surrounding this hunk - it will always succeed,
// so there is no point in checking any further. Note that this check is
// also what makes matching against an empty 'from file' work (with no lines),
// as in that case there is no content for us to even match against in the
// first place!
if (hunk.old_file_range.number_of_lines == 0)
return { offset_guess, 0, 0 };

for (LineNumber fuzz = 0; fuzz <= max_fuzz; ++fuzz) {

auto suffix_fuzz = std::max<LineNumber>(fuzz + patch_suffix_content - context, 0);
Expand Down
Loading