Dynamic Links and Editor Integration

[Xiphoseer]

This PR is to provide an implementation for the seemingly abandoned #737. It also includes some changes to the Markdown and WYSIWYG editor, to use the new syntax by default.

I'll provide more details later, but I chose a custom URI as the most appropriate solution to implement this feature, as it works nicely alongside other link-schemes, does not produce URL-encoded characters, wherever it is used. I'll also submit a pull-request for another branch, which does not touch the editors at all, making this feature entirely optional for the user.

[ssddanbrown]

Thanks @Xiphoseer for continuing this work.

I'll also submit a pull-request for another branch, which does not touch the editors at all, making this feature entirely optional for the user.

Don't worry about that, If we were to go down this route I'd prefer it to be all-in rather than an option. If there's a benefit to dynamic links it should be a benefit for all.

To be honest, I'm still in two minds about this. Dynamic links makes the stored content less standard and adds additional complexity to both the editing and rendering systems to achieve only a benefit in edge-case scenarios.

Tricky one....

[Xiphoseer]

As a user, my organisation is currently migrating our documentation from a Trac, some GitLab wikis and a MediaWiki to a single bookstack instance. The URL for this doumentation is not final yet, as we want to use the same URL our previous wiki had at some point. Not being able to set internal links that will survive the switch is really unfortunate for us. At the same time, I know from a past migration (from Silverstripe to DjangoCMS), that it is problematic to have the source-markup only use IDs and the rendered markup being -- well, rendered.

As a developer, I completely understand your hesitant to include a feature that makes a central wiki-feature such as links non-standard. During this re-implementation, I tried some options and would like to lay out my findings below:

• The existing code was limited to href="..." and would not have worked with <links>, which have their own URI as text anyways. I do consider this a major flaw of only supporting a subset of markdown links, and parting from that strategy reduced the code-complexity quite a bit.
• The initial concept was just to replace all occurences of {{entity@id}} with the link. This was the format suggested in POC: Dynamically Render Entity Links #737 and would have worked just fine if the existing code was applied before the markdown to HTML transformation, which it was not. The link was thus escaped to htmlentities, which would have made the search-code more complex.
• As we are talking about links, curly braces are usually (e.g. Vue, Blade) expected to provide full interpolation and not just some regex-replace, and the characters used would not get escaped by default, I decided to go with a custom URI scheme.
• I considered using bs:, bookstack: and x-bs: before reading the corresponding RFC 7595 more closely, which specified in Section 3.8 that vendor-specific formats SHOULD use a prefix based on their domain name, expressed in reverse order ... to prevent collisions and to make it possible to identify the owner of a private-use scheme.
• At that point, I settled on com.bookstackapp.<entity>:<id>, and started implementing that into editor features. As it turns out, while this scheme would be the most standard choice, it does not work well with the Link-Editor in the WYSIWYG editor. A link using the protocol above is prefixed with the current domain, and links such as http://localhost:8080/com.bookstackapp.page:1 show up.
• Some further testing revealed, that this does not happen for a simple protocol name without periods. Ultimately, this is a problem with the TinyMCE link plugin not supporting URI schemes that are standard-compliant, probably because registered schemes are not supposed to use periods to prevent collision with reverse DNS, organisation specific URIs (oh, the irony 🙃).
• I didn't want to wait for changes upstream, so I ended up going with just bookstackapp:. That is close enough to the domain that it should avoid conflicts with schemes registered in the future 😁, and doesn't contain any periods.
• The other elephant in the room is the use of IDs vs. semantic identifiers. it should be even easier to implement a scheme variant of bookstack:link:<some-relative-URL> that works for any resource. This implementation has the advantage of abstracting away the base URL of the app, but would not solve any problems relating to the permanently linking pages, even when they change their names. Another disadvantage is that the parsing would get more complex.
• A compromise between these approaches would be something like bookstackapp:book:slug:<book-slug>, as slugs are already used extensively in the URLs and carry some semantic meaning. This would steer away from more compley parsing, but instead require the protocol to URL ttranslation to be implemented for every entities.
• Finally, it just occurred to me, that the short term solution to the issue for my organisation might be to use a plain old HTML <base> tag and just use relative links. Even if that works, I'll try to push towards implementing whichever of the options above you find most appropriate. One could possibly even support all mentioned protocol variants at the same time.

TLDR: RFC says reverse-DNS links, TinyMCE doesn't play nice, <base> may be useful for internal links.

[ssddanbrown]

Thanks for the in-depth discovery @Xiphoseer.

I can see the use-case of not being tied to a specific domain, but in those cases a simple DB find/replace can suffice without adding the overhead of supporting dynamic links, Which has been discussed in #1224 and #1225.

I guess it comes down to this:

1. What would dynamic URLs offer that a DB find/replace wouldn't?
2. Is the answer to the above worth additional complexity on each system that handles URLs?

[Xiphoseer]

Find and replace in the database usually requires some sort of understanding of the database schema, so doing that without a command provided by the application as suggested in #1225 is something that I would consider bad-practice for something that otherwise just depends on how the application is configured. With the <base href="..."> solution, we get pretty much what we want, and I think that would be a much cleaner solution without any additional code for the server to run though, although it may require some restructuring and currently does not play nice with the table of contents feature.

The other and more relevant use case that I'd see for this feature is to be able to link to books, chapters, shelves and possibly images in the same way that you can link to pages via the /link/<id> feature, in a way that does not depend on the name of the target. Producing dead-links on accident, without being necessarily made aware of it just by renaming the page is not ideal.

On adding additional complexity to each system handling URLs, I'd argue that changes to the editor supporting selection of those safe URLs mentioned above are worth it, but the custom protocol may be a bit much.

Finally: Is there a way to send you some security-related observations, without putting it up as a public issue?

[ssddanbrown]

so doing that without a command provided by the application as suggested in #1225 is something that I would consider bad-practice.

I am looking to implement such a command though, in the not-too-distant future.

The other and more relevant use case that I'd see for this feature is to be able to link to books, chapters, shelves and possibly images in the same way that you can link to pages via the /link/<id> feature, in a way that does not depend on the name of the target.

I get the technical idealism in this but I like having links that somewhat describe the content. Even if a link breaks by the content being deleted you still get an idea of what was linked to.

Producing dead-links on accident, without being necessarily made aware of it just by renaming the page is not ideal.

There is some level of handling in this case, although not perfect, via the revision system.

On adding additional complexity to each system handling URLs, I'd argue that changes to the editor supporting selection of those safe URLs mentioned above are worth it

I'm still not 100% convinced. The more I think about it the more I like the absolute-ness and simplicity of absolute URL's.

Is there a way to send you some security-related observations, without putting it up as a public issue?

Sure! You should be able to see my email when you click on my profile, Feel free to report any security concerns there.

[tylerlucas]

Any update on this request, or has it been abandoned?

[ssddanbrown]

@tylerlucas It's essentially blocked by my indecision, especially while I've been focused on the redesign. As per my comments above, I'm still not convinced this solution is the best path to achieving the intended benefits.

[ssddanbrown]

Thanks for your work here @Xiphoseer, But after thinking about it over the last few years I would not want to implement dynamic links in this manner, I remain on the side of my previous comments above.

I would like to standardise on id-based URLs in the future, but I'd like them to still be full valid URLs to avoid any editor or raw content usability complexity. At some point in the future I'll start centralising feedback into a proposal to define a new format.