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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rlaager
Copy link
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
When no MD5 password is set, there should not be extra whitespace.
@barryo barryo self-requested a review May 29, 2023 08:18
@rlaager
Copy link
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
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.

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