Bug #16220

Website's "add trailing slash" trick is partly broken

Added by intrigeri 2018-12-11 18:23:31 . Updated 2019-02-24 17:28:54 .

Status:
Resolved
Priority:
Elevated
Assignee:
groente
Category:
Infrastructure
Target version:
Start date:
2018-12-11
Due date:
% Done:

100%

Feature Branch:
Type of work:
Sysadmin
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

https://tails.boum.org/news/version_3.11 does not redirect to https://tails.boum.org/news/version_3.11/ so included images (whose path is ./$image.png) are broken. Thankfully, only URLs cooked somewhat manually or clicked from recentchanges and maybe a few other places are affected: I thik URLs we display to the vast majority of users work fine.

It might be that the way this trick was implemented can’t work when a directory of the same name as the page exists, which is necessary the case when we add attachments to said page.


Subtasks


Related issues

Related to Tails - Bug #16124: URLs without explicit .html are not redirected anymore Resolved 2018-11-13
Related to Tails - Bug #16227: Weird output when pushing to tails.git Resolved 2018-12-15
Related to Tails - Bug #16259: Redirection on /donate drop the query parameter Resolved 2018-12-30
Has duplicate Tails - Bug #16219: Images URLs broken in 3.11 release notes Duplicate 2018-12-11
Blocks Tails - Feature #13284: Core work: Sysadmin (Adapt our infrastructure) Confirmed 2017-06-30

History

#1 Updated by intrigeri 2018-12-11 18:23:49

  • has duplicate Bug #16219: Images URLs broken in 3.11 release notes added

#2 Updated by intrigeri 2018-12-11 18:24:51

Ah, also it looks like this trick’s redirect won’t trigger for pages whose name contains a dot, such as… news/version_3.11. Looks like we need a more clever regexp.

#3 Updated by intrigeri 2018-12-11 18:28:42

  • related to Bug #16124: URLs without explicit .html are not redirected anymore added

#4 Updated by intrigeri 2018-12-11 18:30:54

  • blocks Feature #13284: Core work: Sysadmin (Adapt our infrastructure) added

#5 Updated by CyrilBrulebois 2018-12-12 11:36:52

Would this work as an interim measure?

+rewrite ^/news/version_([0-9.]+)$ /news/version_$1/ permanent;

that’s for puppet-tails.git’s templates/website/nginx/rewrite_rules.conf.erb

(Disclaimer: first time looking at nginx redirects.)

#6 Updated by groente 2018-12-15 14:03:03

  • Status changed from Confirmed to Resolved
  • Assignee changed from intrigeri to groente

fixed the rewriting trick.

#7 Updated by intrigeri 2018-12-17 09:33:37

  • Status changed from Resolved to In Progress
  • Assignee changed from groente to intrigeri
  • Priority changed from Elevated to High
  • % Done changed from 0 to 10

I had to revert the fix which broke other stuff (at least refreshing the website after pushing to Git, see Bug #16227). I’ll try to find a better fix ASAP.

#8 Updated by intrigeri 2018-12-17 09:33:45

  • related to Bug #16227: Weird output when pushing to tails.git added

#9 Updated by intrigeri 2018-12-17 09:49:31

CyrilBrulebois wrote:
> Would this work as an interim measure?
>
> […]
>
> that’s for puppet-tails.git’s templates/website/nginx/rewrite_rules.conf.erb

Good idea, I just did that.

> (Disclaimer: first time looking at nginx redirects.)

You got it right :)

#10 Updated by intrigeri 2018-12-17 10:43:49

  • Subject changed from Website's "add trailing slash" trick is broken to Website's "add trailing slash" trick is partly broken
  • Priority changed from High to Elevated

With the workaround in place for the biggest problem caused by this bug, I’ll focus on higher priority things for now and will come back to it later.

#11 Updated by intrigeri 2019-01-02 13:20:29

  • related to Bug #16259: Redirection on /donate drop the query parameter added

#12 Updated by intrigeri 2019-01-21 10:15:29

Extended the temporariry hack (with commit 1893905 in puppet-tails) to also apply to calls for testing RCs.

#13 Updated by intrigeri 2019-01-27 11:23:44

  • Target version changed from Tails_3.12 to Tails_3.13

#14 Updated by intrigeri 2019-02-15 16:31:14

  • Assignee changed from intrigeri to groente
  • % Done changed from 10 to 50
  • QA Check set to Ready for QA

Deployed a fix for this + a few other issues (e.g. custom headers not set in some cases), reverted the temporary workarounds, and clarified/documented/refactored the config so hopefully we don’t break this sort of things too easily in the future: 4140a5d88682c049a8f7d4e4757f21de3fb25c7c in the manifests repo.

If this breaks more (or more important) stuff than it fixes while I’m AFK, feel free to revert the whole thing and let me fix it once I’m back. I’m starting to consider writing a test suite for this stuff: since 3 months, too often we broke something else when we fixed a problem; my current manual testing methodology takes a few minutes by iteration, which is too much, and I’m sure I’m not testing everything I should.

#15 Updated by groente 2019-02-24 17:28:54

  • Status changed from In Progress to Resolved
  • % Done changed from 50 to 100
  • QA Check changed from Ready for QA to Pass

looks good!