Skip to content

Conversation

@maurice2k
Copy link

Previously, the logic for resetting push options (like 'route') was based on update_options_found which was local to apply_push_options. This meant that if a PUSH_UPDATE was split across multiple continuation messages, the state was lost, causing routes to be reset multiple times (once per message chunk) rather than once per update sequence.

This patch moves the state tracking to struct options as push_update_options_found, allowing it to persist across the entire PUSH_UPDATE sequence.

This fixes an issue where large route lists sent via PUSH_UPDATE would result in only the last chunk's routes being applied, or previous routes being continuously deleted and re-added.

Added unit test test_incoming_push_continuation_route_accumulation to verify the fix.

Previously, the logic for resetting push options (like 'route') was based on
`update_options_found` which was local to `apply_push_options`. This meant
that if a PUSH_UPDATE was split across multiple continuation messages,
the state was lost, causing routes to be reset multiple times (once per
message chunk) rather than once per update sequence.

This patch moves the state tracking to `struct options` as
`push_update_options_found`, allowing it to persist across the entire
PUSH_UPDATE sequence.

This fixes an issue where large route lists sent via PUSH_UPDATE would
result in only the last chunk's routes being applied, or previous routes
being continuously deleted and re-added.

Added unit test `test_incoming_push_continuation_route_accumulation` to
verify the fix.
@maurice2k
Copy link
Author

Copy link
Contributor

@mrbff mrbff left a comment

Choose a reason for hiding this comment

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

lgtm

cron2 pushed a commit to cron2/openvpn that referenced this pull request Dec 10, 2025
OpenVPN/openvpn#925

Previously, the logic for resetting push options (like 'route') was based on
`update_options_found` which was local to `apply_push_options`. This meant
that if a PUSH_UPDATE was split across multiple continuation messages,
the state was lost, causing routes to be reset multiple times (once per
message chunk) rather than once per update sequence.

This patch moves the state tracking to `struct options` as
`push_update_options_found`, allowing it to persist across the entire
PUSH_UPDATE sequence.

This fixes an issue where large route lists sent via PUSH_UPDATE would
result in only the last chunk's routes being applied, or previous routes
being continuously deleted and re-added.

Added unit test `test_incoming_push_continuation_route_accumulation` to
verify the fix.
Acked-by: Marco Baffo <[email protected]>
Message-Id: <CAM8w-qEE6vHj=yUOpTFbM7DqPKzUV0NupvEG4rUefY=kNB2DxQ@mail.gmail.com>
URL: https://www.mail-archive.com/[email protected]/msg34814.html

Signed-off-by: Gert Doering <[email protected]>
cron2 pushed a commit that referenced this pull request Dec 10, 2025
Previously, the logic for resetting push options (like 'route') was based on
`update_options_found` which was local to `apply_push_options`. This meant
that if a PUSH_UPDATE was split across multiple continuation messages,
the state was lost, causing routes to be reset multiple times (once per
message chunk) rather than once per update sequence.

This patch moves the state tracking to `struct options` as
`push_update_options_found`, allowing it to persist across the entire
PUSH_UPDATE sequence.

This fixes an issue where large route lists sent via PUSH_UPDATE would
result in only the last chunk's routes being applied, or previous routes
being continuously deleted and re-added.

Added unit test `test_incoming_push_continuation_route_accumulation` to
verify the fix.

Github: #925

Signed-off-by: Moritz Fain <[email protected]>
Acked-by: Marco Baffo <[email protected]>
Message-Id: <CAM8w-qEE6vHj=yUOpTFbM7DqPKzUV0NupvEG4rUefY=kNB2DxQ@mail.gmail.com>
URL: https://www.mail-archive.com/[email protected]/msg34814.html

Signed-off-by: Gert Doering <[email protected]>
@cron2 cron2 closed this Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants