Links in emails missing slash separator [was: "shaarli links to comments are broken"]

[roughnecks]

Checklist

• I am running the latest version. Installing Isso from GitHub from the master branch does not fix my issue
• I have checked the troubleshooting guide
• I have searched the open issues, but my issue has not already been reported

What 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.

[general]
name = spacenest
dbpath = /home/isso/lib/comments.db
host =
        https://woodpeckersnest.space/
        https://links.woodpeckersnest.space/
max-age = 15m
notify = smtp
reply-notifications = true
log-file = /home/isso/log/isso.log
gravatar = true
gravatar-url = https://www.gravatar.com/avatar/{}?d=identicon&s=55

[moderation]
enabled = true
approve-if-email-previously-approved = true

[admin]
enabled = true
password = redacted

[server]
listen = http://localhost:8100
reload = off
profile = off
public-endpoint = https://isso.woodpeckersnest.space

[guard]
enabled = true
ratelimit = 2
direct-reply = 3
reply-to-self = false
require-author = false
require-email = false

[welpo]

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.

Potential Solution

I think I found the problematic code:

https://github.com/posativ/isso/blob/1871a93f8298d9c0a88359b3777c78fbc77742b9/isso/views/comments.py#L717

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.

[ix5]

@welpo your suggestion seems valid, but we should fix the underlying issue.

local("origin") is set from the [general] > host config setting:

https://github.com/posativ/isso/blob/1871a93f8298d9c0a88359b3777c78fbc77742b9/isso/__init__.py#L141-L142

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.

[roughnecks]

Both my hosts in the config file have trailing slashes already, as you can see from OP.

[ix5]

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 ?

[roughnecks]

This is what I see in the admin panel, I think that's where the numbers come from:

https://i.imgur.com/VySxTBH.jpeg

[roughnecks]

While for my main domain, I see it like this: https://i.imgur.com/RzW5BKH.jpeg

[welpo]

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)?

[roughnecks]

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

[ix5]

As far as I understand, it seems we need to prepend a slash to thread uris 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?

[welpo]

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.

[ix5]

Seeing this thread while revisiting other issues, it seems we're also being inconsistent with trailing slashes in the config, e.g. #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.