aboutsummaryrefslogtreecommitdiff
path: root/doc/todo/osm_plugin_icon_patch.mdwn
blob: 58df0eea2939cc68a1be8d93976bd76cea722a44 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
[[!template  id=gitbranch branch=cbaines/osm-icon-fixes author="[[cbaines]]"]]
[[!tag patch]]

Currently, the documented icon parameter to the waypoint directive is not used. This patch fixes that, and fixes some related problems in the KML output. 

> That patch looks pretty awesome, thanks for your work on it. I don't have time to test it now, but if it works, I am all for its inclusion. --[[anarcat]]

>     +    my $tag = $params{'tag'};
>
> Please check indentation: you're mixing spaces and hard tabs, apparently
> with the assumption that a tab is worth 4 spaces.
>
>     -	my $icon = $config{'osm_default_icon'} || "ikiwiki/images/osm.png"; # sanitized: we trust $config
>     +	my $icon = $params{'icon'}; # sanitized: we trust $config
>
> So there's a comment there that explains why the value of `$icon` can
> be trusted, but it is no longer true, because it no longer comes from
> `$config`. This does not fill me with confidence. Maybe it's OK to use
> a wiki-editor-supplied icon, maybe not. If it is OK, please justify why,
> and in any case, please do not leave old comments if they are no longer
> true.
>
> In this case I suspect editors may be able to specify an icon whose URL is
> `javascript:alert("cross-site scripting!")` (or something more malicious)
> and have it written into the KML as-is. The osm plugin has had cross-site
> scripting vulnerabilities before, I don't want to add another.
>
>     +            	externalGraphic: "${icon}"
>
> I don't think Perl variable interpolation is going to work in Javascript?
> I suspect this should have been inserting something into the GeoJSON instead?
>
> --[[smcv]]

>> I have now fixed the indentation issues.
>>
>> I have changed the comment relating to the icon parameter, but I don't
>> really understand how ikiwiki handles sanitisation, so I have not changed
>> anything else for this.
>>
>> As for the Perl variable interpolation, see this
>> [documentation](http://docs.openlayers.org/library/feature_styling.html#attribute-replacement-syntax).
>>
>> -- [[cbaines]]