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

net/udpbroadcastrelay: Add options for MSEARCH and Allow/Block CIDR #4169

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ZephireNZ
Copy link

v1.1 of udpbroadcastrelay includes additional parameters like filtering by CIDR and MSEARCH parameters

This adds these so they can be configured via the UI.

I have created a bug to update the upstream port to 1.1: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=280723

@ZephireNZ
Copy link
Author

@fichtner fichtner self-assigned this Aug 27, 2024
@fichtner
Copy link
Member

Nice work, the update came in via opnsense/ports@ed6789626b89b so it will be available on 24.7.3. Let me take a look at the PR now :)

<id>udpbroadcastrelay.msearch</id>
<label>M-SEARCH processing</label>
<type>text</type>
<help><![CDATA[A space-separated list of M-SEARCH processing actions, use a comma before search terms for example <code>block,search-term</code><br/>Please refer to the <a href="https://github.com/marjohn56/udpbroadcastrelay#usage">UDP Broadcast Relay documentation</a> for more information]]></help>
Copy link
Member

Choose a reason for hiding this comment

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

HTML in these parts just ends up bouncing the help text from translations because the translation could break the HTML due to typos which are hard to spot and debug. We don't have a good solution for this.

Copy link
Author

Choose a reason for hiding this comment

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

Did you want me to remove the link to the documentation? Otherwise not sure what I am meant to do for this one

<type>select_multiple</type>
<allownew>true</allownew>
<help><![CDATA[Only allow packets from a range of IP source addresses]]></help>
<advanced>true</advanced>
Copy link
Member

Choose a reason for hiding this comment

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

I see all of these are advanced. I don't mind, but it looks interesting releasing a 1.1 and not offering new features at first glance :)

Copy link
Author

Choose a reason for hiding this comment

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

MSearch definitely should be advanced feature (given the specific sytax requirements and function).

Maybe Allow/Block CIDR could be non-advanced?

<msearch type="TextField">
<default></default>
<Required>N</Required>
<mask>/^(?:(?:block|fwd|proxy|dial)(?:,\S+|) )*(?:block|fwd|proxy|dial)(?:,\S+|)$/</mask>
Copy link
Member

Choose a reason for hiding this comment

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

this is probably as fragile as it gets with CSV and freestyle syntax matched via regex. :(

Copy link
Author

@ZephireNZ ZephireNZ Aug 31, 2024

Choose a reason for hiding this comment

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

Yeah... is quite the ugly one. It could just be free text with no restriction, but not sure that's any better as people will wonder why its not working 😅

I did space separated as it needed to be split into separate args, is there a better input type to use in the UI for that?

@fichtner
Copy link
Member

Do you know if 1.1 is compatible with the current version this plugin. I suppose yes?

Also there is a pkg-descr file that you can use to add a changelog. That would be better for tracking changes through release notes as I can point to it.

@ZephireNZ
Copy link
Author

Do you know if 1.1 is compatible with the current version this plugin. I suppose yes?

As far as I can tell, yes it should be. The only break change (removing -s 1.1.1.3) was added and then removed in between the releases, so nobody on 0.3-beta would have been using it.

Also there is a pkg-descr file that you can use to add a changelog. That would be better for tracking changes through release notes as I can point to it.

Have updated this, let me know if that's what you were wanting 😊

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

Successfully merging this pull request may close these issues.

2 participants