Support for url-encoded forward-slash in URL about http-kit HOT 13 CLOSED

How about this hack:

(def s (str "http://localhost:15672/" (java.net.URLEncoder/encode "api/exchanges/%2F/test") ))
(.getPath (java.net.URI. s)) => "/api/exchanges/%2F/test"

from http-kit.

Almost :)
It solves the URI#getPath issue, but i still have a problem in HttpUtils.java#L219: HttpUtils#encodeURI encodes %2F to %252F.

user> (def s (str "http://localhost:15672/" (java.net.URLEncoder/encode "api/exchanges/%2F/test") ))
#'user/s
user> (org.httpkit.HttpUtils/encodeURI (.getPath (java.net.URI. s)))
"/api/exchanges/%252F/test"

from http-kit.

I understand the problem

'/' is quite a special character, special care needed to make it get right.
In order to send the request http://localhost:15672/中文/api/exchanges/%2F/test, path should be encoded, I take the approach Javascript did, just

encodeURI(path)

JDK's HttpURLConnection did no encoding, just sent

GET /中文/api/exchanges/%2F/test HTTP/1.1
User-Agent: Java/1.6.0_45
Host: localhost:15672
Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2
Connection: keep-alive

http-kit did:

GET /urlencode/%E4%B8%AD%E6%96%87/api/exchanges/%252F/test HTTP/1.1
Host: localhost:15672
Accept: */*
User-Agent: http-kit/2.0
Accept-Encoding: gzip, deflate
Content-Length: 0

clj-http did:

GET /urlencode/%E4%B8%AD%E6%96%87/api/exchanges/%2F/test HTTP/1.1
Connection: close
accept-encoding: gzip, deflate
Host: localhost:15672
User-Agent: Apache-HttpClient/4.2.3 (java 1.5)

The way you suggest, Is that http-kit should follow HttpURLConnection or clj-http's way?

from http-kit.

clj-http's way do encoding is better, I suspect. I should consult the rfc to make it more clear what should be done.

from http-kit.

Yes, clj-http's way looks better. URL-encoded '/' in URLs is really a special case. What about adding :use-raw-path (default false) to the opts argument of org.httpkit.client/request?

from http-kit.

What do you think about this change?

from http-kit.

Oh, actually i missed the difference between http-kit and clj-http encoding. The clj-http one will not require any workaround.

from http-kit.

Hi, thanks for the patch.

I am looking at clj-http's code to see how it do the encoding. Anyway, it's way of encoding is quite make sense.
Will keep you updated.

from http-kit.

Do you mean clj-http just did it right for you?

I am a little confusing,
Here is clj-http's relevant code: https://github.com/dakrone/clj-http/blob/master/src/clj_http/client.clj#L71

% is a special char, should be encoded as well, but it seems that clj-http ignore % according to the regex
[^a-zA-Z0-9\.\-\_\~\!\$\&\'\(\)\*\+\,\;\=\:\@\/\%\?]

If clj-http is right, http-kit should model this way: 98d46d1

I am not sure clj-http is right. Since this is how encodeURI is documented in MDC: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURI

A little confusing now.

from http-kit.

I didn't try clj-http yet, just looked at the output that you posted earlier. I'll try it later today and will let you know.

from http-kit.

Tried clj-http 0.7.5 today. It works (encodes unicode characters in URL and doesn't mess parts that were already URL-encoded).

user=> (client/get "http://localhost:15672/中文/api/exchanges/%2F/test" {:debug true)}
Request: nil
{:scheme :http,
 :http-url "http://localhost:15672/中文/api/exchanges/%2F/test",
 :request-method :get,
 :query-string nil,
 :uri "/中文/api/exchanges/%2F/test",
 :server-name "localhost",
 :headers {"accept-encoding" "gzip, deflate"},
 :debug true,
 :body-type nil,
 :server-port 15672,
 :body nil,
 :user-info nil}
HttpRequest:
{:requestLine
 #<BasicRequestLine GET http://localhost:15672/%E4%B8%AD%E6%96%87/api/exchanges/%2F/test HTTP/1.1>,
 :protocolVersion #<HttpVersion HTTP/1.1>,
 :params
 #<BasicHttpParams org.apache.http.params.BasicHttpParams@3b5f8a0a>,
 :method "GET",
 :class org.apache.http.client.methods.HttpGet,
 :allHeaders
 [#<BasicHeader Connection: close>,
  #<BasicHeader accept-encoding: gzip, deflate>],
 :aborted false,
 :URI #<URI http://localhost:15672/中文/api/exchanges/%2F/test>}

from http-kit.

Thanks for the feedback, http-kit now follows clj-http's way of encoding URI:

[http-kit "2.1.7"]

from http-kit.

Awesome, 2.1.7 works, thanks a lot.

from http-kit.