doc/todo/osm_plugin_icon_patch.mdwn

   1 [[!template  id=gitbranch branch=cbaines/osm-icon-fixes author="[[cbaines]]"]]
   2 [[!tag patch]]
   3
   4 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.
   5
   6 > 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]]
   7
   8 >     +    my $tag = $params{'tag'};
   9 >
  10 > Please check indentation: you're mixing spaces and hard tabs, apparently
  11 > with the assumption that a tab is worth 4 spaces.
  12 >
  13 >     - my $icon = $config{'osm_default_icon'} || "ikiwiki/images/osm.png"; # sanitized: we trust $config
  14 >     + my $icon = $params{'icon'}; # sanitized: we trust $config
  15 >
  16 > So there's a comment there that explains why the value of `$icon` can
  17 > be trusted, but it is no longer true, because it no longer comes from
  18 > `$config`. This does not fill me with confidence. Maybe it's OK to use
  19 > a wiki-editor-supplied icon, maybe not. If it is OK, please justify why,
  20 > and in any case, please do not leave old comments if they are no longer
  21 > true.
  22 >
  23 > In this case I suspect editors may be able to specify an icon whose URL is
  24 > `javascript:alert("cross-site scripting!")` (or something more malicious)
  25 > and have it written into the KML as-is. The osm plugin has had cross-site
  26 > scripting vulnerabilities before, I don't want to add another.
  27 >
  28 >     +                 externalGraphic: "${icon}"
  29 >
  30 > I don't think Perl variable interpolation is going to work in Javascript?
  31 > I suspect this should have been inserting something into the GeoJSON instead?
  32 >
  33 > --[[smcv]]
  34
  35 >> I have now fixed the indentation issues.
  36 >>
  37 >> I have changed the comment relating to the icon parameter, but I don't
  38 >> really understand how ikiwiki handles sanitisation, so I have not changed
  39 >> anything else for this.
  40 >>
  41 >> As for the Perl variable interpolation, see this
  42 >> [documentation](http://docs.openlayers.org/library/feature_styling.html#attribute-replacement-syntax).
  43 >>
  44 >> -- [[cbaines]]