Skip to content
This repository was archived by the owner on Jun 24, 2025. It is now read-only.

refactor(cookiePath): remove non-working cookiePath option#1686

Merged
eliandoran merged 10 commits intodevelopfrom
refactor_remove-cookiePath
Apr 13, 2025
Merged

refactor(cookiePath): remove non-working cookiePath option#1686
eliandoran merged 10 commits intodevelopfrom
refactor_remove-cookiePath

Conversation

@pano9000
Copy link
Copy Markdown
Contributor

Hi,

this PR basically reverts TriliumNext/Trilium#5473

After some further investigation I found out that this option will currently never work out of the box – and in my testing it only worked due to some special reverse-proxy config in combination I hadn't considered… 🙈 Sorry about that!

Under normal circumstances the cookie will never be set by the server, if you use a different path other than "/", because express never serves that path and hence the cookie is never set or even read.

In order for this to work we would need to introduce some kind of "custom route prefix", that would make express serve the routes with the custom prefix — but that kinda falls more into a reverse proxy job territory and IMHO probably out of scope for Trilium Server itself (correct me if I'm wrong though).

So let's remove this feature for now and amend the docs on how to correctly handle the cookies per instance via the reverse proxy.

For nginx e.g. the correct way to handle this is to set the proxy_cookie_path directive, that will rewrite the cookiePath for us and allow us to have different cookies per instance:

location /trilium/instanceA {
       proxy_pass   http://127.0.0.1:8080; // or whatever you use here

       proxy_cookie_path / /trilium/instanceA;
       …
}

location /trilium/instanceB {
       proxy_pass   http://127.0.0.1:8081; // or whatever you use here

       proxy_cookie_path / /trilium/instanceB;
       …
}

just need to figure out where in the Docs this info fits the best and I will amend them accordingly and set the PR to "Ready for review".

closes TriliumNext/Trilium#5632

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 12, 2025

Folder/File Previous size New size Difference
/upload/TriliumNextNotes-Server-1686-merge-linux-x64.tar.xz 82.73MB +82.73MB (+100.00%)
TOTAL +82.73MB

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 12, 2025

Folder/File Previous size New size Difference
/upload/TriliumNextNotes-1686-merge-linux-x64.deb 135.33MB +135.33MB (+100.00%)
/upload/TriliumNextNotes-1686-merge-linux-x64.flatpak 128.09MB +128.09MB (+100.00%)
/upload/TriliumNextNotes-1686-merge-linux-x64.rpm 143.2MB +143.2MB (+100.00%)
/upload/TriliumNextNotes-1686-merge-linux-x64.zip 173.84MB +173.84MB (+100.00%)
TOTAL +580.47MB

this option will currently not work => the cookie will never
be set by the server, if you use a different path other than "/"

in order for this to work we would need to introduce some kind of
"custom route prefix", that would make express serve the routes with
the custom prefix — but that kinda falls more into a reverse proxy
job territory.

So let's remove this feature for now and amend the docs on how to
correctly handle the cookies per instance via the reverse proxy.
@pano9000 pano9000 force-pushed the refactor_remove-cookiePath branch from 1952d94 to 324223f Compare April 13, 2025 08:53
@pano9000 pano9000 marked this pull request as ready for review April 13, 2025 08:53
@eliandoran eliandoran merged commit 9d1e99f into develop Apr 13, 2025
8 checks passed
@eliandoran eliandoran deleted the refactor_remove-cookiePath branch April 13, 2025 14:12
pano9000 added a commit that referenced this pull request Apr 14, 2025
…g-sample.ini

not sure how I managed to forget to commit this as well, this should've been part of (the now already approved & merged) PR #1686 as well
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Setting cookiePath in config.ini causes crash

3 participants