Project

General

Profile

Actions

Bug #9887

closed

Rule separator positions change when deleting multiple rules

Added by Jim Pingle over 4 years ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Low
Category:
Rules / NAT
Target version:
Start date:
11/07/2019
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
23.01
Release Notes:
Default
Affected Version:
All
Affected Architecture:
All

Description

When deleting rules around a separator at the end of the ruleset, separator positions can change unintentionally. Similar to #6801 but doesn't appear to be identical.

I was only able to reproduce it when removing the last few rules of an interface ruleset. If I deleted single rules or multiples around/between higher separator bars, there was no issue and things stayed in the expected places.

See attached before/after photos to see how to reproduce it.


Files

sep-test2-after.jpg (123 KB) sep-test2-after.jpg Test #2 - After Delete Jim Pingle, 11/07/2019 09:46 AM
sep-test2-before.jpg (157 KB) sep-test2-before.jpg Test #2 - Before Delete Jim Pingle, 11/07/2019 09:46 AM
sep-test1-before.jpg (159 KB) sep-test1-before.jpg Test #1 - Before Delete Jim Pingle, 11/07/2019 09:46 AM
sep-test1-after.jpg (137 KB) sep-test1-after.jpg Test #1 - After Delete Jim Pingle, 11/07/2019 09:46 AM
sep-test2-after.JPG (28.8 KB) sep-test2-after.JPG Dan Mackie, 12/17/2020 02:19 AM
sep-test2-before.jpg (37.9 KB) sep-test2-before.jpg Dan Mackie, 12/17/2020 02:19 AM
sep-test1-after.JPG (31.2 KB) sep-test1-after.JPG Dan Mackie, 12/17/2020 02:19 AM
sep-test1-before.jpg (38.1 KB) sep-test1-before.jpg Dan Mackie, 12/17/2020 02:19 AM
sep-test4-after.JPG (47.1 KB) sep-test4-after.JPG Dan Mackie, 12/17/2020 04:26 AM
sep-test4-before.JPG (57.5 KB) sep-test4-before.JPG Dan Mackie, 12/17/2020 04:26 AM
sep-test3-after.JPG (72.5 KB) sep-test3-after.JPG Test for #6801 - After Dan Mackie, 12/17/2020 04:26 AM
sep-test3-before.JPG (91.1 KB) sep-test3-before.JPG Test for #6801 - Before Dan Mackie, 12/17/2020 04:26 AM
sep-test2-after-830-fail.png (43.3 KB) sep-test2-after-830-fail.png Jim Pingle, 07/05/2022 09:41 AM
9887.patch (3 KB) 9887.patch Christopher Cope, 07/07/2022 12:18 PM
before.png (32.4 KB) before.png Lev Prokofev, 07/09/2022 01:23 AM
after.png (26.7 KB) after.png Lev Prokofev, 07/09/2022 01:23 AM
Actions #1

Updated by Anonymous over 3 years ago

  • Assignee set to Anonymous
Actions #2

Updated by Anonymous over 3 years ago

  • Priority changed from Normal to Low
Actions #3

Updated by Anonymous over 3 years ago

  • Target version changed from 2.5.0 to CE-Next
Actions #5

Updated by Dan Mackie over 3 years ago

PR: https://github.com/pfsense/pfsense/pull/4491

Fix allows both Jim Pingle's screenshots from this issue and those in #6801 to work as expected.

Note - If possible, delete the images from comment No. 4 above as they are before I realised I had not actually fixed the issue correctly.

Actions #6

Updated by Renato Botelho about 3 years ago

  • Status changed from New to Feedback

PR has been merged. Thanks!

Actions #7

Updated by Dan Mackie about 3 years ago

  • % Done changed from 0 to 100
Actions #8

Updated by Jim Pingle almost 3 years ago

  • Target version changed from CE-Next to 2.6.0
Actions #9

Updated by Jim Pingle almost 3 years ago

  • Plus Target Version set to 21.05
Actions #10

Updated by Jim Pingle almost 3 years ago

Already in 21.05 branch.

Actions #11

Updated by Jim Pingle almost 3 years ago

  • Status changed from Feedback to New
  • % Done changed from 100 to 50
  • Plus Target Version changed from 21.05 to 21.09

Still broken but not a blocker so moving forward. The scenario in my first test "sep-test1" is OK. The second scenario "sep-test2" still breaks. The behavior is different, however, as now the second separator "Sep 2" has moved to the top of the interface rules instead of staying at the bottom.

Actions #12

Updated by Jim Pingle over 2 years ago

  • Plus Target Version changed from 21.09 to 22.01

Moving ahead

Actions #13

Updated by Jim Pingle over 2 years ago

  • Target version changed from 2.6.0 to CE-Next
  • Plus Target Version changed from 22.01 to 22.05
Actions #14

Updated by Jim Pingle almost 2 years ago

  • Plus Target Version changed from 22.05 to 22.09
Actions #15

Updated by Jim Pingle over 1 year ago

  • Plus Target Version changed from 22.09 to 22.11
Actions #16

Updated by Christopher Cope over 1 year ago

  • File 0001-Separators-position-fix.-Issue-9887.patch added
  • Status changed from New to Pull Request Review

https://gitlab.netgate.com/pfSense/pfSense/-/merge_requests/830

All tests in the original ticket worked as expected with these changes. I'd appreciate an extra pair of eyes, though.

Actions #17

Updated by Jim Pingle over 1 year ago

This fails in a new/different way when applied. When attempting "test 2" from my original attachments, it puts the separators into an even different (and wrong) layout.

Looking at my original attachments, "sep-test1" worked even before this patch now, but "sep-test2" failed in the way shown in the new image attached.

Actions #18

Updated by Christopher Cope over 1 year ago

Here's a new patch with missing fixes. Seems to pass all tests this time.

Actions #19

Updated by Christopher Cope over 1 year ago

  • File deleted (0001-Separators-position-fix.-Issue-9887.patch)
Actions #20

Updated by Jim Pingle over 1 year ago

  • Assignee set to Christopher Cope
  • Target version changed from CE-Next to 2.7.0
  • % Done changed from 50 to 100

Latest patch tests OK for me.

Actions #21

Updated by Christopher Cope over 1 year ago

  • Status changed from Pull Request Review to Feedback

Fix merged

Actions #22

Updated by Lev Prokofev over 1 year ago

Tested, and it works for me.

Actions #23

Updated by Jim Pingle over 1 year ago

  • Status changed from Feedback to Resolved

Looks good on the latest snapshot.

Actions #24

Updated by Jim Pingle over 1 year ago

  • Plus Target Version changed from 22.11 to 23.01
Actions

Also available in: Atom PDF