]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/blobdiff - doc/todo/osm_plugin_icon_patch.mdwn
Added a comment
[git.ikiwiki.info.git] / doc / todo / osm_plugin_icon_patch.mdwn
index 947fa5e0a12ab004a3b143079cf65d40506e302a..58df0eea2939cc68a1be8d93976bd76cea722a44 100644 (file)
@@ -3,4 +3,42 @@
 
 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. 
 
 
 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. It does seem that it overlaps with [[bugs/osm_plugin_error_TypeError:_mapProjection_is_null]] however, maybe you'd want to separate the patches for clarity... --[[anarcat]]
+> 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]]