patch: improve validation of ed diffs #968

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.

Sign up for GitHub

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

Jump to bottom

Merged

briandfoy merged 1 commit into briandfoy:master from mknos:patch-ed-range

Feb 26, 2025

Contributor

mknos commented Feb 26, 2025

Make more of an effort to reject patches where line addresses are not valid
Reading over the ed command parser in Patch::Ed::apply(), the regex assignment was not always correct
test1: invalid command "19,0d" was incorrectly interpreted as (valid) "19,19d" thanks to the '||' operator
test2: "19,18d" command was accepted, but the range is backwards
test3: "0,19d" command was accepted, but address 0 is not allowed for "d"
test4: "0,19c" command was accepted, but address 0 is not allowed for "c"
I tested this by skipping to PLAN_J, using a diff with an update on line1 (ed command 0a is valid and means append after line 0)
Modifying elements of the diff input, I could trigger the new validation errors

%perl diff -e env env2 | perl tee env.diff # original test input
19,20d
5,8c
META DATA
AND MORE
.
0a
#NEW LINE 1!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
.


          patch: improve validation of ed diffs

af6f9d8

* Make more of an effort to reject patches where line addresses are not valid
* Reading over the ed command parser in Patch::Ed::apply(), the regex assignment was not always correct
* test1: "19,0d" command was incorrectly interpreted as "19,19d" thanks to the '||' operator
* test2: "19,18d" command was accepted, but the range is backwards
* test3: "0,19d" command was accepted, but address 0 is not allowed for "d"
* test4: "0,19c" command was accepted, but address 0 is not allowed for "c"
* I tested this by skipping to PLAN_J, using a modified diff with an update on line1 (ed command 0a is valid and means append after line 0)
* Modifying elements of the diff input, I could trigger the new validation errors

%perl diff -e env env2 | perl tee env.diff
19,20d
5,8c
META DATA
AND MORE
.
0a
#NEW LINE 1!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
.

mknos temporarily deployed to housekeeping

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

github-actions bot added the Priority: low label

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

github-actions bot added the Status: needs verification label

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

github-actions bot added Type: bug Priority: low labels

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

github-actions bot added the Status: needs verification label

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

github-actions bot added the Type: bug label

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

github-actions bot added Type: enhancement Program: patch labels

mknos temporarily deployed to automated_testing

February 26, 2025 03:19

— with

GitHub Actions Inactive

mknos temporarily deployed to automated_testing

February 26, 2025 03:20

— with

GitHub Actions Inactive

coveralls commented Feb 26, 2025 •

edited

Loading

Pull Request Test Coverage Report for Build 13535808664

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.7%) to 72.349%

Totals
Change from base Build 13523294333:	-0.7%
Covered Lines:	348
Relevant Lines:	481

💛 - Coveralls

Owner

briandfoy commented Feb 26, 2025

changes: reject patches where the range is not valid

briandfoy self-assigned this

briandfoy merged commit 1f404df into briandfoy:master

23 of 24 checks passed

briandfoy temporarily deployed to housekeeping

February 26, 2025 14:13

— with

GitHub Actions Inactive

github-actions bot added Status: accepted and removed Status: needs verification Priority: low labels

briandfoy added Status: released and removed Status: accepted labels

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Program: patch Status: released Type: bug Type: enhancement