WIP: Fix invalid XML explain plans for track_io_timing

From: Markus Winand <markus(dot)winand(at)winand(dot)at>
To: pgsql-hackers(at)postgresql(dot)org
Subject: WIP: Fix invalid XML explain plans for track_io_timing
Date: 2016-10-20 15:00:20
Message-ID: E0BF6A45-68E8-45E6-918F-741FB332C6BB@winand.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Fix-invalid-XML-explain-plans-for-track_io_timing.patch application/octet-stream 1.4 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-10-20 15:11:50 Re: Renaming of pg_xlog and pg_clog
Previous Message Grigory Smolkin 2016-10-20 14:58:23 Re: File content logging during execution of COPY queries