yada icon indicating copy to clipboard operation
yada copied to clipboard

Null pointer exception in multipart.clj finish-up

Open josf opened this issue 8 years ago • 9 comments

In the :part and :in-partial parts of finish-up, there is a small bug (AFAICT):

(if-let [e (+ (last positions) (:delimiter-size m))]

I get a null pointer here. I'm assuming it is because (last positions) is returning nil. In any case, this will always either throw an exception or return true (if + succeeds). Probably want to test for truth before adding.

josf avatar Apr 08 '16 09:04 josf

I can see how this might happen. Do you happen to have a failing test or some input data that causes the stack-trace that you can share? Even the size of the input data and boundary positions would be useful.

malcolmsparks avatar Apr 08 '16 09:04 malcolmsparks

I'll try to write this up as a test, or at least dump out the content. Just playing around on my app, I have determined that the error only occurs on files bigger than about 75 KB.

Update: more details:

java.lang.NullPointerException
    at clojure.lang.Numbers.ops(Numbers.java:1013)
    at clojure.lang.Numbers.add(Numbers.java:128)
    at yada.multipart$finish_up.invoke(multipart.clj:313)
    at yada.multipart$process_chunk.invoke(multipart.clj:334)
    at yada.multipart$parse_multipart$this__23960__auto____44239$fn__44240$fn__44241.invoke(multipart.clj:420)
    at manifold.deferred$eval23869$chain___23890.invoke(deferred.clj:860)
    at yada.multipart$parse_multipart$this__23960__auto____44239$fn__44240.invoke(multipart.clj:414)
    at yada.multipart$parse_multipart$this__23960__auto____44239.invoke(multipart.clj:402)
    at clojure.lang.AFn.applyToHelper(AFn.java:156)
    at clojure.lang.AFn.applyTo(AFn.java:144)
    at clojure.core$apply.invoke(core.clj:632)
    at yada.multipart$parse_multipart$this__23960__auto____44239$fn__44267.invoke(multipart.clj:402)
    at manifold.deferred.Listener.onSuccess(deferred.clj:219)
    at manifold.deferred.Deferred$fn__23671$fn__23672.invoke(deferred.clj:396)
    at clojure.lang.AFn.run(AFn.java:22)
    at io.aleph.dirigiste.Executor$Worker$1.run(Executor.java:62)
    at manifold.executor$thread_factory$reify__23072$f__23073.invoke(executor.clj:36)
    at clojure.lang.AFn.run(AFn.java:22)
    at java.lang.Thread.run(Thread.java:745)

The input on a failing POST:

POST /bibliotheque/traite/nouveau HTTP/1.1
Host: localhost:9999
Cache-Control: no-cache
Postman-Token: 092492f3-7d13-9a77-9274-cd2f2407c2fa
Content-Type: multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW

----WebKitFormBoundary7MA4YWxkTrZu0gW
Content-Disposition: form-data; name="lastname"

Bob
----WebKitFormBoundary7MA4YWxkTrZu0gW
Content-Disposition: form-data; name="title"

zook
----WebKitFormBoundary7MA4YWxkTrZu0gW
Content-Disposition: form-data; name="date_vis"

1999
----WebKitFormBoundary7MA4YWxkTrZu0gW
Content-Disposition: form-data; name="date_calc"

1999
----WebKitFormBoundary7MA4YWxkTrZu0gW
Content-Disposition: form-data; name="language"

fr
----WebKitFormBoundary7MA4YWxkTrZu0gW
Content-Disposition: form-data; name="treatiseurl"

blah
----WebKitFormBoundary7MA4YWxkTrZu0gW
Content-Disposition: form-data; name="document"; filename=""
Content-Type: 


----WebKitFormBoundary7MA4YWxkTrZu0gW

The file itself was a 127 KB png.

The yada resource looks like this:

(y/resource
                {:produces ["text/html"]
                 :methods {:get {:response (:bib-new handler-map)}
                           :post {:consumes {:media-type #{"multipart/form-data"}}
                                  :response (:bib-new-post handler-map)
                                  :parameters {:form rb/NewTreatiseForm}}}})}))

josf avatar Apr 08 '16 11:04 josf

set {:raw-stream? true} on your aleph.http/start-server opts and this problem goes away

https://github.com/ztellman/aleph/blob/master/src/aleph/http.clj#L32

and it only took about 18h of furious debugging to figure this out

mccraigmccraig avatar Apr 15 '16 00:04 mccraigmccraig

@josf Does this clear up your problem? If so let me know and let's close this

malcolmsparks avatar Apr 15 '16 07:04 malcolmsparks

@malcolmsparks Yes, this works now! Thank you, @mccraigmccraig, for the furious debugging. I feel your pain.

One last thing: there still seems to be a logical error in this if-let condition, which can never evaluate to anything other than true, since the call to + will never return a non-truthy value.

Thanks again!

josf avatar Apr 15 '16 21:04 josf

I'll keep this open to remind me to revisit the if-let condition

malcolmsparks avatar Apr 18 '16 12:04 malcolmsparks

yep, that's what this commit i did while debugging prevents (and thus reveals the underlying exception)

https://github.com/juxt/yada/commit/63872b9eb78136cb9a62e3c9640952b1b3c8443a

mccraigmccraig avatar Apr 18 '16 13:04 mccraigmccraig

Encountered this NPE, or a very similar one, when negative testing my multi-part consumer implementation. To recreate throw an exception out of an implementation of start-partial.

tcoupland avatar Nov 16 '16 10:11 tcoupland

Thanks for this Tom. Will look into it!

malcolmsparks avatar Nov 16 '16 11:11 malcolmsparks