Project

General

Profile

Bug #16220

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

Added by intrigeri 7 months ago. Updated 5 months ago.

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

100%

Spent time:
Feature Branch:
Type of work:
Sysadmin
Blueprint:
Starter:
Affected tool:

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.


Related issues

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

History

#1 Updated by intrigeri 7 months ago

  • Duplicated by Bug #16219: Images URLs broken in 3.11 release notes added

#2 Updated by intrigeri 7 months ago

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 7 months ago

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

#4 Updated by intrigeri 7 months ago

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

#5 Updated by CyrilBrulebois 7 months ago

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 7 months ago

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

fixed the rewriting trick.

#7 Updated by intrigeri 7 months ago

  • 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 #16227). I'll try to find a better fix ASAP.

#8 Updated by intrigeri 7 months ago

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

#9 Updated by intrigeri 7 months ago

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 7 months ago

  • 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 7 months ago

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

#12 Updated by intrigeri 6 months ago

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

#13 Updated by intrigeri 6 months ago

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

#14 Updated by intrigeri 5 months ago

  • 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 5 months ago

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

looks good!

Also available in: Atom PDF