On 4 July 2012 10:18, Oleg Kalnichevski <[email protected]> wrote: > On Tue, 2012-07-03 at 16:39 +0100, sebb wrote: >> On 3 July 2012 16:04, Oleg Kalnichevski <[email protected]> wrote: >> > On Tue, 2012-06-26 at 17:55 +0100, sebb wrote: >> >> On 26 June 2012 14:33, sebb <[email protected]> wrote: >> >> > On 26 June 2012 13:43, Oleg Kalnichevski <[email protected]> wrote: >> >> >> On Tue, 2012-06-26 at 12:13 +0100, sebb wrote: >> >> >>> The setQuery(String) method currently expects an escaped, ASCII-only >> >> >>> string - basically encoded form data - whereas all the other set >> >> >>> methods expect unescaped input. >> >> >>> This is a bit confusing. >> >> >>> >> >> >> >> >> >> Yes, I am aware of it. I was going to deprecate this method in 4.3 and >> >> >> replace it with something like #parseQuery. >> >> >> >> >> >>> AFAIK, a query string does not have to consist of name value pairs >> >> >>> (i.e. form data), it can be any arbitrary string. >> >> >>> There's currently no way for the end-user to provide such a query. >> >> >>> They would have to use the URI class. >> >> >> >> >> >> In this case it is basically a lone parameter without a value. >> >> > >> >> > Yes and no; depends what is allowed for a parameter name. >> >> >> >> The URI class allows spaces in the query segment. >> >> AFAICT this is currently impossible to replicate using URIBuilder, >> >> because it converts space to '+'. >> >> >> >> > Also, '+' only means space in the context of form data; otherwise it >> >> > is just another safe character in the query. >> >> > >> >> > Presumably applications that use arbitrary query strings decode the >> >> > query differently from ones that expect form data. >> >> > >> >> >>> Maybe that is OK, in which case we just need to clarify the URIBuilder >> >> >>> Javadoc to state that it is only intended for use with form data >> >> >>> queries. >> >> >>> >> >> >>> Perhaps there should be a setQuery(NVP) method which handles form >> >> >>> data directly? >> >> >>> >> >> >> >> >> >> Makes sense. The question is whether we should add this method in 4.2.1 >> >> >> or wait until 4.3. >> >> > >> >> > It could wait. >> >> > >> >> > As could providing support for non-form data query strings; the URI >> >> > class can be used meanwhile. >> >> >> > >> > Sebastian >> > >> > I deprecated #setQuery method and added new method to set arbitrary >> > custom query component. Please review. >> > >> > http://svn.apache.org/viewvc?rev=1356775&view=rev >> >> Mostly OK. >> >> I'm not quite sure why query and queryParams are both used to build >> the same URI query >> [Equally, why setCustomQuery does not set queryParams=null.] >> >> Is there a use case that requires both a custom query and query params? >> If so, then there should be a test case for it. >> > > URI uri = new URIBuilder() > .setCustomQuery("this") > .addParameter("that", null).build(); > System.out.println(uri); > > What do you see as an expected result of this operation? > '?this&that' or '?that'?
I'm not sure the sequence makes sense, so perhaps the code should throw a runtime error. A custom query string is not form data, so space should be encoded as '%20' rather than '+'. > Oleg > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
