Skip to content

[BF] bird2: Remove stray whitespace when no MD5#852

Closed
rlaager wants to merge 1 commit intoinex:mainfrom
rlaager:md5-password-formatting
Closed

[BF] bird2: Remove stray whitespace when no MD5#852
rlaager wants to merge 1 commit intoinex:mainfrom
rlaager:md5-password-formatting

Conversation

@rlaager
Copy link
Copy Markdown
Contributor

@rlaager rlaager commented May 27, 2023

[BF] bird2: Remove stray whitespace when no MD5

When no MD5 password is set, there should not be extra whitespace.

This is a minor thing right now, but its something that intersects with other changes I made for BFD.

Please verify the output before merging. This works on my system, but I have another line added after it in my skin. Also, I do not run a route collector, so the change there has not been tested.

In addition to the above, I have:

  • ensured all relevant template output is escaped to avoid XSS attached with <?= $t->ee( $data ) ?> or equivalent.
  • ensured appropriate checks against user privilege / resources accessed
  • API calls (particular for add/edit/delete/toggle) are not implemented with GET and use CSRF tokens to avoid CSRF attacks

@rlaager rlaager mentioned this pull request May 27, 2023
8 tasks
@rlaager rlaager force-pushed the md5-password-formatting branch from 2990ea6 to 2f856a0 Compare May 28, 2023 17:30
When no MD5 password is set, there should not be extra whitespace.
@rlaager rlaager force-pushed the md5-password-formatting branch from 2f856a0 to 2367c7d Compare May 28, 2023 17:35
@barryo barryo self-requested a review May 29, 2023 08:18
@rlaager
Copy link
Copy Markdown
Contributor Author

rlaager commented May 29, 2023

BTW, even if you don't care about BFD upstream, I would still appreciate if this change could be merged, as it makes it easier for me to skin in BFD.

@barryo
Copy link
Copy Markdown
Member

barryo commented May 30, 2023

@rlaager - yes, I'll look at merging this. We're super sensitive about anything that touches router configuration and especially route server configuration as those are production critical services.

This means generally that merges have to be covered by the test database and by the sample known good configs; and that they have to apply cleanly without interrupting BGP sessions.

At a glance, the no-md5 use case is either not covered by the configs / test cases and so I'll need to add that in.

@barryo
Copy link
Copy Markdown
Member

barryo commented Feb 16, 2026

Templates have been updated since this PR was submitted.

@barryo barryo closed this Feb 16, 2026
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.

2 participants