budjb/http-requests

Some Basic URL Encoding?

Closed this issue · 2 comments

Would it be possible to provide some level of basic URL Encoding for things like spaces?

The following code-snippet of a Grails Controller action (using http-requests-grails:1.0.2) will demonstrate the error:

def index() {
        Slf4jLoggingFilter slf4jLoggingFilter = new Slf4jLoggingFilter()
        slf4jLoggingFilter.setLogger(log)

        HttpClient httpClient = httpClientFactory.createHttpClient()
        httpClient.clearFilters()
        httpClient.addFilter(slf4jLoggingFilter)
        httpClient.addFilter(new HttpStatusExceptionFilter())

        HttpRequest httpRequest = new HttpRequest('http://127.0.0.1:8080/foo bar')

        httpClient.get(httpRequest)
}

For convenience, I would expect the URL to be encoded as http://127.0.0.1:8080/foo+bar for me by the library, but the following error occurs:

java.net.URISyntaxException: Illegal character in path at index 25: http://127.0.0.1:8080/foo bar
	at java.net.URI$Parser.fail(URI.java:2848)
	at java.net.URI$Parser.checkChars(URI.java:3021)
	at java.net.URI$Parser.parseHierarchical(URI.java:3105)
	at java.net.URI$Parser.parse(URI.java:3053)
	at java.net.URI.<init>(URI.java:588)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at org.springsource.loaded.ri.ReflectiveInterceptor.jlrConstructorNewInstance(ReflectiveInterceptor.java:1075)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:83)
	at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrapNoCoerce.callConstructor(ConstructorSite.java:105)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:60)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:235)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:247)
	at com.budjb.httprequests.HttpRequest.parseUri(HttpRequest.groovy:420)
	at com.budjb.httprequests.HttpRequest.<init>(HttpRequest.groovy:114)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at org.springsource.loaded.ri.ReflectiveInterceptor.jlrConstructorNewInstance(ReflectiveInterceptor.java:1075)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:83)
	at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrapNoCoerce.callConstructor(ConstructorSite.java:105)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:60)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:235)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:247)
	at com.rackspace.vdo.sandbox.SandboxController.index(SandboxController.groovy:41)
	... 59 common frames omitted

While this use-case is demonstrative just of spaces, general encoding may be helpful / convenient.

Corrected example to exclude the new URL wrapping, as I was experimenting to see if that would help / work.

Corrected the URI to just be a String.

budjb commented

The error occurs when parsing the string with Java's URI class. The URL path component should be property encoded before passing to the client. This pertains to the actual URL, since the URL you've posted in the example is actually invalid. Query parameters set in the request should have URL encoding done on them.