Debian Bug report logs - #911356
ikiwiki: "po" plugin can insert raw ".po" file contents with [[!inline ... ]] directives

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: submit@bugs.debian.org

Subject: ikiwiki: "po" plugin can insert raw ".po" file contents with [[!inline ... ]] directives

Date: Thu, 18 Oct 2018 20:11:32 -0400

[Message part 1 (text/plain, inline)]

Source: ikiwiki
Version: 3.20180311-1
Severity: important
Tags: patch

Hi,

There is an issue where an initial "inline" directive would be
translated correctly but subsequent inlines of the same page would
result in the raw contents of the ".po" file (ie. starting with the raw
copyright headers!) being inserted into the page instead.

§

For example, given a "index.mdwn" containing:

    [[!inline pages="inline" raw="yes"]]
    [[!inline pages="inline" raw="yes"]]

… and an "index.de.po" of:

    msgid "[[!inline pages=\"inline\" raw=\"yes\"]]\n"
    msgstr "[[!inline pages=\"inline.de\" raw=\"yes\"]]\n"

… together with an "inline.mdwn" of:

   This is inlined content.

… and an "inline.de.po" of:

    msgid "This is inlined content."
    msgstr "This is German inlined content."

§

This would result in the following translation:

    This is the inlined content.
    # SOME DESCRIPTIVE TITLE
    # Copyright (C) YEAR Free Software Foundation, Inc.
    # This file is distributed under the same license as the PACKAGE package.
    # FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.

… instead of (of course)

    This is the inlined content.
    This is the inlined content.

§

Patch against ikiwiki 3.20180311-1 attached.


Best wishes,

-- 
      ,''".
     : :'  :     Chris Lamb
     ". "'"      lamby@debian.org / chris-lamb.co.uk
       "-

[ikiwiki.diff.txt (text/plain, attachment)]

Message #12 received at 911356@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: 911356@bugs.debian.org

Subject: Re: ikiwiki: "po" plugin can insert raw ".po" file contents with [[!inline ... ]] directives

Date: Thu, 18 Oct 2018 20:18:29 -0400

forwarded 911356 https://ikiwiki.info/todo/Re-use_translated_content_instead_of_skipping_if_previously_translated/
thanks

This is being tracked upstream here:

  https://ikiwiki.info/todo/Re-use_translated_content_instead_of_skipping_if_previously_translated/

However, I can't seem to encourage upstream to accept the patch and/or
even know what the next steps are, hence filing it here.

Could the Debian maintainer(s) poke upstream on this and could this be
applied in Debian in the meantime?


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #17 received at 911356@bugs.debian.org (full text, mbox, reply):

From: Simon McVittie <smcv@debian.org>

To: Chris Lamb <lamby@debian.org>, 911356@bugs.debian.org

Subject: Re: Bug#911356: ikiwiki: "po" plugin can insert raw ".po" file contents with [[!inline ... ]] directives

Date: Fri, 19 Oct 2018 09:11:48 +0100

On Thu, 18 Oct 2018 at 20:18:29 -0400, Chris Lamb wrote:
> However, I can't seem to encourage upstream to accept the patch and/or
> even know what the next steps are, hence filing it here.

Sorry, neither the Debian maintainer nor the most frequent upstream
maintainer have as much time to review ikiwiki patches as they would
like. (Both are me.)

The po plugin is, in general, rather complicated, which tends to push it
down my priority list - I can't review po patches without first working
out (again) how it works and why it's the way it is, and because I don't
use it myself, I'm concerned that I'll break it and not know.

If you can put together a regression test for this bug that renders a
translated page and inspects the HTML output (t/img.t, t/meta.t, or a
combination of t/po.t and t/render.t might be a good basis) that would
make me a lot more confident about accepting patches. At the moment I
don't think we have any "full stack" test coverage that actually
renders HTML from a translated page.

Thanks,
    smcv

Message #22 received at 911356@bugs.debian.org (full text, mbox, reply):

From: Simon McVittie <smcv@debian.org>

To: Chris Lamb <lamby@debian.org>, 911356@bugs.debian.org

Cc: intrigeri@debian.org

Subject: Re: Bug#911356: ikiwiki: "po" plugin can insert raw ".po" file contents with [[!inline ... ]] directives

Date: Thu, 31 Jan 2019 20:53:34 +0000

Control: tags -1 + moreinfo

On Thu, 18 Oct 2018 at 20:11:32 -0400, Chris Lamb wrote:
> There is an issue where an initial "inline" directive would be
> translated correctly but subsequent inlines of the same page would
> result in the raw contents of the ".po" file (ie. starting with the raw
> copyright headers!) being inserted into the page instead.

As noted on the upstream bug, I've added a failing test-case for this to
ikiwiki.

Unfortunately, I don't think either the patch on this bug or the patch sent
upstream was correct: they introduce a cache that holds a large amount of wiki
content in memory, which ikiwiki tries hard to avoid for performance, and I'm
not confident that the cache is invalidated frequently enough for correctness.

I have referenced an alternative solution (mostly code deletion) on the
upstream bug. Please try:
https://git.pseudorandom.co.uk/smcv/ikiwiki.git/commitdiff/wip/po-filter-every-time

I would particularly appreciate review and testing from intrigeri, who wrote
the code that I'm deleting and hopefully has a better picture of why it was/is
necessary.

(I would also like to have more extensive test coverage for the po plugin in
general: I don't think any of the ikiwiki maintainers use it, so automated
tests are the only way we can make sure it hasn't regressed.)

Thanks,
    smcv

Message #29 received at 911356@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: Simon McVittie <smcv@debian.org>, 911356@bugs.debian.org

Cc: intrigeri@debian.org

Subject: Re: Bug#911356: ikiwiki: "po" plugin can insert raw ".po" file contents with [[!inline ... ]] directives

Date: Wed, 06 Feb 2019 23:57:15 +0100

Hi Simon et al.,

> I would particularly appreciate review and testing from intrigeri, who wrote
> the code that I'm deleting and hopefully has a better picture of why it was/is
> necessary.

ACK. FYI this is being tracked in Tails at:

  https://redmine.tails.boum.org/code/issues/6907


Best wishes,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org 🍥 chris-lamb.co.uk
       `-

Message #34 received at 911356@bugs.debian.org (full text, mbox, reply):

From: intrigeri <intrigeri@debian.org>

To: Simon McVittie <smcv@debian.org>, 911356@bugs.debian.org

Cc: Chris Lamb <lamby@debian.org>

Subject: Re: Bug#911356: ikiwiki: "po" plugin can insert raw ".po" file contents with [[!inline ... ]] directives

Date: Sat, 09 Feb 2019 19:39:13 +0100

Control: tag -1 - moreinfo

Hi Simon,

Simon McVittie:
> I have referenced an alternative solution (mostly code deletion) on the
> upstream bug. Please try:
> https://git.pseudorandom.co.uk/smcv/ikiwiki.git/commitdiff/wip/po-filter-every-time

> I would particularly appreciate review and testing from intrigeri, who wrote
> the code that I'm deleting and hopefully has a better picture of why it was/is
> necessary.

(I've tried to reply on the upstream bug but the anonpush mechanism
seems to be broken and the CGI rejects my edit, on the grounds that
the blogspam plugin is unhappy with it — presumably because I'm
using Tor. So here we go :)

First, thanks a lot for diving into this topic.

Joey's commit 192ce7a2 was prompted by [[bugs/po_vs_templates]] and
back then, it fixed that bug (and allowed me to remove some very ugly
workarounds: d877b964, d4136aea), so surely it was _somewhat_ relevant
to `po` users back then. But indeed, I doubt it was relevant for
`inline`, which is a different beast, in that it still calls `filter`
itself on the inlined content.

I fully agree that "output of a filter hook is never passed back
through filter hooks" should be an invariant.

I've just spent quite some time trying to understand what these
"preprocessing loops" were, and I've failed. I suspect that's related
to the code the "Avoid loops of preprocessed pages preprocessing"
comment in `IkiWiki.pm` is about, but I can't find how it could be.
So I'm afraid I won't be able to shed any light on this: ten years
have passed and I'm sorry my commit message back then was
suboptimal :/

I'm not keen on keeping a workaround (the `alreadyfiltered` mechanism)
for an under-specified bug that surely existed 10 years ago but might
have been fixed since then, especially when this workaround itself
clearly causes a well understood bug which causes major trouble for
what might be the main user of this plugin nowadays (the Tails
website, for which the `po` plugin was developed in the first place).
So in principle, I'm very much in favor of removing the buggy
workaround as you did.

I've tested your proposed branch locally on the Tails website (957
Markdown and HTML files, 1758 PO files), that uses nested `inline`,
`pagetemplate`, `toggle`, `sidebar` and more. I could not spot any
issue, be it after a full rebuild, or when triggering an incremental
refresh after modifying some pages involved in the most intense usage
we have of the `po` plugin combined with these other ones.

I will now deploy a modified version of ikiwiki, that includes your
patch, on our production website, which will give it exposure to more
realistic usage. I'll report back in 7-10 days, which hopefully should
leave sufficient time for getting the fix in Buster; but if this
timeframe is not adequate for you, feel free to release and upload
without waiting for further test results from me.

> (I would also like to have more extensive test coverage for the po plugin in
> general: I don't think any of the ikiwiki maintainers use it, so automated
> tests are the only way we can make sure it hasn't regressed.)

I share these feelings: even though I'm using this plugin a lot,
I deeply regret having written it before I learnt much about software
testing. I can't realistically promise I'll increase the test coverage
in general, but in the future I'll try my best to at least add tests
for the areas affected by changes I'll submit (there's one more branch
in the pipeline), simply because I have happily unlearned how to write
code without writing tests.

Cheers,
-- 
intrigeri

Message #41 received at 911356@bugs.debian.org (full text, mbox, reply):

From: intrigeri <intrigeri@debian.org>

To: Simon McVittie <smcv@debian.org>, 911356@bugs.debian.org

Cc: Chris Lamb <lamby@debian.org>

Subject: Re: Bug#911356: ikiwiki: "po" plugin can insert raw ".po" file contents with [[!inline ... ]] directives

Date: Mon, 18 Feb 2019 18:17:57 +0100

Hi Simon,

intrigeri:
> I will now deploy a modified version of ikiwiki, that includes your
> patch, on our production website, which will give it exposure to more
> realistic usage. I'll report back in 7-10 days, which hopefully should
> leave sufficient time for getting the fix in Buster;

Nine days later, I've not spotted any issue with your patch there.
It's worth noting that in the meantime we've released a new Tails,
that pretty often triggers this very bug. So please apply your patch
to ikiwiki upstream :)

It would be sweet if this could make it into Buster but if that's too
late, no worries (it took me some time to reply in the first place).

Cheers,
-- 
intrigeri

Message #46 received at 911356@bugs.debian.org (full text, mbox, reply):

From: Simon McVittie <smcv@debian.org>

To: intrigeri <intrigeri@debian.org>, 911356@bugs.debian.org

Cc: Chris Lamb <lamby@debian.org>

Subject: Re: Bug#911356: ikiwiki: "po" plugin can insert raw ".po" file contents with [[!inline ... ]] directives

Date: Sun, 24 Feb 2019 18:30:11 +0000

On Mon, 18 Feb 2019 at 18:17:57 +0100, intrigeri wrote:
> Nine days later, I've not spotted any issue with your patch there.
> It's worth noting that in the meantime we've released a new Tails,
> that pretty often triggers this very bug. So please apply your patch
> to ikiwiki upstream :)

This is fixed in git master and will be in the next upstream release.

    smcv

Message #51 received at 911356-close@bugs.debian.org (full text, mbox, reply):

From: Simon McVittie <smcv@debian.org>

To: 911356-close@bugs.debian.org

Subject: Bug#911356: fixed in ikiwiki 3.20190228-1

Date: Thu, 28 Feb 2019 18:28:29 +0000

Source: ikiwiki
Source-Version: 3.20190228-1

We believe that the bug you reported is fixed in the latest version of
ikiwiki, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 911356@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Simon McVittie <smcv@debian.org> (supplier of updated ikiwiki package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Format: 1.8
Date: Tue, 26 Feb 2019 23:04:42 +0000
Source: ikiwiki
Architecture: source
Version: 3.20190228-1
Distribution: unstable
Urgency: high
Maintainer: Simon McVittie <smcv@debian.org>
Changed-By: Simon McVittie <smcv@debian.org>
Closes: 911356
Changes:
 ikiwiki (3.20190228-1) unstable; urgency=high
 .
   * New upstream release
     - aggregate: Use LWPx::ParanoidAgent if available.
       Previously blogspam, openid and pinger used this module if available,
       but aggregate did not. This prevents server-side request forgery or
       local file disclosure, and mitigates denial of service when slow
       "tarpit" URLs are accessed.
       (CVE-2019-9187)
     - blogspam, openid, pinger: Use a HTTP proxy if configured, even if
       LWPx::ParanoidAgent is installed.
       Previously, only aggregate would obey proxy configuration. If a proxy
       is used, the proxy (not ikiwiki) is responsible for preventing attacks
       like CVE-2019-9187.
     - aggregate, blogspam, openid, pinger: Do not access non-http, non-https
       URLs.
       Previously, these plugins would have allowed non-HTTP-based requests if
       LWPx::ParanoidAgent was not installed. Preventing file URIs avoids local
       file disclosure, and preventing other rarely-used URI schemes like
       gopher mitigates request forgery attacks.
     - aggregate, openid, pinger: Document LWPx::ParanoidAgent as strongly
       recommended.
       These plugins can request attacker-controlled URLs in some site
       configurations.
     - blogspam: Document LWPx::ParanoidAgent as desirable.
       This plugin doesn't request attacker-controlled URLs, so it's
       non-critical here.
     - blogspam, openid, pinger: Consistently use cookiejar if configured.
       Previously, these plugins would only obey this configuration if
       LWPx::ParanoidAgent was not installed, but this appears to have been
       unintended.
     - po: Always filter .po files.
       The po plugin in previous ikiwiki releases made the second and
       subsequent filter call per (page, destpage) pair into a no-op,
       apparently in an attempt to prevent *recursive* filtering (which as
       far as we can tell can't happen anyway), with the undesired effect
       of interpreting the raw .po file as page content (e.g. Markdown)
       if it was inlined into the same page twice, which is apparently
       something that tails.org does. Simplify this by deleting the code
       that prevented repeated filtering. Thanks, intrigeri
       (Closes: #911356)
Checksums-Sha1:
 23fbaf51ff241ee4a4217acad7ec314487d5cfc3 2522 ikiwiki_3.20190228-1.dsc
 46f5b0a1498c1e098fe248eae1f2e3f56b25dc2f 2672244 ikiwiki_3.20190228.orig.tar.xz
 3117a095beda469da00db825ea5d3862f507dbb9 86996 ikiwiki_3.20190228-1.debian.tar.xz
 4d0bde5b1ba48cd44057fae507dde5aee250e52c 4969 ikiwiki_3.20190228-1_source.buildinfo
Checksums-Sha256:
 963d9cc94926faddd17e21c10cc20b72e2d49280a7e61cf2986f8e20f6f6da60 2522 ikiwiki_3.20190228-1.dsc
 d07a4d0da60c3e4de698a4dc54d0445547e762b37f0d433b0d664d88155dfe9e 2672244 ikiwiki_3.20190228.orig.tar.xz
 0bc38826600d23b572fe03704b8f10cd13ec111cf6bcd94bf0d9d09f83d2e42d 86996 ikiwiki_3.20190228-1.debian.tar.xz
 10cb6aaf5ea89b43aac162abddc9b95e98c577ccb4e21d84321f8593b5f68c2d 4969 ikiwiki_3.20190228-1_source.buildinfo
Files:
 985468a47fdec9139ad07389d6b588d9 2522 web optional ikiwiki_3.20190228-1.dsc
 7d3b0b1fd375fc94b30b3397b260e61f 2672244 web optional ikiwiki_3.20190228.orig.tar.xz
 fb48c5068071da26b100da3b878b32c4 86996 web optional ikiwiki_3.20190228-1.debian.tar.xz
 3d736ea01b161e377cdad06461d87ad8 4969 web optional ikiwiki_3.20190228-1_source.buildinfo

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEENuxaZEik9e95vv6Y4FrhR4+BTE8FAlx4Ig4ACgkQ4FrhR4+B
TE+YlBAAk/soeKca813hRM1csFMfb10nuy02w2rYLd9mU8iifN3qV0MpIp5xO+uU
N+kIaA3+9L70NyZA6Aovh+0V4ktjdIiyTw0l4/Ir+LDjeT0apq6Y6AMpw76x4wx+
7Yuifys3nmfHb0QzV5v/owjFFBHZYzOS9uJ/S9LNqF2j1rJXEzUhdonNK+A5sYp+
ih/94SpwV5eljMQQ5aII8dUGP/5KBopeCv4tuFPfMC2o7XxeVh/9/1gws74Krqpo
CeMWktcw82AT66IZkUcfEfiaxmcCYZeZKt+n9YQoCH7cySOQ8f+x/tz+Q1xciBHy
aH/yinIEBOYlUmOSNj8y1LqX1UPZBdZylsMiIrCItOeaEulxxKQLTsyjzTKSYMpr
hvo3FlXIK+dcP/dl4NB3tYjq+Veb/OxUQ6bcqy7ELXfjtyoSnUetOOO3L9+dIkYq
KJ7pBgRWdm5qTCUWkqu9Wdj/G7LKbFbwi/mnivD7zPGm9S0Joq6UhW83UZu2qSyp
UiWCU1W1UjG3uQnbQ+mZZfBM5O5BWDYvWStGVGyniTeDfIqfMHlnCby/vi2V/wFB
3S0fcui720SkPtyFTXCXp8KBWYAeUJQsjlZEYQuxUDDsQO3znl5yvV5LOyoOjoNq
cPKsUdrwGyrpszO3QfrcCVdrEHmb9dSz+CpRh3IJR7zxzfOwrVo=
=eBiv
-----END PGP SIGNATURE-----

Send a report that this bug log contains spam.