]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/blobdiff - doc/plugins/po.mdwn
po: more security-related reports
[git.ikiwiki.info.git] / doc / plugins / po.mdwn
index d27b5af1d78509ca236c8d4a160c98342cceeb98..4350e1a7311b36ae77943619afb186bd2750ede6 100644 (file)
@@ -198,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.
 
-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.
 
+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
 -----------
 
@@ -215,6 +219,22 @@ interface could also be implemented at some point).
 If [[tips/untrusted_git_push]] is setup, one can edit the PO files in one's
 preferred `$EDITOR`, without needing to be online.
 
+Markup languages support
+------------------------
+
+Markdown is well supported. Some other markup languages supported by
+ikiwiki mostly work, but some pieces of syntax are not rendered
+correctly on the slave pages:
+
+* [[reStructuredText|rst]]: anonymous hyperlinks and internal
+  cross-references
+* [[wikitext]]: conversion of newlines to paragraphs
+* [[creole]]: verbatim text is wrapped, tables are broken
+* [[html]] and LaTeX: not supported yet; the dedicated po4a modules
+  could be used to support them, but they would need a security audit
+* other markup languages have not been tested.
+
+
 TODO
 ====
 
@@ -281,8 +301,6 @@ an initial goal, and analysing in detail the possible issues.
   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
@@ -299,24 +317,21 @@ means the `Text` module only.
   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`.
+
+#### To be checked
+
 ##### 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]]
+> I proposed a patch based on Joey's to po4a-devel, allowing to fully
+> disable this module's use. When it is merged upstream, we'll need to add
+> `use Locale::Po4a::Common qw(nowrapi18n)` to `po.pm`, before loading
+> any other `Locale::Po4a` module. A versioned dependency may be needed.
+> --[[intrigeri]]
 
 ##### Term::ReadKey
 
@@ -325,42 +340,29 @@ 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?
+`Term::ReadKey` has too far reaching implications for us to
+be able to guarantee anything wrt. security.
 
-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).
+> The option that disables `Text::WrapI18N` also disables
+> `Term::ReadKey` as a consequence. [[--intrigeri]]
 
-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]]
+### msgmerge
 
-> Update: adding an option to disable `Text::WrapI18N`, as Nicolas
-> François suggested, would as a bonus disable `Term::ReadKey`
-> as well. [[--intrigeri]]
+`refreshpofiles()` runs this external program.
 
-### msgmerge
+A po4a developer answered he does "not expect any security issues from
+it". I did not manage to crash it with `zzuf`, nor was able to find
+any past security holes.
 
-`refreshpofiles()` runs this external program. A po4a developer
-answered he does "not expect any security issues from it".
+### msgfmt
 
-### Fuzzing input
+`isvalidpo()` runs this external program.
 
-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),
-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]]
+* I could not manage to make it behave badly using zzuf, it exits
+  cleanly when too many errors are detected.
+* I could not find any past security holes.
 
-> po4a was not fuzzy-tested, but according to one of its developers,
-> "it would be really appreciated". [[--intrigeri]]
+### Fuzzing input
 
 Test conditions:
 
@@ -404,6 +406,9 @@ and
 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
@@ -432,18 +437,9 @@ While:
 
 ... 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`.
+The root of this bug lies in `Text::WrapI18N`, see above for
+possible solutions.
 
 gettext/po4a rough corners
 --------------------------
@@ -463,65 +459,51 @@ gettext/po4a rough corners
   into the Pot file, and let it propagate; should be fixed in
   `773de05a7a1ee68d2bed173367cf5e716884945a`, time will tell.
 
-When committing a translatable page to the repository, msgmerge
-happens and then:
-
-        Use of uninitialized value $page in pattern match (m//) at /usr/local/share/perl/5.10.0/IkiWiki.pm line 624.
-        Use of uninitialized value $p in hash element at /usr/local/share/perl/5.10.0/IkiWiki/Render.pm line 423.
-        Use of uninitialized value $file in string eq at /usr/local/share/perl/5.10.0/IkiWiki/Render.pm line 444.
-        Use of uninitialized value $page in pattern match (m//) at /usr/local/share/perl/5.10.0/IkiWiki.pm line 624.
-
-And then tons of:
-        Use of uninitialized value $page in pattern match (m//) at /usr/local/share/perl/5.10.0/IkiWiki.pm line 1860.
-        Use of uninitialized value $page in concatenation (.) or string at /usr/local/share/perl/5.10.0/IkiWiki.pm line 1869.
-
-Page titles in links
---------------------
-
-To use the page titles set with the [meta](plugins/meta) plugin when
-rendering links would be very much nicer, than the current
-"filename.LL" format. This is actually a duplicate for
-[[bugs/pagetitle_function_does_not_respect_meta_titles]].
-
-Page formats
+Better links
 ------------
 
-Markdown is well supported, great, but what about others?
+### Page title in links
 
-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.
+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 into ikiwiki upstream, though.
 
-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.
+Robustness tests
+----------------
 
-**TODO**: test the more popular formats and write proper documentation
-about it.
+### Enabling/disabling the plugin
 
-Translation quality assurance
------------------------------
+- enabling the plugin with `po_translatable_pages` set
+- enabling the plugin without `po_translatable_pages` set: **OK**
+- disabling the plugin: **OK**
 
-Modifying a PO file via the CGI must be forbidden if the new version
-is not a valid PO file. As a bonus, check that it provides a more
-complete translation than the existing one.
+### Changing the plugin config
 
-A new `cansave` type of hook would be needed to implement this.
+- 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)
 
-Note: committing to the underlying repository is a way to bypass
-this check.
+### Creating/deleting/renaming pages
 
-Broken links
-------------
+All cases of master/slave page creation/deletion/rename, both via RCS
+and via CGI, have been tested.
+
+### Misc
 
-See [[contrib/po]].
+- 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 is maybe
-complex enough to deserve this.
+translators, wiki administrators, hackers. This plugin may be complex
+enough to deserve this.