Hi!
The XML output of explain potentially outputs the XML tag names "I/O-Write-Time"
and "I/O-Read-Time", which are invalid due to the slash.
This can easily be seen with this:
set track_io_timing = on;
explain (analyze true, buffers true, format xml) select 1;
[...]
<I/O-Read-Time>0.000</I/O-Read-Time>
<I/O-Write-Time>0.000</I/O-Write-Time>
[...]
Attached is a patch against master that translates slashes to dashes during XML
formatting (very much like spaces are already translated to dashes).
Removing the slash from those property names is another option, but is an
incompatible change to the other formats (neither JSON nor YAML have issues
with '/‘ in key names).
Although the patch fixes the problem for the moment, it is incomplete in that
sense that it continues to check against an incomplete black list. I guess
this is how it slipped in: XML explain was added in 9.0, I/O timings in 9.2.
Checking against an (abbreviated?) white list would be more future proof if new
explain properties are added. Let me know if you consider this a better approach.
I've also done a simple check to see if there are other dangerous
characters used in explain properties at the moment:
sed -n 's/.*ExplainProperty[^(]*(\s*"\([^"]*\)\".*/\1/p' src/backend/commands/explain.c |grep '[^-A-Za-z /]'
Result: none.
A similar check could be used at build-time to prevent introducing new property
names that invalidate the XML output (not sure if this could ever reach 100%
safety).
Comments?
--
Markus Winand - winand.at
