Ghost: `{{url}}` helper shows incorrect url when subdirectory equals post slug

[kirrg001]

Issue Summary

If you create a post called /test/. And you switch to a subdirectory called /test/. Then the post should live at /test/test/, but the {{url}} helper displays /test/.

I am pretty sure it's caused by the deduplicate sub dir helper function, see.

This bug must exist for a while (since we introduced this function).

To Reproduce

1. Create post with slug "test" (without subdirectory)
2. Switch to subdirectory /test/
3. Serve your blog and see that the post url is /test/

Solution

Write a test to proof this bug. Come up with an idea how to protect against this use case.

Technical details:

• Ghost Version: latest

[tiendq]

What is this step really? Sorry, I'm still new to Ghost :)

1. Switch to subdirectory /test/

[kirrg001]

@tiendq https://docs.ghost.org/docs/config#section-subpath - url configuration in your config file.

[tiendq]

@kirrg001 I have added tests for deduplicateSubDir, it seems works correctly.

[PranavRudra]

Hey! So I'm new to Ghost but I really want to help out with this issue.

I was trying to reproduce the problematic behavior but I just can't figure out what you mean by slug. Is that another name for a post's tag or is it some other meta information that's included whenever you publish a post?

[kirrg001]

@PranavRudra The slug is the url of a post. If you create a post and open the PSM, you will see an input field on top, which reflects the post slug.

There is already an open PR for this. But many thanks for your help!

[allouis]

@kirrg001 pull #9690 looks stale - I'm gonna pick this up :small_airplane:

[allouis]

From what I can tell, this issue will only occur if you create the slug, and _then_ switch your subdirectory to the same name. If you try to create a slug the same name as the current subdirectory then the api returns with -post appended. So that's good because it means this is fairly edge case.

As for a solution I propose a two step fix to the deduplicateSubDir function.

1. AFAIK slugs always appear at the _end_ of a url? in that case we should be able to make a regex that only deduplicates the subDir if it is in the middle of a url e.g. /subdir/subdir/thing and not /subdir/subdir. That will fix for this usecase, but still maintain the protection of someone using joinUrl with arguments such as 'http://host.tld/subdir', '/subdir/some-page'

2. As slugs cannot contain a / character, if the subDir contains a / (it's >= two levels deep) then we deduplicate as usual and skip the previous logic

@kirrg001 What do you think?

[kirrg001]

@allouis Thanks for getting involved.

This one is actually trickier than i thought 👻

First of all, the deduplicateSubDir was invented to deduplicate the subdir when configuring the admin url. We expect that you don't need to add the subdir in your config file, but if you do so, we deduplicate the subdir.

So that's good because it means this is fairly edge case.

Yeah it's an edge case. In general, if you switch to a subdirectory with existing content, your images will break, which we tell users they have to fix it on their own, because we store the image path with subdirectory currently.

AFAIK slugs always appear at then end of a url?

In theory you can define what you want as permalink structure e.g. /{slug}/{year}/.

As slugs cannot contain a / character

True. Slugs itself cannot, because it's just a string we store in the database, but resource paths can.

---

IMO:

We have to ask ourselfs why we have to use deduplicateSubDir in the urlJoin function.

There is for example this case

https://github.com/TryGhost/Ghost/blob/85213ed5530b2eb62259ebba10419e14aefd30c4/core/server/adapters/storage/utils.js#L18

which contains twice the subdirectory and it get's deduplicated. It's probably/maybe a developer mistake, because we could ask the url utility to get an absolute url of a path without concating the subdirectory manually. I think this is quite a big task, because we have to go through the whole codebase and double check the usages of createUrl, urlJoin, urlFor etc. - we have many entry points, which makes it harder to control the usages.

Maybe we should close this issue and open a new one, which sums up refactoring approaches for the usages of the url utility? e.g. less entry points, rules how to call the url utility? What do you think? If you think that this bug is still solvable without needing to refactor, let me know :) I can't think of a nice code adaption which can solve this problem nicely 😆

[allouis]

Thanks for the info @kirrg001

In theory you can define what you want as permalink structure e.g. /{slug}/{year}/.

This throws the whole approach off then :crab: :crab:

Yeah you're right - refactoring code so we no longer need to call deduplicateSubDir is probably best!

We could maybe replace usages with it in createUrl etc...

from:

url = deduplicateSubDir(url);
return url;

to:

var dedupedUrl = deduplicateSubDir(url);
if (dedupedUrl !== url) {
  // warn about usage
}
return dedupedUrl;

Which should help give us an idea about how widespread the issue is, not sure how you feel about deprecation style warnings like that tho :shrimp:

[kirrg001]

I think that is in general a good idea 👍Only disadvantage is, it's not telling us who has called this function. If we want to add this, I think this should be a debug log - the log should not appear on production blogs, because that is nothing the user has control of. Not sure how much the debug log will help or tell us, because you have to explicitly enable the debug logs 🤔

[kirrg001]

Similar report: https://github.com/TryGhost/Ghost/issues/10309

[Ajphil]

What is the term slug that u have mentioned

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.