Re: It seems LO doesn't handle correctly complex HTTP queries

On 10/24/2016 03:19 PM, Giuseppe Castagno wrote:
> working on bug
> <https://bugs.documentfoundation.org/show_bug.cgi?id=99499>
>
> I came across a couple of complex HTTP queries that IMHO LO doesn't
> handle correctly.
>
> One query full URI is:
> <http://www.digikey.com/web%20export/common/mkt/en/help.png?requestedName=help?requestedName=help?requestedName=help?requestedName=help?requestedName=help>
>
>
> while LO WebDAV percent-encodes into:
> <http://www.digikey.com/web%20export/common/mkt/en/help.png%3FrequestedName=help%3FrequestedName=help%3FrequestedName=help%3FrequestedName=help?requestedName=help>
>
>
> That is, LO detects a HTTP query start at the first '?' from the end of
> the URI and not at the first '?' after the last segment of the path, as
> it should be according to
> <https://tools.ietf.org/html/rfc3986#section-3>, and
> <https://tools.ietf.org/html/rfc3986#section-3.4>.
>
> Furthermore, it doesn't seems to encode correctly the query body, but I
> still have to dig deeper for that.
>
> I think that the WebDAV UCP provider should not change the HTTP query it
> receives, but the query should be correctly set and percent-encoded by
> the client application (e.g. framework).
>
> This means that the '?' character should be percent-encoded as %3F into
> the path part of the URI before being presented to WebDAV provider, this
> to prevent possibly wrong interpretation.
>
> I'd like to change the way the WebDAV provider deals with HTTP query,
> that implies the correct percent-encoding of '?' into %3F in UCB
> provider clients.
> The query re-encoding is done here:
> <http://opengrok.libreoffice.org/xref/core/ucb/source/ucp/inc/urihelper.hxx#45>

ucb_impl::urihelper::encodeURI (ucb/source/ucp/inc/urihelper.hxx) indeed 
looks very broken; it should not be used.  (If there is real need to 
"fix" URIs, INetURLObject has capabilities that are probably less broken 
than those of this encodeURI.)

Apart from the use of encodeURI in 
ucb/source/ucp/webdav-neon/NeonUri.cxx, the four uses in 
ucb/source/ucp/webdav-neon/DAVTypes.cxx of the form

  ucb_impl::urihelper::encodeURI( NeonUri::unescape( rURL ) )

look especially worrying---NeonUri::unescape is apparently meant to 
unescape individual URL path segments; applying it to a complete URL 
breaks things.