From: Jeff Smith Subject: Re: [PATCH v3 2/3] xmllite: Whitespace node not returned when followed by invalid character. Message-Id: Date: Fri, 6 Dec 2019 15:22:46 -0600 In-Reply-To: <6a7459e5-eb39-a426-4370-abb2e0a4a067@codeweavers.com> References: <20191205195326.928106-1-whydoubt@gmail.com> <20191205195326.928106-2-whydoubt@gmail.com> <6a7459e5-eb39-a426-4370-abb2e0a4a067@codeweavers.com> On Fri, Dec 6, 2019 at 11:13 AM Nikolay Sivov wrote: > > On 12/5/19 10:53 PM, Jeff Smith wrote: > > Signed-off-by: Jeff Smith > > --- > > dlls/xmllite/reader.c | 12 ++++++++++-- > > dlls/xmllite/tests/reader.c | 2 -- > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/dlls/xmllite/reader.c b/dlls/xmllite/reader.c > > index eddc4d8eec..79e5c2253a 100644 > > --- a/dlls/xmllite/reader.c > > +++ b/dlls/xmllite/reader.c > > @@ -1113,8 +1113,8 @@ static inline UINT reader_get_cur(xmlreader *reader) > > static inline WCHAR *reader_get_ptr(xmlreader *reader) > > { > > encoded_buffer *buffer = &reader->input->buffer->utf16; > > - WCHAR *ptr = (WCHAR*)buffer->data + buffer->cur; > > - if (!*ptr) reader_more(reader); > > + if (buffer->cur*sizeof(WCHAR) >= buffer->written) > > + reader_more(reader); > > return (WCHAR*)buffer->data + buffer->cur; > > } Hi Nikolay, > Why do you need to change that? It's used everywhere. Since the test is fixed even without this, I will probably take this chunk out of this patch set. > > > > @@ -1714,8 +1714,16 @@ static HRESULT reader_parse_whitespace(xmlreader *reader) > > { > > strval value; > > UINT start; > > + const encoded_buffer *buffer = &reader->input->buffer->utf16; > > > > reader_skipspaces(reader); > > + > > + /* Do NOT return Whitespace node if followed by a character other than '<'. > > + * The reader_skipspaces call should have already read in the character. */ > > + if (buffer->cur*sizeof(WCHAR) < buffer->written && > > + *reader_get_ptr2(reader, buffer->cur) != '<') > > + return WC_E_SYNTAX; > > + > Buffer access should not be exposed like that. OK, I will try to improve where that is handled. Though that could potentially entail changing more things that are used everywhere. Regards, Jeff