NullPointerException in ring.middleware.multipart-params middleware

[marcomorain]

Hi there,

We are getting an NPE thrown from the ring.middleware.multipart-params and I don't know where to start looking to track the problem down. Any help would be appreciated.

This happens very rarely, 19 times in the last 7 days, on a HTTP endpoint that is accessed many, many thousands of times per day.

The exception is thrown from the org.apache.commons.fileupload library, which is called from ring.middleware.multipart-params. There are a couple of our middleware items on the callstack, but I don't think these are very likely to be the cause.

Callstack

class java.lang.NullPointerException
File "MultipartStream.java", line 999 in org.apache.commons.fileupload.MultipartStream$ItemInputStream.makeAvailable
File "MultipartStream.java", line 943 in org.apache.commons.fileupload.MultipartStream$ItemInputStream.close
File "MultipartStream.java", line 922 in org.apache.commons.fileupload.MultipartStream$ItemInputStream.close
File "IOUtils.java", line 339 in org.apache.commons.io.IOUtils.closeQuietly
File "IOUtils.java", line 270 in org.apache.commons.io.IOUtils.closeQuietly
File "Streams.java", line 123 in org.apache.commons.fileupload.util.Streams.copy
File "Streams.java", line 70 in org.apache.commons.fileupload.util.Streams.copy
File "MultipartStream.java", line 593 in org.apache.commons.fileupload.MultipartStream.readBodyData
File "MultipartStream.java", line 617 in org.apache.commons.fileupload.MultipartStream.discardBodyData
File "MultipartStream.java", line 634 in org.apache.commons.fileupload.MultipartStream.skipPreamble
File "FileUploadBase.java", line 1023 in org.apache.commons.fileupload.FileUploadBase$FileItemIteratorImpl.findNextItem
File "FileUploadBase.java", line 1003 in org.apache.commons.fileupload.FileUploadBase$FileItemIteratorImpl.<init>
File "FileUploadBase.java", line 310 in org.apache.commons.fileupload.FileUploadBase.getItemIterator
File "multipart_params.clj", line 46 in ring.middleware.multipart-params/file-item-seq
File "multipart_params.clj", line 42 in ring.middleware.multipart-params/file-item-seq
File "multipart_params.clj", line 63 in ring.middleware.multipart-params/parse-multipart-params
File "multipart_params.clj", line 59 in ring.middleware.multipart-params/parse-multipart-params
File "multipart_params.clj", line 91 in ring.middleware.multipart-params/multipart-params-request
File "multipart_params.clj", line 80 in ring.middleware.multipart-params/multipart-params-request
File "RestFn.java" line 423 in clojure.lang.RestFn.invoke
File "multipart_params.clj", line 118 in ring.middleware.multipart-params/wrap-multipart-params[fn]
File "ssl.clj", line 37 in circle.http.ssl/wrap-force-ssl[fn]
File "methods.clj", line 10 in circle.http.methods/wrap-deny-unacceptable-methods[fn]
File "exceptions.clj", line 107 in circle.http.exceptions/wrap-exceptions[fn]
File "errors.clj", line 62 in circle.web.http.errors/wrap-status-pages[fn]
File "logging.clj", line 209 in circle.http.logging/wrap-logging[fn]
File "logging.clj", line 208 in circle.http.logging/wrap-logging[fn]
File "core.clj", line 31 in circleci.request-trace.core/wrap-request-id[fn]
File "RingHandler.java", line 91 in org.httpkit.server.HttpHandler.run
File "Executors.java" line 511 in java.util.concurrent.Executors$RunnableAdapter.call
File "FutureTask.java" line 266 in java.util.concurrent.FutureTask.run
File "ThreadPoolExecutor.java" line 1149 in java.util.concurrent.ThreadPoolExecutor.runWorker
File "ThreadPoolExecutor.java" line 624 in java.util.concurrent.ThreadPoolExecutor$Worker.run
File "Thread.java" line 748 in java.lang.Thread.run

Some context we have gathered:

