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

miniupnpd: Increase port map description length limit to 255 #746

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

Conversation

Self-Hosting-Group
Copy link
Contributor

@Self-Hosting-Group Self-Hosting-Group commented May 28, 2024

  • Same limit as the original Linksys implementation
  • Remove the currently inconsistent length limit of 76 for GetGenericPortMappingEntry/GetSpecificPortMappingEntry and 64 for GetListOfPortMappings (IGDv2)
  • No longer truncate description to 127 and 89 on daemon restarts, with lease file
  • When using pf firewall backend, 63 (pf label) limit remains

The maximum length is not defined in the UPnP IGD standards. The built-in Windows client allows any description length (>20k tested). The PCP description option standard (not implemented) has a length limit of 1016. Other limits: iptables/nftables comments are limited to 256/128 (not used).

@miniupnp miniupnp self-assigned this May 29, 2024
@miniupnp
Copy link
Owner

miniupnp commented Jun 4, 2024

miniupnpd was first developped for pf and use the pf rule "label" which is limited to 64 bytes to store the description.

Copy link
Owner

@miniupnp miniupnp left a comment

Choose a reason for hiding this comment

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

This PR completely misses the fact that 64 characters is the hard limit in miniupnpd at least with pf backend.

@Self-Hosting-Group Self-Hosting-Group force-pushed the description-length branch 2 times, most recently from 07de68b to 3cb5571 Compare June 11, 2024 07:27
@miniupnp
Copy link
Owner

In fact the limit should be removed from miniupnpc (API needs to be changed)
but miniupnpd is limited to 64 chars

@Self-Hosting-Group Self-Hosting-Group marked this pull request as draft November 15, 2024 16:16
@Self-Hosting-Group Self-Hosting-Group changed the title Standardisation of port map description length to 1016 miniupnpd: Increase port map description length limit to 255 Jan 6, 2025
Self-Hosting-Group added a commit to Self-Hosting-Group/miniupnp that referenced this pull request Jan 9, 2025
@Self-Hosting-Group Self-Hosting-Group marked this pull request as ready for review January 9, 2025 12:31
Self-Hosting-Group referenced this pull request Mar 3, 2025
The changes are minor, no client code should be using
what's removed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants