[PR #3383] [CLOSED] WIP: Fix for UPNP port-forwarding issue #3339 #9494

Closed
opened 2026-02-07 06:03:09 +03:00 by OVERLORD · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/jellyfin/jellyfin/pull/3383
Author: @BaronGreenback
Created: 6/19/2020
Status: Closed

Base: masterHead: Mono


📝 Commits (9)

  • 31a221f Imported Mono.Nat
  • 5ba6e33 DeviceLost, FinishDiscovery, BeginDiscovery implemented.
  • f0bc8d0 Created initial version of InternetChecker.cs which checks if we we have internet access.
  • d6fc087 Packages updated
  • 503cfd0 Fixed issue in Mono, and put additional locks on portforwarding
  • d741380 Bug fix
  • 6c43e32 Implimented Ping monitoring of the gateway routers in an attempt to ensure inbound access is always available.
  • 8eddec5 Fixes issue outlined here.
  • a5e77b6 Fixed resolve.

📊 Changes

60 files changed (+5160 additions, -89 deletions)

View changed files

Common/Networking/Common.Networking.csproj (+44 -0)
Common/Networking/InternetAccess/GatewayEventArgs.cs (+33 -0)
Common/Networking/InternetAccess/GatewayMonitor.cs (+230 -0)
Common/Networking/InternetAccess/IGatewayMonitor.cs (+29 -0)
Common/Networking/InternetAccess/Properties/AssemblyInfo.cs (+36 -0)
📝 Emby.Server.Implementations/ApplicationHost.cs (+3 -0)
📝 Emby.Server.Implementations/Emby.Server.Implementations.csproj (+4 -3)
📝 Emby.Server.Implementations/EntryPoints/ExternalPortForwarding.cs (+184 -58)
📝 Emby.Server.Implementations/ScheduledTasks/ScheduledTaskWorker.cs (+14 -9)
📝 Jellyfin.Server/Properties/launchSettings.json (+4 -3)
📝 MediaBrowser.Model/Configuration/ServerConfiguration.cs (+7 -0)
📝 MediaBrowser.sln (+34 -15)
Mono.Nat/AssemblyInfo.cs (+8 -0)
Mono.Nat/AsyncExtensions.cs (+131 -0)
Mono.Nat/AsyncResults/TaskAsyncResult.cs (+96 -0)
Mono.Nat/Copyright.txt (+23 -0)
Mono.Nat/Enums/NatProtocol.cs (+42 -0)
Mono.Nat/Enums/ProtocolType.cs (+42 -0)
Mono.Nat/EventArgs/DeviceEventArgs.cs (+48 -0)
Mono.Nat/Exceptions/MappingException.cs (+116 -0)

...and 40 more files

📄 Description

Author has changed Mono.NAT in light of the issues found. Am currently testing that out.

Fixes

Bug 1) If pnp forwarding is disabled, Mono keeps the device list and udp sockets. (Memory should ideally be freed up).
Fix: The implementation of Mono included is no longer a singleton, and clears memory/ closes sockets when pnp forwarding is disabled.

Bug 2: Security wise, if UPNP is disabled, the port forwarding mappings should be removed, but the RaiseDeviceLost event in mono isn't attached to anything.
Fix: As it's now an instance, the RaiseDeviceLost method is now triggered once pnp-forwarding is disabled, and an attempt is made to remove the ports on the router..

Bug3: If something happens to the router (ie. reboot), then as the device stays in the mono's device list, it won't be re-discovered, so there is no way of remapping ports. A restart of jellyfin will be required to get it working again.
Fix: Have linked Mono to the network change event. A change in the network also stops, clears the devices and re-starts the UPNP scan. This means that if it was a router reboot, Jellyfin will pick up the device again, and re-insert the port-forwarding.
In addition, every router scanned is now added to a monitor that will ping it every 30 seconds (configurable in the options). If it doesn't reply to pings for some reason, it triggers a gateway down event, which in turn - restarts the port-forwarding.

Bug 4: If there are any network changes after Jellyfin starts - the mono class will never reflect these.
Fix: Linked into the config change event, which restarts the port-forwarding.