headers.accept: */*
headers.host: circleci.com
headers.user-agent: PostmanRuntime/7.4.0
(our internal) request_id: 410e1f74-63a8-4f96-b145-df7147fca973
request_method: get
url: http://circleci.com:80/api/v1.1/recent-builds
timestamp: 1544718529 - 2018-12-13 16:28:49

In all cases, the User Agent is either PostmanRuntime/7.4.0 or PostmanRuntime/7.3.0.

[marcomorain]

For CircleCI reference, this is tracked in Rollbar.

[marcomorain]

Some relevant dependencies:

amalloy/ring-gzip-middleware "0.1.3-7f5ffd654029fa7c9e90314e2ef0a6fb306c4348"
commons-beanutils "1.9.2"
commons-codec "1.9"
commons-collections "3.2.2"
commons-digester "1.8.1"
commons-fileupload "1.3.3"
commons-io "2.5"
commons-logging "1.2"
commons-validator "1.6"
org.apache.commons/commons-compress "1.3"
org.apache.commons/commons-lang3 "3.3.2"
org.apache.commons/commons-pool2 "2.4.1"
org.apache.commons/commons-text "1.4"
org.apache.httpcomponents/httpclient "4.5.3"
org.apache.httpcomponents/httpcore "4.4.6"
org.apache.httpcomponents/httpmime "4.5.2"
ring "1.3.2"
ring/ring-anti-forgery "1.3.0"
ring/ring-codec "1.0.0"
ring/ring-core "1.3.2"
ring/ring-defaults "0.3.2"
ring/ring-devel "1.3.2"
ring/ring-headers "0.3.0"
ring/ring-servlet "1.3.2"
ring/ring-ssl "0.3.0"

[weavejester]

My guess is that this is caused by incoming requests that have a content-type of "multipart/form-data" but have no associated body. The wrap-multipart-params assumes that if the content type is correct the body should be an input stream. In the case of any servlet-derived adapter this is correct, but it doesn't apply in general, and you're using http-kit.

If I'm right, then the fix is to add a check to this function to ensure that the body is not nil. I don't have time to spin up a test http-kit environment right now and confirm my guess, so if you want to do that it would be very helpful. I'd also welcome any PRs to address this issue, once the cause has been confirmed.

[marcomorain]

Hi James, thanks for the pointers.

I can't quite reproduce - I'm getting a similar but different error when setting Content-Type: multipart/form-data on a GET request:

dev.circlehost             | org.apache.commons.fileupload.FileUploadException: the request was rejected because no multipart boundary was found
dev.circlehost             | 	at org.apache.commons.fileupload.FileUploadBase$FileItemIteratorImpl.<init>(FileUploadBase.java:990)
dev.circlehost             | 	at org.apache.commons.fileupload.FileUploadBase.getItemIterator(FileUploadBase.java:310)
dev.circlehost             | 	at ring.middleware.multipart_params$file_item_seq.invokeStatic(multipart_params.clj:46)
dev.circlehost             | 	at ring.middleware.multipart_params$file_item_seq.invoke(multipart_params.clj:42)
dev.circlehost             | 	at ring.middleware.multipart_params$parse_multipart_params.invokeStatic(multipart_params.clj:63)
dev.circlehost             | 	at ring.middleware.multipart_params$parse_multipart_params.invoke(multipart_params.clj:59)
dev.circlehost             | 	at ring.middleware.multipart_params$multipart_params_request.invokeStatic(multipart_params.clj:91)
dev.circlehost             | 	at ring.middleware.multipart_params$multipart_params_request.doInvoke(multipart_params.clj:80)
dev.circlehost             | 	at clojure.lang.RestFn.invoke(RestFn.java:423)
dev.circlehost             | 	at ring.middleware.multipart_params$wrap_multipart_params$fn__84567.invoke(multipart_params.clj:118)
dev.circlehost             | 	at circle.http.ssl$wrap_force_ssl$fn__96696.invoke(ssl.clj:39)
dev.circlehost             | 	at circle.http.methods$wrap_deny_unacceptable_methods$fn__95303.invoke(methods.clj:10)
dev.circlehost             | 	at circle.http.exceptions$wrap_exceptions$fn__63267.invoke(exceptions.clj:107)
dev.circlehost             | 	at circle.web.http.errors$wrap_status_pages$fn__84393.invoke(errors.clj:62)
dev.circlehost             | 	at circle.http.logging$wrap_logging$fn__96796$thunk__21931__auto____96797.invoke(logging.clj:209)
dev.circlehost             | 	at circle.http.logging$wrap_logging$fn__96796.invoke(logging.clj:208)
dev.circlehost             | 	at circleci.request_trace.core$wrap_request_id$fn__15483.invoke(core.clj:31)
dev.circlehost             | 	at org.httpkit.server.HttpHandler.run(RingHandler.java:91)
dev.circlehost             | 	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
dev.circlehost             | 	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
dev.circlehost             | 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
dev.circlehost             | 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
dev.circlehost             | 	at java.lang.Thread.run(Thread.java:748)

[marcomorain]

I'm wondering what the correct way to handle if org.apache.commons.fileupload.FileUploadException is throws, or the subclasses:

• InvalidContentTypeException
• IOFileUploadException
• SizeException
• UnknownSizeException

It's arguable that some of these are 4xx (size, context type) are some are 5xx (IO).

[valerauko]

You can reproduce this by creating an empty file such as hoge.bin and then using the following curl command

curl --data-binary @hoge.bin -H "Content-Type: multipart/form-data; boundary=Boundary+0xAbCdEfGbOuNdArY" localhost:3000/path/with/multipart/middleware

Trying to upload an empty file (Content-Length: 0) 100% results in the exception, so @weavejester's guess about the empty body is correct. (Reproduced with latest clj-commons/aleph)