aleph.http.core.NettyRequest - bug in c.l.IPersistentCollection.cons(Map) implementation

[kumarshantanu]

(This might be related to Potemkin, I'm not sure.)

When I have a request of type aleph.http.core.NettyRequest, the call (conj request {:foo 10}) does not merge the latter map into the request. Whereas, the call (conj {:bar 20} {:foo 10}) merges the two maps into one.

[kachayev]

@kumarshantanu conj works by calling cons on the IPersistentCollection which is handled by the potermkin's AbstractMap. So... it should work 🤔 Would you please share the code that fails?

[kachayev]

Double checked

user=> (import 'io.netty.handler.codec.http.DefaultFullHttpRequest)
io.netty.handler.codec.http.DefaultFullHttpRequest
user=> (import 'io.netty.handler.codec.http.HttpMethod)
io.netty.handler.codec.http.HttpMethod
user=> (import 'io.netty.handler.codec.http.HttpVersion)
io.netty.handler.codec.http.HttpVersion
user=> (require '[aleph.http.core :as c])
nil
user=> (def req (DefaultFullHttpRequest. HttpVersion/HTTP_1_1 HttpMethod/GET "https://google.com"))
#'user/req
user=> (def nr (c/netty-request->ring-request req false nil nil))
#'user/nr
user=> nr
{:aleph/request-arrived 3582944625596447, :aleph/keep-alive? true, :remote-addr nil, :headers {}, :server-port nil, :uri "https://google.com", :server-name nil, :query-string nil, :body nil, :scheme :http,:request-method :get}
user=> (class nr)
aleph.http.core.NettyRequest
user=> (conj nr {:foo :bar})
{:aleph/request-arrived 3582944625596447, :aleph/keep-alive? true, :remote-addr nil, :headers {}, :server-port nil, :foo :bar, :uri "https://google.com", :server-name nil, :query-string nil, :body nil, :scheme :http, :request-method :get}

As you can see, the result of conj contains :foo key.

[kumarshantanu]

@kachayev Thanks for providing a REPL script! My original bug report example was inaccurate, though the bug shows up when you use a java.util.HashMap instead of Clojure's persistent hashmap. For example, if you modify the last step as follows then the bug is evident:

user=> (import 'java.util.HashMap)
java.util.HashMap
=> (conj nr (doto (HashMap.) (.put :foo :bar)))
{#object[java.util.HashMap$Node 0x59340f92 ":foo=:bar"] nil, :aleph/request-arrived 1217282039121021, :aleph/keep-alive? true, :remote-addr nil, :headers {}, :server-port nil, :uri "https://google.com", :server-name nil, :query-string nil, :body nil, :scheme :http, :request-method :get}

This discrepancy doesn't exist in Clojure's persistent map implementations.

[kachayev]

@kumarshantanu Ah, I see now. The problem is that java.util.HashMap does not implement IPersistentMap and potemkin does not have a special handling for this case in place (clojure's APersistentMap does).

Workaround here is pretty simple,

(conj nr (into {} (doto (HashMap.) (.put :foo :bar))))

should work just fine. For the longer term, I will open a PR to handle this in potemkin's AbstractMap implementation.

[kumarshantanu]

Just FYI, this issue is similar to https://issues.jboss.org/projects/IMMUTANT/issues/IMMUTANT-640 but not found with HTTP Kit or Ring-jetty-adapter.

[kachayev]

Immutant uses potemkin as well, so I think the root of the problem there is just the same.

[kachayev]

As far as aleph targets potemkin 0.4.5, this should be covered by clj-commons/potemkin@844f403