Open roughnecks opened 10 months ago
I'm having the same issue, even when installing from master
, unrelated to Shaarli (maybe we should update the issue title?).
The emails I get to moderate comments are missing a slash between the domain name (host
) and the identifier/post.
For example, for the post/identifier name-of-the-post
I would get:
Anonymous wrote:
Here is a nice comment.
IP address: 111.111.111.111
Link to comment: https://example.comname-of-the-post#isso-7
Likewise, when clicking on the Delete/Activate links, after confirming the action, I am redirected to the same broken URL.
I tried setting a trailing slash on the host
, but that changes nothing.
I think I found the problematic code:
It concatenates local("origin")
directly with thread["uri"]
without checking for a trailing or leading slash, which could result in URLs lacking the separating slash between the domain and the path. The same issue could manifest anywhere this pattern of URL construction is used in the codebase.
To fix it, we could ensure there is a separating slash between local("origin")
and thread["uri"]
, like this:
origin = local("origin")
uri = thread["uri"]
if not origin.endswith("/") and not uri.startswith("/"):
uri = "/" + uri
link = origin + uri + "#isso-%i" % item["id"]
This checks whether a trailing slash exists in origin
or a leading slash exists in uri
and adds one if neither is present, thereby avoiding the malformed URL issue.
EDIT: It might be relevant that I'm manually setting the data-isso-id
and data-title
.
@welpo your suggestion seems valid, but we should fix the underlying issue.
local("origin")
is set from the [general]
>
host
config setting:
We're recommending people to use a trailing slash for the config [general]
>
host
(e.g. host = http://localhost:8000/
, but we should normalize that setting for them to always include a trailing slash.
Both my hosts in the config file have trailing slashes already, as you can see from OP.
Apologies. Seems rather that host
always gets stripped of trailing slashes in isso/wsgi.py
. However, it is apparently assumed that the thread uri
should always start with a slash.
Could you please confirm in your database that the thread uris start with slashes?
Also, @roughnecks I don't understand where that trailing 41
is coming from - are you using numerical ids for your posts? E.g. the thread would then be /41/
and comments under /41/#isso-17
?
This is what I see in the admin panel, I think that's where the numbers come from:
While for my main domain, I see it like this: https://i.imgur.com/RzW5BKH.jpeg
Could you please confirm in your database that the thread uris start with slashes?
For me they do not:
sqlite> SELECT * FROM threads;
1|post-name|post-name
Perhaps it's only affecting setups which manually set the data-isso-id
and data-title
(like I do)?
Here's mine:
sqlite> SELECT * FROM threads; 1|/|Space Nest! 2|32|32 3|33|33 4|25|25 5|27|27 6|28|28 7|29|29 8|36|36
As far as I understand, it seems we need to prepend a slash to thread uri
s which lack it, though that is an ugly and incomplete fix.
Underlying issue would be allowing custom data-isso-id
values which cannot then be resolved to a URL in the admin and email interfaces. Maybe we should document the drawbacks of using them more clearly?
After some further investigation, I've come to the conclusion that the issue might not necessarily lie within Isso. Instead, the problem seems to be with how I was setting the data-isso-id
and data-title
attributes in my site.
Previously, I was setting these attributes to the page's slug, which resulted in malformed URLs when Isso tried to construct moderation and notification email links.
To solve this, I modified the code to use a more complete path for these attributes. Specifically, I extracted the current path of the page after stripping out the language segment and used it for data-isso-id
and data-title
. (I am using custom attributes to merge the comments for a multilingual site.)
This adjustment allowed Isso to form URLs correctly, showing that correct configuration on the user end can mitigate the issue. I'm not sure it would make sense to change anything at all on Isso's end.
Thank you all for your time and for working on this project.
Seeing this thread while revisiting other issues, it seems we're also being inconsistent with trailing slashes in the config, e.g. https://github.com/posativ/isso/issues/971 disallows trailing slashes for public-endpoint
, but for [server] origin
, we're encouraging setting it.
Would be great for someone to go over this whole business and figure out something more consistent.
Checklist
master
branch does not fix my issueWhat is not working?
First of all I'm using pip to install isso and I just upgraded it to latest version.
When posting comments to a shaarli instance, the link to the comment I receive in my email is like this:
https://links.woodpeckersnest.space41/#isso-17
Someone is adding that trailing number after the TLD and all links result broken.
How can one reproduce this issue?
Have a shaarli instance at latest release - Post a comment under a link.