Remove absolute URIs from settings page#1261
Conversation
| request = self.context.get("request") | ||
| favicon_file_path = static(obj.favicon_file) | ||
| favicon_url = request.build_absolute_uri(favicon_file_path) | ||
| favicon_url = static(obj.favicon_file) |
There was a problem hiding this comment.
There's two issues here:
- This will result in a relative URL that on its own will not resolve to the favicon, which makes it less useful.
- It's a breaking change. Any client that displays fav icons would now have to be changed to prepend some base URL. That alone makes it a no go for me.
I'm not sure what a proper fix would be but it would probably involve passing a specific header to linkding or configuring some environment variable. Django uses this to determine the host name, so maybe that gives some pointers: https://github.com/django/django/blob/d6ae2ed868e43671afc4d433c3d8f4d27f7eb555/django/http/request.py#L167-L183
There was a problem hiding this comment.
That makes sense to me. I missed that these URLs were used for the API and not for HTML templates. I can take a look at the django options. https://docs.djangoproject.com/en/6.0/ref/settings/#std-setting-USE_X_FORWARDED_HOST seems promising.
It seems like the convention here has been to add a linkding specific env var that maps to a django config option. Would exposing this option as something like LD_USE_X_FORWARDED_HOST make sense?
There was a problem hiding this comment.
I guess adding a setting for this would be fine. For now it would be good to have confirmation that this would actually solve this issue though.
The change in the settings page looks good to me.
|
API issues aside, is the proposed solution for the settings page acceptable? I understand the need for a full URI in an API response, but for an HTML page having an absolute path withing scheme/hostname would match what is done in most other parts of the app. |
packrat386
force-pushed
the
no_absolute_uris
branch
from
03f739d to
6e87452
Compare
The rest of the links on this page are absolute paths without a specified hostname, but these in particlar use build_absolute_uri. I am running linkding behind two different load balancers which makes these links bubble up the "internal" hostname instead of the hostname I actually got to the page from.
packrat386
force-pushed
the
no_absolute_uris
branch
from
6e87452 to
5c71dd2
Compare
|
Confirmed the new config option works to override the hostname returned by the API for static URLs like favicons. |
sissbruecker
changed the title
2 participants
The rest of the links on this page are absolute paths without a specified hostname, but these in particlar use
build_absolute_uri. I am running linkding behind two different load balancers which makes these links bubble up the "internal" hostname instead of the hostname I actually got to the page from.