Relax sync.server.origin to allow paths
Hi! How would you feel about relaxing the restriction that sync.server.origin must be protocol + domain (https://tcsync.example.com) and allow paths also (https://example.com/tcsync/)?
This is useful for me since I run a bunch of services on a single server behind nginx:
proxy_set_header X-Forwarded-For $remote_addr;
location /calibre/ {
proxy_buffering off;
proxy_pass http://127.0.0.1:8080$request_uri;
}
location /calibre {
# we need a trailing slash for the Application Cache to work
rewrite /calibre /calibre/ permanent;
}
# Web UI for taskwarrior
location /tasks-chromy/ {
proxy_pass http://127.0.0.1:8001$request_uri;
}
# root page
location / {
root /usr/share/nginx/html;
try_files /index.html =404;
}
# ...etc
Previously I ran taskd on its dedicated port (53589) which meant I had to configure this ports to be open etc which was always a little worrying. Now with the advent of taskchampion-sync-server I can very almost run it behind port 80/443:
location /taskchampion/ {
proxy_pass http://127.0.0.1:7999/;
}
with exception of this check. Just removing that was sufficient but I have beefed up the error handling a little and added some unittests while I was here.
I appreciate this is a little niche ...but running ones own taskchampion-sync-server instance is already fairly niche so maybe it won't so uncommon given the selection effect ;)
Description
Previously sync.server.origin had to be a scheme + FQDN, for example: https://tcsync.example.com After this we support paths in sync.server.origin, for example: https://example.com/tcsync/ If a path is present it must end with a slash.
This allows for taskchampion-sync-server run behind e.g. nginx on port 80 / 443 at the same time as other services. This is desirable since:
- It avoids the need to choose between a. configuring additional non-standard ports or b. having dedicated HW/VMs/DNS routes just for taskchampion.
- Increases the flexibility of taskchampion.
Also:
- Error on some degenerate URLs (mailto:[email protected])
- Handle sync.server.origin = https://tcsync.example.com?foo=bar more correctly. Previously we geneated invalid URLs of the form: https://tcsync.example.com?foo=barv1/foo/bar.
- Add some unittests for the various cases
- Fix a typo impelementations -> implementations.
Additional information...
-
[ ] I changed C++ code or build infrastructure. Please run the test suite and include the output of
cd build/test && make && ./problems. -
[x] I changed Rust code or build infrastructure. Please run
cargo testand address any failures before submitting.
This appears to be implementing #3414.
This appears to be implementing #3414.
It is! Awesome! @davidhelbig already landed the Taskchampion part of this in https://github.com/GothenburgBitFactory/taskchampion/issues/367, so what remains is #3209 (which will remove much of the taskchampion code modified here) and then the Taskwarrior part of this work, which is present in this PR.
So, if you don't mind waiting a bit for me to finish up #3209 and then rebasing this, that will finish the work! I've been trying to get to #3209 but honestly have been spending all my available time replying to issues. I have a few hours to myself tonight so hopefully can get it done during that time.
OK, #3209 has landed! We'll need to make another TaskChampion release to depend on from here, but it's late :)
However, I see that this PR doesn't change the name of the config option. An origin doesn't, by definition, have a path, and I think we should be consistent about that naming, as mentioned in #3414. One the new version of TaskChampion is out, would you be willing to make an update to TaskWarrior to use a new config name (with compatibility with the old)?
Some of the failing checks are because of #3485.
Thanks! Rebased, now I think the failures are due to taskwarrior still being on TaskChampion v0.5 and the new changes not being released yet. I assume I have started to run the TaskChampion releasing checklist (slowly due to train internet) to see if anything will need fixing but I will only be able to get so far until some with ACLs for crates.io needs to step in.
https://docs.rs/taskchampion/0.6.0/taskchampion/index.html is released, with the origin -> url change!