| blob | 6161f80dfc1a19c2d1c5c88fe48945d64624fbcc |
1 Having tried out `field`, some comments (from [[smcv]]):
3 The general concept looks great.
5 The `pagetemplate` hook seems quite namespace-polluting: on a site containing
6 a list of books, I'd like to have an `author` field, but that would collide
7 with IkiWiki's use of `<TMPL_VAR AUTHOR>` for the author of the *page*
8 (i.e. me). Perhaps it'd be better if the pagetemplate hook was only active for
9 `<TMPL_VAR FIELD_AUTHOR>` or something? (For those who want the current
10 behaviour, an auxiliary plugin would be easy.)
12 > No, please. The idea is to be *able* to override field names if one wishes to, and choose, for yourself, non-colliding field names if one wishes not to. I don't wish to lose the power of being able to, say, define a page title with YAML format if I want to, or to write a site-specific plugin which calculates a page title, or other nifty things.
13 >It's not like one is going to lose the fields defined by the meta plugin; if "author" is defined by \[[!meta author=...]] then that's what will be found by "field" (provided the "meta" plugin is registered; that's what the "field_register" option is for).
14 >--[[KathrynAndersen]]
16 >> Hmm. I suppose if you put the title (or whatever) in the YAML, then
17 >> "almost" all the places in IkiWiki that respect titles will do the
18 >> right thing due to the pagetemplate hook, with the exception being
19 >> anything that has special side-effects inside `meta` (like `date`),
20 >> or anything that looks in `$pagestate{foo}{meta}` directly
21 >> (like `map`). Is your plan that `meta` should register itself by
22 >> default, and `map` and friends should be adapted to
23 >> work based on `getfield()` instead of `$pagestate{foo}{meta}`, then?
25 >>> Based on `field_get_value()`, yes. That would be my ideal. Do you think I should implement that as an ikiwiki branch? --[[KathrynAndersen]]
27 >>>> This doesn't solve cases where certain fields are treated specially; for
28 >>>> instance, putting a `\[[!meta permalink]]` on a page is not the same as
29 >>>> putting it in `ymlfront` (in the latter case you won't get your
30 >>>> `<link>` header), and putting `\[[!meta date]]` is not the same as putting
31 >>>> `date` in `ymlfront` (in the latter case, `%pagectime` won't be changed).
32 >>>>
33 >>>> One way to resolve that would be to have `ymlfront`, or similar, be a
34 >>>> front-end for `meta` rather than for `field`, and call
35 >>>> `IkiWiki::Plugin::meta::preprocess` (or a refactored-out function that's
36 >>>> similar).
37 >>>>
38 >>>> There are also some cross-site scripting issues (see below)... --[[smcv]]
40 >> (On the site I mentioned, I'm using an unmodified version of `field`,
41 >> and currently working around the collision by tagging books' pages
42 >> with `bookauthor` instead of `author` in the YAML.) --s
44 >> Revisiting this after more thought, the problem here is similar to the
45 >> possibility that a wiki user adds a `meta` shortcut
46 >> to [[shortcuts]], or conversely, that a plugin adds a `cpan` directive
47 >> that conflicts with the `cpan` shortcut that pages already use. (In the
48 >> case of shortcuts, this is resolved by having plugin-defined directives
49 >> always win.) For plugin-defined meta keywords this is the plugin
50 >> author's/wiki admin's problem - just don't enable conflicting plugins! -
51 >> but it gets scary when you start introducing things like `ymlfront`, which
52 >> allow arbitrary, wiki-user-defined fields, even ones that subvert
53 >> other plugins' assumptions.
54 >>
55 >> The `pagetemplate` hook is particularly alarming because page templates are
56 >> evaluated in many contexts, not all of which are subject to the
57 >> htmlscrubber or escaping; because the output from `field` isn't filtered,
58 >> prefixed or delimited, when combined with an arbitrary-key-setting plugin
59 >> like `ymlfront` it can interfere with other plugins' expectations
60 >> and potentially cause cross-site scripting exploits. For instance, `inline`
61 >> has a `pagetemplate` hook which defines the `FEEDLINKS` template variable
62 >> to be a blob of HTML to put in the `<head>` of the page. As a result, this
63 >> YAML would be bad:
64 >>
65 >> ---
66 >> FEEDLINKS: <script>alert('code injection detected')</script>
67 >> ---
68 >>
69 >> (It might require a different case combination due to implementation
70 >> details, I'm not sure.)
71 >>
72 >> It's difficult for `field` to do anything about this, because it doesn't
73 >> know whether a field is meant to be plain text, HTML, a URL, or something
74 >> else.
75 >>
76 >> If `field`'s `pagetemplate` hook did something more limiting - like
77 >> only emitting template variables starting with `field_`, or from some
78 >> finite set, or something - then this would cease to be a problem, I think?
79 >>
80 >> `ftemplate` and `getfield` don't have this problem, as far as I can see,
81 >> because their output is in contexts where the user could equally well have
82 >> written raw HTML directly; the user can cause themselves confusion, but
83 >> can't cause harmful output. --[[smcv]]
85 From a coding style point of view, the `$CamelCase` variable names aren't
86 IkiWiki style, and the `match_foo` functions look as though they could benefit
87 from being thin wrappers around a common `&IkiWiki::Plugin::field::match`
88 function (see `meta` for a similar approach).
90 I think the documentation would probably be clearer in a less manpage-like
91 and more ikiwiki-like style?
93 > I don't think ikiwiki *has* a "style" for docs, does it? So I followed the Perl Module style. And I'm rather baffled as to why having the docs laid out in clear sections... make them less clear. --[[KathrynAndersen]]
95 >> I keep getting distracted by the big shouty headings :-)
96 >> I suppose what I was really getting at was that when this plugin
97 >> is merged, its docs will end up split between its plugin
98 >> page, [[plugins/write]] and [[ikiwiki/PageSpec]]; on some of the
99 >> contrib plugins I've added I've tried to separate the docs
100 >> according to how they'll hopefully be laid out after merge. --s
102 If one of my branches from [[todo/allow_plugins_to_add_sorting_methods]] is
103 accepted, a `field()` cmp type would mean that [[plugins/contrib/report]] can
104 stop reimplementing sorting. Here's the implementation I'm using, with
105 your "sortspec" concept (a sort-hook would be very similar): if merged,
106 I think it should just be part of `field` rather than a separate plugin.
108 # Copyright © 2010 Simon McVittie, released under GNU GPL >= 2
109 package IkiWiki::Plugin::fieldsort;
110 use warnings;
111 use strict;
112 use IkiWiki 3.00;
113 use IkiWiki::Plugin::field;
115 sub import {
116 hook(type => "getsetup", id => "fieldsort", call => \&getsetup);
117 }
119 sub getsetup () {
120 return
121 plugin => {
122 safe => 1,
123 rebuild => undef,
124 },
125 }
127 package IkiWiki::SortSpec;
129 sub cmp_field {
130 if (!length $_[0]) {
131 error("sort=field requires a parameter");
132 }
134 my $left = IkiWiki::Plugin::field::field_get_value($_[0], $a);
135 my $right = IkiWiki::Plugin::field::field_get_value($_[0], $b);
137 $left = "" unless defined $left;
138 $right = "" unless defined $right;
139 return $left cmp $right;
140 }
142 1;
144 ----
146 Disclaimer: I've only looked at this plugin and ymlfront, not other related
147 stuff yet. (I quite like ymlfront, so I looked at this as its dependency. :)
148 I also don't want to annoy you with a lot of design discussion
149 if your main goal was to write a plugin that did exactly what you wanted.
151 My first question is: Why we need another plugin storing metadata
152 about the page, when we already have the meta plugin? Much of the
153 complication around the field plugin has to do with it accessing info
154 belonging to the meta plugin, and generalizing that to be able to access
155 info stored by other plugins too. (But I don't see any other plugins that
156 currently store such info). Then too, it raises points of confusion like
157 smcv's discuission of field author vs meta author above. --[[Joey]]
159 > The point is exactly in the generalization, to provide a uniform interface for accessing structured data, no matter what the source of it, whether that be the meta plugin or some other plugin.
161 > There were a few reasons for this:
163 >1. In converting my site over from PmWiki, I needed something that was equivalent to PmWiki's Page-Text-Variables (which is how PmWiki implements structured data).
164 >2. I also wanted an equivalent of PmWiki's Page-Variables, which, rather than being simple variables, are the return-value of a function. This gives one a lot of power, because one can do calculations, derive one thing from another. Heck, just being able to have a "basename" variable is useful.
165 >3. I noticed that in the discussion about structured data, it was mired down in disagreements about what form the structured data should take; I wanted to overcome that hurdle by decoupling the form from the content.
166 >4. I actually use this to solve (1), because, while I do use ymlfront, initially my pages were in PmWiki format (I wrote (another) unreleased plugin which parses PmWiki format) including PmWiki's Page-Text-Variables for structured data. So I needed something that could deal with multiple formats.
168 > So, yes, it does cater to mostly my personal needs, but I think it is more generally useful, also.
169 > --[[KathrynAndersen]]
171 >> Is it fair to say, then, that `field`'s purpose is to take other
172 >> plugins' arbitrary per-page data, and present it as a single
173 >> merged/flattened string => string map per page? From the plugins
174 >> here, things you then use that merged map for include:
175 >>
176 >> * sorting - stolen by [[todo/allow_plugins_to_add_sorting_methods]]
177 >> * substitution into pages with Perl-like syntax - `getfield`
178 >> * substitution into wiki-defined templates - the `pagetemplate`
179 >> hook
180 >> * substitution into user-defined templates - `ftemplate`
181 >>
182 >> As I mentioned above, the flattening can cause collisions (and in the
183 >> `pagetemplate` case, even security problems).
184 >>
185 >> I wonder whether conflating Page Text Variables with Page Variables
186 >> causes `field` to be more general than it needs to be?
187 >> To define a Page Variable (function-like field), you need to write
188 >> a plugin containing that Perl function; if we assume that `field`
189 >> or something resembling it gets merged into ikiwiki, then it's
190 >> reasonable to expect third-party plugins to integrate with whatever
191 >> scaffolding there is for these (either in an enabled-by-default
192 >> plugin that most people are expected to leave enabled, like `meta`
193 >> now, or in the core), and it doesn't seem onerous to expect each
194 >> plugin that wants to participate in this mechanism to have code to
195 >> do so. While it's still contrib, `field` could just have a special case
196 >> for the meta plugin, rather than the converse?
197 >>
198 >> If Page Text Variables are limited to being simple strings as you
199 >> suggest over in [[forum/an_alternative_approach_to_structured_data]],
200 >> then they're functionally similar to `meta` fields, so one way to
201 >> get their functionality would be to extend `meta` so that
202 >>
203 >> \[[!meta badger="mushroom"]]
204 >>
205 >> (for an unrecognised keyword `badger`) would store
206 >> `$pagestate{$page}{meta}{badger} = "mushroom"`? Getting this to
207 >> appear in templates might be problematic, because a naive
208 >> `pagetemplate` hook would have the same problem that `field` combined
209 >> with `ymlfront` currently does.
210 >>
211 >> One disadvantage that would appear if the function-like and
212 >> meta-like fields weren't in the same namespace would be that it
213 >> wouldn't be possible to switch a field from being meta-like to being
214 >> function-like without changing any wiki content that referenced it.
215 >>
216 >> Perhaps meta-like fields should just *be* `meta` (with the above
217 >> enhancement), as a trivial case of function-like fields? That would
218 >> turn `ymlfront` into an alternative syntax for `meta`, I think?
219 >> That, in turn, would hopefully solve the special-fields problem,
220 >> by just delegating it to meta. I've been glad of the ability to define
221 >> new ad-hoc fields with this plugin without having to write an extra plugin
222 >> to do so (listing books with a `bookauthor` and sorting them by
223 >> `"field(bookauthor) title"`), but that'd be just as easy if `meta`
224 >> accepted ad-hoc fields?
225 >>
226 >> --[[smcv]]
228 >>> Your point above about cross-site scripting is a valid one, and something I
229 >>> hadn't thought of (oops).
231 >>> I still want to be able to populate pagetemplate templates with field, because I
232 >>> use it for a number of things, such as setting which CSS files to use for a
233 >>> given page, and, as I said, for titles. But apart from the titles, I
234 >>> realize I've been setting them in places other than the page data itself.
235 >>> (Another unreleased plugin, `concon`, uses Config::Context to be able to
236 >>> set variables on a per-site, per-directory and a per-page basis).
238 >>> The first possible solution is what you suggested above: for field to only
239 >>> set values in pagetemplate which are prefixed with *field_*. I don't think
240 >>> this is quite satisfactory, since that would still mean that people could
241 >>> put un-scrubbed values into a pagetemplate, albeit they would be values
242 >>> named field_foo, etc. --[[KathrynAndersen]]
244 >>>> They can already do similar; `PERMALINK` is pre-sanitized to
245 >>>> ensure that it's a "safe" URL, but if an extremely confused wiki admin was
246 >>>> to put `COPYRIGHT` in their RSS/Atom feed's `<link>`, a malicious user
247 >>>> could put an unsafe (e.g. Javascript) URL in there (`COPYRIGHT` *is*
248 >>>> HTML-scrubbed, but "javascript:alert('pwned!')" is just text as far as a
249 >>>> HTML sanitizer is concerned, so it passes straight through). The solution
250 >>>> is to not use variables in situations where that variable would be
251 >>>> inappropriate. Because `field` is so generic, the definition of what's
252 >>>> appropriate is difficult. --[[smcv]]
254 >>> An alternative solution would be to classify field registration as "secure"
255 >>> and "insecure". Sources such as ymlfront would be insecure, sources such
256 >>> as concon (or the $config hash) would be secure, since they can't be edited
257 >>> as pages. Then, when doing pagetemplate substitution (but not ftemplate
258 >>> substitution) the insecure sources could be HTML-escaped.
259 >>> --[[KathrynAndersen]]
261 >>>> Whether you trust the supplier of data seems orthogonal to whether its value
262 >>>> is (meant to be) interpreted as plain text, HTML, a URL or what?
263 >>>>
264 >>>> Even in cases where you trust the supplier, you need to escape things
265 >>>> suitably for the context, not for security but for correctness. The
266 >>>> definition of the value, and the context it's being used in, changes the
267 >>>> processing you need to do. An incomplete list:
268 >>>>
269 >>>> * HTML used as HTML needs to be html-scrubbed if and only if untrusted
270 >>>> * URLs used as URLs need to be put through `safeurl()` if and only if
271 >>>> untrusted
272 >>>> * HTML used as plain text needs tags removed regardless
273 >>>> * URLs used as plain text are safe
274 >>>> * URLs or plain text used in HTML need HTML-escaping (and URLs also need
275 >>>> `safeurl()` if untrusted)
276 >>>> * HTML or plain text used in URLs need URL-escaping (and the resulting
277 >>>> URL might need sanitizing too?)
278 >>>>
279 >>>> I can't immediately think of other data types we'd be interested in beyond
280 >>>> text, HTML and URL, but I'm sure there are plenty.
282 >>>>> But isn't this a problem with anything that uses pagetemplates? Or is
283 >>>>> the point that, with plugins other than `field`, they all know,
284 >>>>> beforehand, the names of all the fields that they are dealing with, and
285 >>>>> thus the writer of the plugin knows which treatment each particular field
286 >>>>> needs? For example, that `meta` knows that `title` needs to be
287 >>>>> HTML-escaped, and that `baseurl` doesn't. In that case, yes, I see the problem.
288 >>>>> It's a tricky one. It isn't as if there's only ever going to be a fixed set of fields that need different treatment, either. Because the site admin is free to add whatever fields they like to the page template (if they aren't using the default one, that is. I'm not using the default one myself).
289 >>>>> Mind you, for trusted sources, since the person writing the page template and the person providing the variable are the same, they themselves would know whether the value will be treated as HTML, plain text, or a URL, and thus could do the needed escaping themselves when writing down the value.
291 >>>>> Looking at the content of the default `page.tmpl` let's see what variables fall into which categories:
293 >>>>> * **Used as URL:** BASEURL, EDITURL, PARENTLINKS->URL, RECENTCHANGESURL, HISTORYURL, GETSOURCEURL, PREFSURL, OTHERLANGUAGES->URL, ADDCOMMENTURL, BACKLINKS->URL, MORE_BACKLINKS->URL
294 >>>>> * **Used as part of a URL:** FAVICON, LOCAL_CSS
295 >>>>> * **Needs to be HTML-escaped:** TITLE
296 >>>>> * **Used as-is (as HTML):** FEEDLINKS, RELVCS, META, PERCENTTRANSLATED, SEARCHFORM, COMMENTSLINK, DISCUSSIONLINK, OTHERLANGUAGES->PERCENT, SIDEBAR, CONTENT, COMMENTS, TAGS->LINK, COPYRIGHT, LICENSE, MTIME, EXTRAFOOTER
298 >>>>> This looks as if only TITLE needs HTML-escaping all the time, and that the URLS all end with "URL" in their name. Unfortunately the FAVICON and LOCAL_CSS which are part of URLS don't have "URL" in their name, though that's fair enough, since they aren't full URLs.
300 >>>>> --K.A.
302 >>>> One reasonable option would be to declare that `field` takes text-valued
303 >>>> fields, in which case either consumers need to escape
304 >>>> it with `<TMPL_VAR FIELD_FOO ESCAPE=HTML>`, and not interpret it as a URL
305 >>>> without first checking `safeurl`), or the pagetemplate hook needs to
306 >>>> pre-escape.
308 >>>>> Since HTML::Template does have the ability to do ESCAPE=HTML/URL/JS, why not take advantage of that? Some things, like TITLE, probably should have ESCAPE=HTML all the time; that would solve the "to escape or not to escape" problem that `meta` has with titles. After all, when one *sorts* by title, one doesn't really want HTML-escaping in it; only when one uses it in a template. -- K.A.
310 >>>> Another reasonable option would be to declare that `field` takes raw HTML,
311 >>>> in which case consumers need to only use it in contexts that will be
312 >>>> HTML-scrubbed (but it becomes unsuitable for using as text - problematic
313 >>>> for text-based things like sorting or URLs, and not ideal for searching).
314 >>>>
315 >>>> You could even let each consumer choose how it's going to use the field,
316 >>>> by having the `foo` field generate `TEXT_FOO` and `HTML_FOO` variables?
317 >>>> --[[smcv]]
319 >>>>> Something similar is already done in `template` and `ftemplate` with the `raw_` prefix, which determines whether the variable should have `htmlize` run over it first before the value is applied to the template. Of course, that isn't scrubbing or escaping, because with those templates, the scrubbing is done afterwards as part of the normal processing.
321 >>> Another problem, as you point out, is special-case fields, such as a number of
322 >>> those defined by `meta`, which have side-effects associated with them, more
323 >>> than just providing a value to pagetemplate. Perhaps `meta` should deal with
324 >>> the side-effects, but use `field` as an interface to get the values of those special fields.
326 >>> --[[KathrynAndersen]]
328 -----
330 I think the main point is: what is (or should be) the main point of the
331 field plugin? If it's essentially a way to present a consistent
332 interface to access page-related structured information, then it makes
333 sense to have it very general. Plugins registering with fields would
334 then present ways for recovering the structure information from the page
335 (`ymlfront`, `meta`, etc), ways to manipulate it (like `meta` does),
336 etc.
338 In this sense, security should be entirely up to the plugins, although
339 the fields plugin could provide some auxiliary infrastructure (like
340 determining where the data comes from and raise or lower the security
341 level accoringly).
343 Namespacing is important, and it should be considered at the field
344 plugin interface level. A plugin should be able to register as
345 responsible for the processing of all data belonging to a given
346 namespace, but plugins should be able to set data in any namespace. So
347 for example, `meta` register are `meta` fields processing, and whatever
348 method is used to set the data (`meta` directive, `ymlfront`, etc) it
349 gets a say on what to do with data in its namespace.
351 What I'm thinking of is something you could call fieldsets. The nice
352 thing about them is that, aside from the ones defined by plugins (like
353 `meta`), it would be possible to define custom ones (with a generic,
354 default processor) in an appropriate file (like smileys and shortcuts)
355 with a syntax like:
357 [[!fieldset book namespace=book
358 fields="author title isbn"
359 fieldtype="text text text"]]
361 after which, you coude use
363 [[!book author="A. U. Thor"
364 title="Fields of Iki"]]
366 and the data would be available under the book namespace, and thus
367 as BOOK_AUTHOR, BOOK_TITLE etc in templates.
369 Security, in this sense, would be up to the plugin responsible for the
370 namespace processing (the default handler would HTML-escape text fields
371 scrub, html fields, safeurl()ify url fields, etc.)
373 > So, are you saying that getting a field value is sort of a two-stage process? Get the value from anywhere, and then call the "security processor" for that namespace to "secure" the value? I think "namespaces" are really orthogonal to this issue. What the issue seems to be is:
375 * what form do we expect the raw field to be in? (text, URL, HTML)
376 * what form do we expect the "secured" output to be in? (raw HTML, scrubbed HTML, escaped HTML, URL)
378 > Only if we know both these things will we know what sort of security processing needs to be done.
380 >> Fieldsets are orthogonal to the security issue in the sense that you can use
381 >> them without worrying about the field security issue, but they happen to be
382 >> a rather clean way of answering those two questions, by allowing you to
383 >> attach preprocessing attributes to a field in a way that the user
384 >> (supposedly) cannot mingle with.
386 > There is also a difference between field values that are used inside pagetemplate, and field values which are used as part of a page's content (e.g. with ftemplate). If you have a TITLE, you want it to be HTML-escaped if you're using it inside pagetemplate, but you don't want it to be HTML-escaped if you're using it inside a page's content. On the other hand, if you have, say, FEEDLINKS used inside pagetemplate, you don't wish it to be HTML-escaped at all, or your page content will be completely stuffed.
388 >> Not to talk about the many different ways date-like fields might be need
389 >> processing. It has already been proposed to solve this problem by exposing
390 >> the field values under different names depending on the kind or amout of
391 >> postprocessing they had (e.g. RAW_SOMEFIELD, SOMEFIELD, to which we could add
392 >> HTML_SOMEFIELD, URL_SOMEFIELD or whatever). Again, fieldsets offer a simple way
393 >> of letting Ikiwiki know what kind of postprocessing should be offered for
394 >> that particular field.
396 > So, somehow, we have to know the meaning of a field before we can use it properly, which kind of goes against the idea of having something generic.
398 >> We could have a default field type (text, for example), and a way to set a
399 >> different field type (which is what my fieldset proposal was about).
401 > --[[KathrynAndersen]]
403 -----
405 I was just looking at HTML5 and wondered if the field plugin should generate the new Microdata tags (as well as the internal structures)? <http://slides.html5rocks.com/#slide19> -- [[Will]]
407 > This could just as easily be done as a separate plugin. Feel free to do so. --[[KathrynAndersen]]