search

netbox-community / netbox-topology-views

A netbox plugin that draws topology views
Apache License 2.0
751 stars 63 forks source link

Using BASE_PATH results in broken image URLs #247

Closed HolySephi closed 1 year ago

HolySephi commented 1 year ago

NetBox version

v3.4.2

Topology Views version

v3.2.1

Steps to Reproduce

Image URLs from plugin now are shown in HTML source as: <img src="test//test/static/netbox_topology_views/img/circuit.png" />

other navigation inside netbox is working correctly like: <a type="button" class="nav-link" href="/test/static/docs/" target="_blank">

Expected Behavior

Don't add BASE_PATH as prefix to the static folder because netbox already includes it so it doesn't get added twice.

Observed Behavior

BASE_PATH gets applied twice to image URLs

HolySephi commented 1 year ago

I think its this line - but i'm not entirely sure: https://github.com/mattieserver/netbox-topology-views/blob/6fafbeb7c369837997ff5f63d5dc53d0371fc5f2/netbox_topology_views/utils.py#L35

dreng commented 1 year ago

Seems to be realted to #178, but the other way round.

HolySephi commented 1 year ago

Yeah maybe - #178 is for hyperlinks that are generated. This issue here is for image links - other than with hyperlinks the image links are based on STATIC_URL to find the correct URL to the static files path - but that variable already includes the BASE_PATH

dreng commented 1 year ago

@HolySephi I don't know what's going on and I don't know when I'll get a chance to look at the disaster in detail. Base path is missing in #178 while it's being added twice in your scenario. It would be great if you could take a deep dive into the problem as a whole. I'm afraid it wouldn't help to just patch that one line you mentioned, because other things very likely brake instead.

Azmodeszer commented 1 year ago

Any news on this? I also had this bug with the image URLs being malformed and managed to fix it with some modifications to the utils file, and while images are now being displayed, I am unable to assign new ones to empty icon slots. It says "saved", but the modification doesn't persist.

dreng commented 1 year ago

I think its this line - but i'm not entirely sure:

https://github.com/mattieserver/netbox-topology-views/blob/6fafbeb7c369837997ff5f63d5dc53d0371fc5f2/netbox_topology_views/utils.py#L35

I don't think so. The variable url_path is used to strip BASE_PATH and STATIC_URL from the url variable. Unfortunately I am not able to reproduce this issue because I didn't manage to use NetBox with a BASE_PATH yet. It could be helpful to get some more information. Please provide detailed instructions how to install NetBox with BASE_PATH or provide a link to the point where it is documented. Futhermore you may provide not only your BASE_PATH but STATIC_URL and a corresponding database entry (use "python3 manage.py dbshell").

@Azmodeszer @HolySephi You both are welcome to take work on it in your environment. It's python, the code is open. Anything you change in the code can be used immediately. It's not rocket science.

Azmodeszer commented 1 year ago

I'm not a coder and I don't know a whole lot about Python, so it kinda is rocket science to me, I'm afraid. Was just curious what the status was and whether there was an obvious fix for the issue. Thanks for the work anyway, will be following this in case something comes up. :)

HolySephi commented 1 year ago

Reproduction is pretty easy - just install netbox to a subfolder like domain.tld/netbox and set BASE_PATH in netbox configuration.py to 'netbox/'

@dreng i'll fiddle around with the code - could you give me a hint to where the urls to images are generated if its not the line i posted? I think the only fix you need would just use STATIC_ROOT from netbox and NOT add BASE_PATH manually to this because netbox already adds BASE_PATH to STATIC_ROOT: https://github.com/netbox-community/netbox/blob/master/netbox/netbox/settings.py#LL438C35-L438C35

Your line of code would be: https://github.com/mattieserver/netbox-topology-views/blob/73490b6c64f8f09380c16779215b3a2534a6fc74/netbox_topology_views/utils.py#L32

dreng commented 1 year ago

Reproduction is not that easy, I'm afraid. First of all I use a development environment for debugging, which is different from to classic install method. Second, as in #178 described, it has got something to do with the webserver and it's document root. I don't use a webserver like NGINX or Apache. For development (and especially for debugging) I use the built in webserver that comes with django. But I think I see what you mean. Let's see what I can do.

The answer to your question about the point where the urls are generated depends. When you save the settings, an url is generated in order to store the path in the database. Maybe that's the culprit already. Another path is generated after reading from the database. That's why I asked for BASE_PATH, STATIC_URL and a corresponding database entry. Line 32 might be the one we need to change. But it may lead to other changes as well. Are you experiencing this issue on the topology page as well as on the images page or just one of them?

HolySephi commented 1 year ago

Yeah okay - its pretty easy if you compare/do a normal install. You don't have any extra BASE_PATH with the builtin development webserver of course. Here for example the BASE_PATH is 'demo/': Topology page: const brokenImage = 'demo//demo/static/netbox_topology_views/img/role-unknown.png'; Topology images: <img src="demo//demo/static/netbox_topology_views/img/wan-network.png" title="wan-network" width="64" data-role="ct1" data-image="demo//demo/static/netbox_topology_views/img/wan-network.png"/>

so its the same for both pages.

simply removing "settings.BASE_PATH + " from line 32 does show the images on both pages correctly for me

The next question would be - what is line 35 for? i don't need to change that in order to get working images - but its the same logic of adding BASE_PATH again to image urls?

dreng commented 1 year ago

As far as I understand the code, line 35 just prepares a string in order to strip that prefix before storing the path into the database. That makes sense because you wouldn't want to have a path in the database that's not universal.

I set up nginx in my dev environment and I am able to use BASE_PATH now. When I change line 32 like above the icons show up as expected but when I try to change an icon I get an error after clicking the save button. Can you confim this?

HolySephi commented 1 year ago

Yes, saving/changing icons is triggering an error