Subject: Re: Fix XML handling with DOCTYPE
To: Tom Lane <tgl@sss.pgh.pa.us>
References: 
 <CAN-V+g-6JqUQEQZ55Q3toXEN6d5Ez5uvzL4VR+8KtvJKj31taw@mail.gmail.com>
 <5C8D5D16.7090206@anastigmatix.net> <23319.1552769738@sss.pgh.pa.us>
 <5C8D6681.4070504@anastigmatix.net> <24203.1552771272@sss.pgh.pa.us>
 <5C8D79AF.7010204@anastigmatix.net> <5146.1552837534@sss.pgh.pa.us>
 <5C8E7FC0.8040401@anastigmatix.net> <31664.1552842991@sss.pgh.pa.us>
Cc: Ryan Lambert <ryan@rustprooflabs.com>, pgsql-hackers@postgresql.org
From: Chapman Flack <chap@anastigmatix.net>
Message-ID: <5C8E8E2F.9050600@anastigmatix.net>
Date: Sun, 17 Mar 2019 14:13:03 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.0
MIME-Version: 1.0
In-Reply-To: <31664.1552842991@sss.pgh.pa.us>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03/17/19 13:16, Tom Lane wrote:
> Chapman Flack <chap@anastigmatix.net> writes:
>> What I was doing in the patch is the reverse: parsing with the expectation
>> of CONTENT to see if a DTD gets tripped over. It isn't allowed for an
>> element to precede a DTD, so that approach can be expected to fail fast
>> if the other branch needs to be taken.
> 
> Ah, right.  I don't have any problem with trying the CONTENT approach
> before the DOCUMENT approach rather than vice-versa.  What I was concerned
> about was adding a lot of assumptions about exactly how libxml would
> report the failure.  IMO a maximally-safe patch would just rearrange
> things we're already doing without adding new things.
> 
>> But a quick pre-scan for the same thing would have the same property,
>> without the libxml dependencies that bother you here. Watch this space.
> 
> Do we need a pre-scan at all?

Without it, we double the time to a failure result in every case that
should actually fail, as well as in this one corner case that we want to
see succeed, and the question you posed earlier about which error message
to return becomes thornier.

If the query asked for CONTENT, any error result should be one you could get
when parsing as CONTENT. If we switch and try parsing as DOCUMENT _because
the input is claiming to have the form of a DOCUMENT_, then it's defensible
to return errors explaining why it's not a DOCUMENT ... but not in the
general case of just throwing DOCUMENT at it any time CONTENT parse fails.

Regards,
-Chap