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

Remove hardcoded IPs in HTML #158

Open
darsto opened this issue Sep 10, 2021 · 7 comments
Open

Remove hardcoded IPs in HTML #158

darsto opened this issue Sep 10, 2021 · 7 comments
Assignees
Labels
enhancement New feature or request wontfix This will not be worked on
Milestone

Comments

@darsto
Copy link

darsto commented Sep 10, 2021

I run into a problem where streamURL resolves to 0.0.0.0 and while the website works, the stream is not visible because it points to 0.0.0.0:81 . It's probably because the initial connection somewhat failed (?) but reconnected soon after. In the code I see the IP is set only at init and not changed after wifi reconnection, which is also error prone. I guess both issues could be fixed by removing the IP from HTML. The website uses javascript already, so it could use document.location.origin instead. Is there any reason why IPs/URLs were hardcoded in the first place?

If it's a good way to go I can try to make the changes and submit a PR.

I'm on current master, 7a29a31

@easytarget
Copy link
Owner

easytarget commented Sep 11, 2021

Good point about the URL's not being re-calculated on reconnect; I've fixed that.
However, passing the stream URL to the client, rather than calculating it from the document properties in JS is a deliberate change that I made, and won't be reverted. It allows the user to override the calculated stream URL in 'myconfig.h' to use a DNS name instead of IP address etc.

@easytarget easytarget self-assigned this Sep 11, 2021
@darsto
Copy link
Author

darsto commented Sep 11, 2021

Thanks! I still don't see why URLs need to be hardcoded into the HTML. IP and hostname is a device thing and doesn't need to be referenced in the served website whatsoever. Removing it from HTML would make the code smaller without removing any features.

document.location.origin is literally what you type into the address bar of your browser, regardless if it's a domain or IP address.

@easytarget
Copy link
Owner

Yep, thinking about it you are right. I really should be pulling the IP/domain from the document properties.

But, it wont reduce complexity much, I still want to allow configuring and passing the stream port number, and will need to code transposing that into the doc properties as needed. Also I currently use a simple substitution to drop it into some of the other pages as they are served; and will need to make them more complex to also use the document properties..

That will still be better than the current situation, so I'm reopening this.

@easytarget easytarget reopened this Sep 14, 2021
@easytarget easytarget added the enhancement New feature or request label Sep 14, 2021
@easytarget easytarget added this to the 4.x milestone Sep 14, 2021
@rdragonrydr
Copy link
Contributor

Is there some reason that relative addressing doesn't work? I have always used /config and equivalent as URLs in my webpages. That should still work even for ESP32 since it's computed in the browser, and it's much simpler to use. I know it works for ESP8266 and in general webpage stuff, but does the streaming mess it up or something?

@darsto
Copy link
Author

darsto commented Mar 9, 2022

@rdragonrydr The custom port. You can only specify it with absolute addresses.

@easytarget
Copy link
Owner

easytarget commented Mar 9, 2022

@rdragonrydr : within the main UI the links are relative, based on document.location.origin

The Issue here is that the stream is served on a different port than the main page. This port is user-configurable, by default it is port 81 but that is not guaranteed.

So; I pass the stream url to the main page as a string calculated by the server based on it's own address but with the correct port number; and this uses the IP address (it is not really hard-coded into the page, it just looks like it).

  • This works but looks a bit weird if you access the camera UI via a hostname rather than ip address.
  • It could break viewing the stream in the main UI page if you are embedding or proxying the stream port, but that is not a common use-case.

Anyway.. enough time wasted here: This is a future change;

  • Calculate stream url in the browser from document.location.origin, changing the port number as appropriate.
  • Allow the user to have a config with the main Interface on one URL, but the stream an a totally different URL
    eg: the stream on a well-known URL but the full UI on an secret one to protect the settings
  • The (near-redundant) URL_HOSTNAME config setting needs re-considering/using properly too.

Anyway... the current operation works; having the stream URL show as a IP address in the UI is sub-optimal but not really a problem for normal users.

@easytarget
Copy link
Owner

I am leaving this issue open for reference; although I am archiving the project

@easytarget easytarget added the wontfix This will not be worked on label Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants