]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/blobdiff - doc/plugins/po.mdwn
po: added postscan hook, to make pages depend on the pages linking to them
[git.ikiwiki.info.git] / doc / plugins / po.mdwn
index 6a2409847965ae42c8feac33052df87d8b619a20..7d8da987e117c538a9f5120d8db6459242cc6bb3 100644 (file)
@@ -6,7 +6,7 @@ gettext, using [po4a](http://po4a.alioth.debian.org/).
 
 It depends on the Perl `Locale::Po4a::Po` library (`apt-get install po4a`).
 
 
 It depends on the Perl `Locale::Po4a::Po` library (`apt-get install po4a`).
 
-[[!toc]]
+[[!toc levels=2]]
 
 Introduction
 ============
 
 Introduction
 ============
@@ -129,6 +129,10 @@ Usage
 Templates
 ---------
 
 Templates
 ---------
 
+When `po_link_to` is not set to `negotiated`, one should replace some
+occurrences of `BASEURL` with `HOMEPAGEURL` to get correct links to
+the wiki homepage.
+
 The `ISTRANSLATION` and `ISTRANSLATABLE` variables can be used to
 display things only on translatable or translation pages.
 
 The `ISTRANSLATION` and `ISTRANSLATABLE` variables can be used to
 display things only on translatable or translation pages.
 
@@ -194,14 +198,18 @@ Also, when the plugin has just been enabled, or when a page has just
 been declared as being translatable, the needed POT and PO files are
 created, and the PO files are checked into version control.
 
 been declared as being translatable, the needed POT and PO files are
 created, and the PO files are checked into version control.
 
-Discussion pages
-----------------
+Discussion pages and other sub-pages
+------------------------------------
 
 Discussion should happen in the language in which the pages are
 written for real, *i.e.* the "master" one. If discussion pages are
 enabled, "slave" pages therefore link to the "master" page's
 discussion page.
 
 
 Discussion should happen in the language in which the pages are
 written for real, *i.e.* the "master" one. If discussion pages are
 enabled, "slave" pages therefore link to the "master" page's
 discussion page.
 
+Likewise, "slave" pages are not supposed to have sub-pages;
+[[WikiLinks|wikilink]] that appear on a "slave" page therefore link to
+the master page's sub-pages.
+
 Translating
 -----------
 
 Translating
 -----------
 
@@ -246,31 +254,134 @@ Can any sort of directives be put in po files that will cause mischief
 (ie, include other files, run commands, crash gettext, whatever)?
 
 > No [documented](http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files)
 (ie, include other files, run commands, crash gettext, whatever)?
 
 > No [documented](http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files)
-> directive is supposed to do so.
+> directive is supposed to do so. [[--intrigeri]]
 
 ### Running po4a on untrusted content
 
 Are there any security issues on running po4a on untrusted content?
 
 
 ### Running po4a on untrusted content
 
 Are there any security issues on running po4a on untrusted content?
 
-> To say the least, this issue is not well covered, at least publicly:
->
-> - the documentation does not talk about it;
-> - grep'ing the source code for `security` or `trust` gives no answer.
->
-> I'll ask their opinion to the po4a maintainers.
->
-> I'm not in a position to audit the code, but I had a look anyway:
->
-> - no use of `system()`, `exec()` or backticks in `Locale::Po4a`; are
->   there any other way to run external programs in Perl?
-> - a symlink attack vulnerability was already discovered, so I "hope"
->   the code has been checked to find some more already
-> - the po4a parts we are using themselves use the following Perl
->   modules: `DynaLoader`, `Encode`, `Encode::Guess`,
->   `Text::WrapI18N`, `Locale::gettext` (`bindtextdomain`,
->   `textdomain`, `gettext`, `dgettext`)
->
->  --[[intrigeri]]
+To say the least, this issue is not well covered, at least publicly:
+
+- the documentation does not talk about it;
+- grep'ing the source code for `security` or `trust` gives no answer.
+
+On the other hand, a po4a developer answered my questions in
+a convincing manner, stating that processing untrusted content was not
+an initial goal, and analysing in detail the possible issues.
+
+#### Already checked
+
+- the core (`Po.pm`, `Transtractor.pm`) should be safe
+- po4a source code was fully checked for other potential symlink
+  attacks, after discovery of one such issue
+- the only external program run by the core is `diff`, in `Po.pm` (in
+  parts of its code we don't use)
+- `Locale::gettext`: only used to display translated error messages
+- Nicolas François "hopes" `DynaLoader` is safe, and has "no reason to
+  think that `Encode` is not safe"
+- Nicolas François has "no reason to think that `Encode::Guess` is not
+  safe". The po plugin nevertheless avoids using it by defining the
+  input charset (`file_in_charset`) before asking `Transtractor` to
+  read any file. NB: this hack depends on po4a internals to stay
+  the same.
+
+#### To be checked
+
+##### Locale::Po4a modules
+
+The modules we want to use have to be checked, as not all are safe
+(e.g. the LaTeX module's behaviour is changed by commands included in
+the content); they may use regexps generated from the content.
+
+`Chooser.pm` only loads the plugin we tell it too: currently, this
+means the `Text` module only.
+
+`Text` module (I checked the CVS version):
+
+- it does not run any external program
+- only `do_paragraph()` builds regexp's that expand untrusted
+  variables; they seem safe to me, but someone more expert than me
+  will need to check. Joey?
+
+  > Freaky code, but seems ok due to use of `quotementa`.
+
+##### Text::WrapI18N
+
+`Text::WrapI18N` can cause DoS (see the
+[Debian bug #470250](http://bugs.debian.org/470250)), but it is
+optional and we do not need the features it provides.
+
+It is loaded if available by `Locale::Po4a::Common`; looking at the
+code, I'm not sure we can prevent this at all, but maybe some symbol
+table manipulation tricks could work; overriding
+`Locale::Po4a::Common::wrapi18n` may be easier. I'm no expert at all
+in this field. Joey? [[--intrigeri]]
+
+> Update: Nicolas François suggests we add an option to po4a to
+> disable it. It would do the trick, but only for people running
+> a brand new po4a (probably too late for Lenny). Anyway, this option
+> would have to take effect in a `BEGIN` / `eval` that I'm not
+> familiar with. I can learn and do it, in case no Perl wizard
+> volunteers to provide the po4a patch. [[--intrigeri]]
+
+>> That doesn't really need to be in a BEGIN. This patch moves it to
+>> `import`, and makes this disable wrap18n:
+>> `use Locale::Po4a::Common q{nowrapi18n}` --[[Joey]]
+
+<pre>
+--- /usr/share/perl5/Locale/Po4a/Common.pm     2008-07-21 14:54:52.000000000 -0400
++++ Common.pm  2008-11-11 18:27:34.000000000 -0500
+@@ -30,8 +30,16 @@
+ use strict;
+ use warnings;
+-BEGIN {
+-    if (eval { require Text::WrapI18N }) {
++sub import {
++    my $class=shift;
++    my $wrapi18n=1;
++    if ($_[0] eq 'nowrapi18n') {
++      shift;
++      $wrapi18n=0;
++    }
++    $class->export_to_level(1, $class, @_);
++
++    if ($wrapi18n && eval { require Text::WrapI18N }) {
+     
+         # Don't bother determining the wrap column if we cannot wrap.
+         my $col=$ENV{COLUMNS};
+</pre>
+
+##### Term::ReadKey
+
+`Term::ReadKey` is not a hard dependency in our case, *i.e.* po4a
+works nicely without it. But the po4a Debian package recommends
+`libterm-readkey-perl`, so it will probably be installed on most
+systems using the po plugin.
+
+If `$ENV{COLUMNS}` is not set, `Locale::Po4a::Common` uses
+`Term::ReadKey::GetTerminalSize()` to get the terminal size. How safe
+is this?
+
+Part of `Term::ReadKey` is written in C. Depending on the runtime
+platform, this function use ioctl, environment, or C library function
+calls, and may end up running the `resize` command (without
+arguments).
+
+IMHO, using Term::ReadKey has too far reaching implications for us to
+be able to guarantee anything wrt. security. Since it is anyway of no
+use in our case, I suggest we define `ENV{COLUMNS}` before loading
+`Locale::Po4a::Common`, just to be on the safe side. Joey?
+[[--intrigeri]]
+
+> Update: adding an option to disable `Text::WrapI18N`, as Nicolas
+> François suggested, would as a bonus disable `Term::ReadKey`
+> as well. [[--intrigeri]]
+
+### msgmerge
+
+`refreshpofiles()` runs this external program. A po4a developer
+answered he does "not expect any security issues from it".
 
 ### Fuzzing input
 
 
 ### Fuzzing input
 
@@ -278,10 +389,99 @@ I was not able to find any public information about gettext or po4a
 having been tested with a fuzzing program, such as `zzuf` or `fusil`.
 Moreover, some gettext parsers seem to be quite
 [easy to crash](http://fusil.hachoir.org/trac/browser/trunk/fuzzers/fusil-gettext),
 having been tested with a fuzzing program, such as `zzuf` or `fusil`.
 Moreover, some gettext parsers seem to be quite
 [easy to crash](http://fusil.hachoir.org/trac/browser/trunk/fuzzers/fusil-gettext),
-so it might be useful to bang gettext/po4a's heads against such
+so it might be useful to bang msgmerge/po4a's heads against such
 a program in order to easily detect some of the most obvious DoS.
 [[--intrigeri]]
 
 a program in order to easily detect some of the most obvious DoS.
 [[--intrigeri]]
 
+> po4a was not fuzzy-tested, but according to one of its developers,
+> "it would be really appreciated". [[--intrigeri]]
+
+Test conditions:
+
+- a 21M file containing 100 concatenated copies of all the files in my
+  `/usr/share/common-licenses/`; I had no existing PO file or
+  translated versions at hand, which renders these tests
+  quite incomplete.
+- po4a was the Debian 0.34-2 package; the same tests were also run
+  after replacing the `Text` module with the CVS one (the core was not
+  changed in CVS since 0.34-2 was released), without any significant
+  difference in the results.
+- Perl 5.10.0-16
+
+#### po4a-gettextize
+
+`po4a-gettextize` uses more or less the same po4a features as our
+`refreshpot` function.
+
+Without specifying an input charset, zzuf'ed `po4a-gettextize` quickly
+errors out, complaining it was not able to detect the input charset;
+it leaves no incomplete file on disk.
+
+So I had to pretend the input was in UTF-8, as does the po plugin.
+
+Two ways of crashing were revealed by this command-line:
+
+        zzuf -vc -s 0:100 -r 0.1:0.5 \
+          po4a-gettextize -f text -o markdown -M utf-8 -L utf-8 \
+            -m LICENSES >/dev/null
+
+They are:
+
+        Malformed UTF-8 character (UTF-16 surrogate 0xdcc9) in substitution iterator at /usr/share/perl5/Locale/Po4a/Po.pm line 1443.
+        Malformed UTF-8 character (fatal) at /usr/share/perl5/Locale/Po4a/Po.pm line 1443.
+
+and
+
+        Malformed UTF-8 character (UTF-16 surrogate 0xdcec) in substitution (s///) at /usr/share/perl5/Locale/Po4a/Po.pm line 1443.
+        Malformed UTF-8 character (fatal) at /usr/share/perl5/Locale/Po4a/Po.pm line 1443.
+
+Perl seems to exit cleanly, and an incomplete PO file is written on
+disk. I not sure whether if this is a bug in Perl or in `Po.pm`.
+
+> It's fairly standard perl behavior when fed malformed utf-8. As long as it doesn't
+> crash ikiwiki, it's probably acceptable. Ikiwiki can do some similar things itself when fed malformed utf-8 (doesn't crash tho) --[[Joey]]
+
+#### po4a-translate
+
+`po4a-translate` uses more or less the same po4a features as our
+`filter` function.
+
+Without specifying an input charset, same behaviour as
+`po4a-gettextize`, so let's specify UTF-8 as input charset as of now.
+
+        zzuf -cv \
+          po4a-translate -d -f text -o markdown -M utf-8 -L utf-8 \
+            -k 0 -m LICENSES -p LICENSES.fr.po -l test.fr
+
+... prints tons of occurences of the following error, but a complete
+translated document is written (obviously with some weird chars
+inside):
+
+        Use of uninitialized value in string ne at /usr/share/perl5/Locale/Po4a/TransTractor.pm line 854.
+        Use of uninitialized value in string ne at /usr/share/perl5/Locale/Po4a/TransTractor.pm line 840.
+        Use of uninitialized value in pattern match (m//) at /usr/share/perl5/Locale/Po4a/Po.pm line 1002.
+
+While:
+
+        zzuf -cv -s 0:10 -r 0.001:0.3 \
+          po4a-translate -d -f text -o markdown -M utf-8 -L utf-8 \
+            -k 0 -m LICENSES -p LICENSES.fr.po -l test.fr
+
+... seems to lose the fight, at the `readpo(LICENSES.fr.po)` step,
+against some kind of infinite loop, deadlock, or any similar beast.
+It does not seem to eat memory, though.
+
+Whatever format module is used does not change anything. This is thus
+probably a bug in po4a's core or in a lib it depends on.
+
+The sub `read`, in `TransTractor.pm`, seems to be a good debugging
+starting point.
+
+#### msgmerge
+
+`msgmerge` is run in our `refreshpofiles` function. I did not manage
+to crash it with `zzuf`.
+
 gettext/po4a rough corners
 --------------------------
 
 gettext/po4a rough corners
 --------------------------
 
@@ -292,28 +492,43 @@ gettext/po4a rough corners
   a PO update, that changes bla.fr.po in repo2; etc.; quickly fixed in
   `629968fc89bced6727981c0a1138072631751fee`, by disabling references
   in Pot files. Using `Locale::Po4a::write_if_needed` might be
   a PO update, that changes bla.fr.po in repo2; etc.; quickly fixed in
   `629968fc89bced6727981c0a1138072631751fee`, by disabling references
   in Pot files. Using `Locale::Po4a::write_if_needed` might be
-  a cleaner solution.
+  a cleaner solution. (warning: this function runs the external
+  `diff` program, have to check security)
 - new translations created in the web interface must get proper
   charset/encoding gettext metadata, else the next automatic PO update
   removes any non-ascii chars; possible solution: put such metadata
   into the Pot file, and let it propagate; should be fixed in
   `773de05a7a1ee68d2bed173367cf5e716884945a`, time will tell.
 
 - new translations created in the web interface must get proper
   charset/encoding gettext metadata, else the next automatic PO update
   removes any non-ascii chars; possible solution: put such metadata
   into the Pot file, and let it propagate; should be fixed in
   `773de05a7a1ee68d2bed173367cf5e716884945a`, time will tell.
 
-Misc. improvements
-------------------
+Better links
+------------
+
+### Page title in links
+
+Using the fix to
+[[bugs/pagetitle_function_does_not_respect_meta_titles]] from
+intrigeri's `meta` branch, the generated links' text is based on the
+page titles set with the [[meta|plugins/meta]] plugin. This has to be
+merged upstream, though.
+
+Page formats
+------------
 
 
-### page titles
+Markdown is well supported, great, but what about others?
 
 
-Use nice page titles from meta plugin in links, as inline already
-does. This is actually a duplicate for
-[[bugs/pagetitle_function_does_not_respect_meta_titles]], which might
-be fixed by something like [[todo/using_meta_titles_for_parentlinks]].
+The [[po|plugins/po]] uses `Locale::Po4a::Text` for every page format;
+this can be expected to work out of the box with most other wiki-like
+formats supported by ikiwiki. Some of their ad-hoc syntax might be
+parsed in a strange way, but the worst problems I can imagine would be
+wrapping issues; e.g. there is code in po4a dedicated to prevent
+re-wrapping the underlined Markdown headers.
 
 
-### source files format
+While it would be easy to better support formats such as [[html]] or
+LaTeX, by using for each one the dedicated po4a module, this can be
+problematic from a security point of view.
 
 
-Markdown is supported, great, but what about others? The set of file
-formats supported both in ikiwiki and po4a probably is greater than
-`{markdown}`.
+**TODO**: test the more popular formats and write proper documentation
+about it.
 
 Translation quality assurance
 -----------------------------
 
 Translation quality assurance
 -----------------------------
@@ -326,3 +541,67 @@ A new `cansave` type of hook would be needed to implement this.
 
 Note: committing to the underlying repository is a way to bypass
 this check.
 
 Note: committing to the underlying repository is a way to bypass
 this check.
+
+Creating new pages on the web
+-----------------------------
+
+See [[contrib/po|contrib/po]].
+
+Renaming pages
+--------------
+
+- Renaming a translation should be forbidden.
+
+Robustness tests
+----------------
+
+### Disabling the plugin
+
+- enabling the plugin with `po_translatable_pages` set
+- enabling the plugin without `po_translatable_pages` set: **OK**
+- disabling the plugin: **OK**
+
+### Changing the plugin config
+
+- adding existing pages to `po_translatable_pages`: **OK**
+- removing existing pages from `po_translatable_pages`: **OK**
+- adding a language to `po_slave_languages`: **OK**
+- removing a language from `po_slave_languages`: **OK**
+- changing `po_master_language`: **OK**
+- replacing `po_master_language` with a language previously part of
+  `po_slave_languages`: needs two rebuilds, but **OK** (this is quite
+  a perverse test actually)
+
+### Creating pages
+
+- creating a master page via RCS: **OK**
+- creating a master page via CGI: **OK**
+
+### Deleting pages
+
+- removing a master page via RCS: **OK**
+- removing a translation via RCS: **OK**
+- removing a master page via CGI: **OK**
+- removing a translation via CGI: **OK**
+
+### Renaming pages
+
+- renaming a master page via RCS: **OK** (but the old translations
+  are lost, because not all RCS track file renaming)
+- renaming a master page and its translations via RCS: **OK**
+- renaming a master page via CGI: **OK**
+- renaming a translation via RCS
+- renaming a translation via CGI
+
+### Misc
+
+- general test with `usedirs` disabled: **OK**
+- general test with `indexpages` enabled
+- general test with `po_link_to=default`
+
+Documentation
+-------------
+
+Maybe write separate documentation depending on the people it targets:
+translators, wiki administrators, hackers. This plugin may be complex
+enough to deserve this.