<div dir="ltr">Incredible!!!!<div>Really cool initiative. </div><div><br></div><div>I had a play around and wanted a way to be able to write the pixel function in a 'stand alone' python module and generate the VRT from it. This would allow independent testing and easier maintenance of the python code. It is fairly easy using lxml to build up the VRT dynamically:</div><div><br></div><div><a href="https://github.com/JamesRamm/gdal_pixel_functions">https://github.com/JamesRamm/gdal_pixel_functions</a><br></div><div><br></div><div>In the example I more-or-less recreate the hillshade example Even posted, although there is a lot of functionality missing from the VRT-builder still, so some nodes are not present. </div><div>It would be quite easy therefore, to expand this to a sort of GDAL 'algorithms library' (I think it can wholesale replace what I had tried here: <a href="https://github.com/JamesRamm/GeoAlg">https://github.com/JamesRamm/GeoAlg</a>).</div><div><br></div><div>One thing I would note (if I understand correctly) is that you have specified any "import" statement as unsafe except numpy/math and numba? It might be better to either allow imports or not allow them at all (unless the config option is set) as I don't see why those libraries would be any more or any less 'safe' than another library...e.g. another standard library module.</div><div><br></div><div>I would agree with Sean and remove 'IF_SAFE' and leave it entirely to the users discretion. Many users who are working with their own local data will likely have no problems about just setting it to 'YES' and I imagine users for whom it would be a probably might already have their own rules on what constitutes 'safe'?</div><div><br></div><div>On a sidenote - does this new functionality overlap considerably with the gdal map algebra project?</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 12 September 2016 at 19:25, Even Rouault <span dir="ltr"><<a href="mailto:even.rouault@spatialys.com" target="_blank">even.rouault@spatialys.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> I found <a href="http://nedbatchelder.com/blog/201206/eval_really_is_dangerous.html" rel="noreferrer" target="_blank">http://nedbatchelder.com/blog/<wbr>201206/eval_really_is_<wbr>dangerous.html</a><br>
> to be a good intro to the risks of eval'ing untrusted Python code.<br>
> Mentioned in there is a notable attempt to make a secure subset of Python<br>
> called "pysandbox", but its developer has since declared it "broken by<br>
> design": <a href="https://lwn.net/Articles/574215/" rel="noreferrer" target="_blank">https://lwn.net/Articles/<wbr>574215/</a>. I'm not knowledgeable enough<br>
> about sandboxing (OS or otherwise) to say if that's right.<br>
<br>
</span>Those links are fascinating. I wouldn't have imagined that there was valid<br>
Python code that could crash the Python interpreter, almost by design ! (so<br>
the osgeo.gdal gotchas are not so bad after all :-) )<br>
OK, so I've followed your suggestion:<br>
* IF_SAFE mode is removed (actually between #if 0 / #endif if someone wants to<br>
pursue the sandboxing effort),<br>
* and the default of GDAL_VRT_ENABLE_PYTHON is now NO.<br>
<span class=""><br>
><br>
> I see that in GDAL 2.0+ we can set options in the VRT XML itself. Is it<br>
> possible to set GDAL_VRT_ENABLE_PYTHON=YES in a VRT and thus override the<br>
> reader's own trust policies?<br>
<br>
</span>No, that's not possible. GDAL_VRT_ENABLE_PYTHON is a config option only, so can<br>
only be set as a env variable or through CPLSetConfigOption().<br>
<br>
We could imagine to provide it as an option of the VRT indeed, but then there<br>
would be interesting situations like the following. Imagine something called<br>
"my.tif" with the following content :<br>
<span class=""><br>
<VRTDataset rasterXSize="20" rasterYSize="20"><br>
</span><span class=""> <VRTRasterBand dataType="Byte" band="1"><br>
</span> <ColorInterp>Gray</<wbr>ColorInterp><br>
<SimpleSource><br>
<SourceFilename relativeToVRT="1"><![CDATA[<br>
<VRTDataset .... evil Python code here ... .... >]]></SourceFilename><br>
<OpenOptions><br>
<OOI key="ENABLE_PYTHON">YES</OOI><br>
</OpenOptions><br>
</SimpleSource><br>
</VRTRasterBand><br>
</VRTDataset><br>
<br>
The internalized VRT would then be allowed with ENABLE_PYTHON=YES.<br>
<br>
So it's probably a good idea to not provide an open option for now. One could<br>
perhaps imagine to introduce one, provided we make it illegal as an item in<br>
<OpenOptions>, so it can only be provided by "top-level" GDALOpenEx() calls.<br>
<span class=""><br>
> My ignorance of how GDAL separates "open"<br>
> options from "config" options might be on display in this question.<br>
<br>
</span>They are just 2 different mechanisms of providing options. There's no automatic<br>
bridge between both concepts. Sometimes some option may exist in both worlds<br>
(because historically there was only global config option, but in some use<br>
cases, some options mostly make sense per dataset) or in just one of them.<br>
<div class="HOEnZb"><div class="h5"><br>
--<br>
Spatialys - Geospatial professional services<br>
<a href="http://www.spatialys.com" rel="noreferrer" target="_blank">http://www.spatialys.com</a><br>
______________________________<wbr>_________________<br>
gdal-dev mailing list<br>
<a href="mailto:gdal-dev@lists.osgeo.org">gdal-dev@lists.osgeo.org</a><br>
<a href="http://lists.osgeo.org/mailman/listinfo/gdal-dev" rel="noreferrer" target="_blank">http://lists.osgeo.org/<wbr>mailman/listinfo/gdal-dev</a></div></div></blockquote></div><br></div>