Changes to Mono.NAT

  • instantiated instead of singleton.
  • logs to an Ilogger - so it can be debugged.
  • RaiseDeviceLost event now working.
    FIX: - It now broadcasts for "urn:schemas-upnp-org:device:InternetGatewayDevice"
    Devices with dual functionality (eg wifi-routers) sometimes only respond to ST: ssdp-all with one view of their device. In my case, my router returned "urn:schemas-wifialliance-org:device:WFADevice:1" meaning PNP-forwarding didn't work. It does now.

Changes to Jellyfin

Removed the _deviceDiscovery.DeviceDiscovered event, as i can only assume this was implemented as Mono wasn't detecting devices like mine.

Removed the multi-fire code (ExternalPortforwarding.cs) as we should now be able to identify the cause, and duplication checking is already present in the Mono code.

Mono.NAT changes

NatUtility.cs

  • Changes to an instantiated class.
  • removed static logging methods

Searcher.cs

  • Instantiated the object
  • Implemented ILogger
  • Linked RaiseDeviceLost event into the Stop method.

UPNPSearcher.cs

  • Instantiated the object
  • Removed initiation code from the constructor and into the Initiation method.
  • ServicesReceived: Added new detection code.
  • HandleMessageReceived: Added more detection code from SOAP responses.
  • SearchAsync - now sends out new SSDP detection message.

PNPSearcher.cs

  • Instantiated the object
  • Removed initiation code from the constructor and into the Initiation method.

DiscoverDeviceMessage.cs

  • Added methods, EncodeSSDPEmpty and EncodeUnicastEmpty

