[PR #1036] [MERGED] Allows hostname instead of ip for streams #3341

Closed
opened 2026-02-26 07:38:56 +03:00 by kerem · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/NginxProxyManager/nginx-proxy-manager/pull/1036
Author: @BjoernAkAManf
Created: 4/23/2021
Status: Merged
Merged: 8/16/2021
Merged by: @jc21

Base: developHead: master


📝 Commits (3)

  • 1a64d44 Merge pull request #955 from jc21/develop
  • 389fd15 allows hostname instead of ip for streams
  • ba7bb57 Incorporate feedback

📊 Changes

10 files changed (+71 additions, -25 deletions)

View changed files

backend/migrations/20210423103500_stream_domain.js (+40 -0)
📝 backend/schema/endpoints/streams.json (+21 -10)
📝 backend/templates/stream.conf (+2 -2)
📝 docker/rootfs/etc/services.d/frontend/run (+1 -0)
📝 docker/rootfs/etc/services.d/manager/run (+1 -0)
📝 frontend/js/app/nginx/stream/form.ejs (+2 -2)
📝 frontend/js/app/nginx/stream/form.js (+1 -8)
📝 frontend/js/app/nginx/stream/list/item.ejs (+1 -1)
📝 frontend/js/i18n/messages.json (+1 -1)
📝 frontend/js/models/stream.js (+1 -1)

📄 Description

I see no Benefit to reduce streams to ip addresses. In particular if running NPM inside docker containers.
This causes a duality in management between proxied http hosts and streamed protocols (e.g. ssh).

Once merged it fixes #44 .

Note: This implementation is a bit rough around the edges. In particular it currently allows input such as host name "example.com/meow" and Port "22". This is obviously not ideal.
As i've been struggeling with the source code a little (e.g. getting the dev environment to work). I wanted to get a bit feedback about this change first. As far as i can see it we should just allow any domain (example.com, test.example.com) and disallow usage of the path component for this to work correctly right? (example.com/meow:22 is obviously wrong, but even example.com:22/meow would not mean anything).

PS: Any reason for the files "fonts" and "images"? Right now theres a need to manually create a sym link to the location specified in that file. Would'nt it make sense to just replace the references to that file in the source code itself?

PPS: I noticed there is no real information about contributing to the Project. Would you mind adding some information to the readme or a dedicated file?

PPPS: Would you be interested in a PR decoupling the authentication of NPM? For example it would be a great feature if the Admin UI could be secured using OIDC itself. The only information to OIDC i could find in the repository right now is unfortunately only for protecting proxied services themselves.

Nethertheless thanks for the great product.


🔄 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/NginxProxyManager/nginx-proxy-manager/pull/1036 **Author:** [@BjoernAkAManf](https://github.com/BjoernAkAManf) **Created:** 4/23/2021 **Status:** ✅ Merged **Merged:** 8/16/2021 **Merged by:** [@jc21](https://github.com/jc21) **Base:** `develop` ← **Head:** `master` --- ### 📝 Commits (3) - [`1a64d44`](https://github.com/NginxProxyManager/nginx-proxy-manager/commit/1a64d44857b8db488f56567b743f787b61e1f7a4) Merge pull request #955 from jc21/develop - [`389fd15`](https://github.com/NginxProxyManager/nginx-proxy-manager/commit/389fd158ad2b41375c7187766ed9949c91db2fd7) allows hostname instead of ip for streams - [`ba7bb57`](https://github.com/NginxProxyManager/nginx-proxy-manager/commit/ba7bb57ca257e570de3ff6d341e220b846a7e8ac) Incorporate feedback ### 📊 Changes **10 files changed** (+71 additions, -25 deletions) <details> <summary>View changed files</summary> ➕ `backend/migrations/20210423103500_stream_domain.js` (+40 -0) 📝 `backend/schema/endpoints/streams.json` (+21 -10) 📝 `backend/templates/stream.conf` (+2 -2) 📝 `docker/rootfs/etc/services.d/frontend/run` (+1 -0) 📝 `docker/rootfs/etc/services.d/manager/run` (+1 -0) 📝 `frontend/js/app/nginx/stream/form.ejs` (+2 -2) 📝 `frontend/js/app/nginx/stream/form.js` (+1 -8) 📝 `frontend/js/app/nginx/stream/list/item.ejs` (+1 -1) 📝 `frontend/js/i18n/messages.json` (+1 -1) 📝 `frontend/js/models/stream.js` (+1 -1) </details> ### 📄 Description I see no Benefit to reduce streams to ip addresses. In particular if running NPM inside docker containers. This causes a duality in management between proxied http hosts and streamed protocols (e.g. ssh). Once merged it fixes #44 . Note: This implementation is a bit rough around the edges. In particular it currently allows input such as host name "example.com/meow" and Port "22". This is obviously not ideal. As i've been struggeling with the source code a little (e.g. getting the dev environment to work). I wanted to get a bit feedback about this change first. As far as i can see it we should just allow any domain (example.com, test.example.com) and disallow usage of the path component for this to work correctly right? (example.com/meow:22 is obviously wrong, but even example.com:22/meow would not mean anything). PS: Any reason for the files "fonts" and "images"? Right now theres a need to manually create a sym link to the location specified in that file. Would'nt it make sense to just replace the references to that file in the source code itself? PPS: I noticed there is no real information about contributing to the Project. Would you mind adding some information to the readme or a dedicated file? PPPS: Would you be interested in a PR decoupling the authentication of NPM? For example it would be a great feature if the Admin UI could be secured using OIDC itself. The only information to OIDC i could find in the repository right now is unfortunately only for protecting proxied services themselves. Nethertheless thanks for the great product. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-02-26 07:38:56 +03:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/nginx-proxy-manager-NginxProxyManager#3341
No description provided.