ResponseMessages.cs

  • Decode: Added more detection code from SOAP responses.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/jellyfin/jellyfin/pull/3383 **Author:** [@BaronGreenback](https://github.com/BaronGreenback) **Created:** 6/19/2020 **Status:** ❌ Closed **Base:** `master` ← **Head:** `Mono` --- ### 📝 Commits (9) - [`31a221f`](https://github.com/jellyfin/jellyfin/commit/31a221fa60a0e77c4f64c6c07290b91e9e08b434) Imported Mono.Nat - [`5ba6e33`](https://github.com/jellyfin/jellyfin/commit/5ba6e33096db6920a6672ccc98df2e0061691558) DeviceLost, FinishDiscovery, BeginDiscovery implemented. - [`f0bc8d0`](https://github.com/jellyfin/jellyfin/commit/f0bc8d0b74fd93d47f06117bb7376eed494d3c77) Created initial version of InternetChecker.cs which checks if we we have internet access. - [`d6fc087`](https://github.com/jellyfin/jellyfin/commit/d6fc0875438e7462763fbae5d583adf4d7bb4864) Packages updated - [`503cfd0`](https://github.com/jellyfin/jellyfin/commit/503cfd0c740da9d784e47fb4a551004f9bc0b454) Fixed issue in Mono, and put additional locks on portforwarding - [`d741380`](https://github.com/jellyfin/jellyfin/commit/d7413801b6c2cbae49b8f3da3086e5050acb4f79) Bug fix - [`6c43e32`](https://github.com/jellyfin/jellyfin/commit/6c43e320f36f633ff123b11e405b3986f50075a1) Implimented Ping monitoring of the gateway routers in an attempt to ensure inbound access is always available. - [`8eddec5`](https://github.com/jellyfin/jellyfin/commit/8eddec5b9871afbb5a9e1c2d28e0447da1dbed7d) Fixes issue outlined here. - [`a5e77b6`](https://github.com/jellyfin/jellyfin/commit/a5e77b64efbca768edb0b1eb683508f28e22effe) Fixed resolve. ### 📊 Changes **60 files changed** (+5160 additions, -89 deletions) <details> <summary>View changed files</summary> ➕ `Common/Networking/Common.Networking.csproj` (+44 -0) ➕ `Common/Networking/InternetAccess/GatewayEventArgs.cs` (+33 -0) ➕ `Common/Networking/InternetAccess/GatewayMonitor.cs` (+230 -0) ➕ `Common/Networking/InternetAccess/IGatewayMonitor.cs` (+29 -0) ➕ `Common/Networking/InternetAccess/Properties/AssemblyInfo.cs` (+36 -0) 📝 `Emby.Server.Implementations/ApplicationHost.cs` (+3 -0) 📝 `Emby.Server.Implementations/Emby.Server.Implementations.csproj` (+4 -3) 📝 `Emby.Server.Implementations/EntryPoints/ExternalPortForwarding.cs` (+184 -58) 📝 `Emby.Server.Implementations/ScheduledTasks/ScheduledTaskWorker.cs` (+14 -9) 📝 `Jellyfin.Server/Properties/launchSettings.json` (+4 -3) 📝 `MediaBrowser.Model/Configuration/ServerConfiguration.cs` (+7 -0) 📝 `MediaBrowser.sln` (+34 -15) ➕ `Mono.Nat/AssemblyInfo.cs` (+8 -0) ➕ `Mono.Nat/AsyncExtensions.cs` (+131 -0) ➕ `Mono.Nat/AsyncResults/TaskAsyncResult.cs` (+96 -0) ➕ `Mono.Nat/Copyright.txt` (+23 -0) ➕ `Mono.Nat/Enums/NatProtocol.cs` (+42 -0) ➕ `Mono.Nat/Enums/ProtocolType.cs` (+42 -0) ➕ `Mono.Nat/EventArgs/DeviceEventArgs.cs` (+48 -0) ➕ `Mono.Nat/Exceptions/MappingException.cs` (+116 -0) _...and 40 more files_ </details> ### 📄 Description Author has changed Mono.NAT in light of the issues found. Am currently testing that out. Fixes ===== Bug 1) If pnp forwarding is disabled, Mono keeps the device list and udp sockets. (Memory should ideally be freed up). Fix: The implementation of Mono included is no longer a singleton, and clears memory/ closes sockets when pnp forwarding is disabled. Bug 2: Security wise, if UPNP is disabled, the port forwarding mappings should be removed, but the RaiseDeviceLost event in mono isn't attached to anything. Fix: As it's now an instance, the RaiseDeviceLost method is now triggered once pnp-forwarding is disabled, and an attempt is made to remove the ports on the router.. Bug3: If something happens to the router (ie. reboot), then as the device stays in the mono's device list, it won't be re-discovered, so there is no way of remapping ports. A restart of jellyfin will be required to get it working again. Fix: Have linked Mono to the network change event. A change in the network also stops, clears the devices and re-starts the UPNP scan. This means that if it was a router reboot, Jellyfin will pick up the device again, and re-insert the port-forwarding. In addition, every router scanned is now added to a monitor that will ping it every 30 seconds (configurable in the options). If it doesn't reply to pings for some reason, it triggers a gateway down event, which in turn - restarts the port-forwarding. Bug 4: If there are any network changes after Jellyfin starts - the mono class will never reflect these. Fix: Linked into the config change event, which restarts the port-forwarding. Changes to Mono.NAT ============ - instantiated instead of singleton. - logs to an Ilogger - so it can be debugged. - RaiseDeviceLost event now working. FIX: - It now broadcasts for "urn:schemas-upnp-org:device:InternetGatewayDevice" Devices with dual functionality (eg wifi-routers) sometimes only respond to ST: ssdp-all with one view of their device. In my case, my router returned "urn:schemas-wifialliance-org:device:WFADevice:1" meaning PNP-forwarding didn't work. It does now. Changes to Jellyfin ============ Removed the _deviceDiscovery.DeviceDiscovered event, as i can only assume this was implemented as Mono wasn't detecting devices like mine. Removed the multi-fire code (ExternalPortforwarding.cs) as we should now be able to identify the cause, and duplication checking is already present in the Mono code. Mono.NAT changes ============= NatUtility.cs - Changes to an instantiated class. - removed static logging methods Searcher.cs - Instantiated the object - Implemented ILogger - Linked RaiseDeviceLost event into the Stop method. UPNPSearcher.cs - Instantiated the object - Removed initiation code from the constructor and into the Initiation method. - ServicesReceived: Added new detection code. - HandleMessageReceived: Added more detection code from SOAP responses. - SearchAsync - now sends out new SSDP detection message. PNPSearcher.cs - Instantiated the object - Removed initiation code from the constructor and into the Initiation method. DiscoverDeviceMessage.cs - Added methods, EncodeSSDPEmpty and EncodeUnicastEmpty ResponseMessages.cs - Decode: Added more detection code from SOAP responses. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
OVERLORD added the pull-request label 2026-02-07 06:03:09 +03:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/jellyfin#9494