cognitect-labs / aws-api

AWS, data driven
Apache License 2.0
724 stars 100 forks source link

ByteBuffer.hasArray should be called before calling ByteBuffer.array in util/sha-256 #220

Closed zerg000000 closed 1 year ago

zerg000000 commented 1 year ago

In Javadoc, ByteBuffer.array may return UnsupportedOperationException and Javadoc suggest to call .hasArray first.

https://docs.oracle.com/javase/8/docs/api/java/nio/ByteBuffer.html#array--

Dependencies

{:deps {com.cognitect.aws/s3 {:mvn/version "809.2.734.0"}
        com.cognitect.aws/api  {:mvn/version "0.8.505"}
        com.cognitect.aws/endpoints {:mvn/version "1.1.11.1001"}}}

Description with failing test case

(require '[cognitect.aws.client.api :as aws])
(import '[java.io RandomAccessFile])
(import '[java.nio.channels FileChannel$MapMode])

(def s3 (aws/client {:api :s3}))
(aws/invoke s3 {:op :PutObject
                :request {:Body (-> (RandomAccessFile. "deps.edn" "r") 
                                    (.getChannel) 
                                    (.map FileChannel$MapMode/READ_ONLY 0 10))
                          :Bucket "example-bucket"
                          :Key    "sample.txt"})

;; will throw UnsupportedOperationException

Stacktrace

{:cognitect.anomalies/category :cognitect.anomalies/fault,
 :cognitect.aws.client/throwable #error{:cause nil,
                                        :via [{:type java.lang.UnsupportedOperationException,
                                               :message nil,
                                               :at [java.nio.ByteBuffer array "ByteBuffer.java" 1471]}],
                                        :trace [[java.nio.ByteBuffer array "ByteBuffer.java" 1471]
                                                [cognitect.aws.util$sha_256 invokeStatic "util.clj" 89]
                                                [cognitect.aws.util$sha_256 invoke "util.clj" 81]
                                                [cognitect.aws.signers$hashed_body invokeStatic "signers.clj" 101]
                                                [cognitect.aws.signers$hashed_body invoke "signers.clj" 99]
                                                [cognitect.aws.signers$v4_sign_http_request
                                                 invokeStatic
                                                 "signers.clj"
                                                 148]
                                                [cognitect.aws.signers$v4_sign_http_request doInvoke "signers.clj" 137]
                                                [clojure.lang.RestFn invoke "RestFn.java" 529]
                                                [cognitect.aws.signers$eval17255$fn__17256 invoke "signers.clj" 163]
                                                [clojure.lang.MultiFn invoke "MultiFn.java" 244]
                                                [cognitect.aws.client$send_request$fn__16928$state_machine__8482__auto____16955$fn__16957$fn__16971
                                                 invoke
                                                 "client.clj"
                                                 97]
                                                [cognitect.aws.client$send_request$fn__16928$state_machine__8482__auto____16955$fn__16957
                                                 invoke
                                                 "client.clj"
                                                 96]
                                                [cognitect.aws.client$send_request$fn__16928$state_machine__8482__auto____16955
                                                 invoke
                                                 "client.clj"
                                                 84]
                                                [clojure.core.async.impl.ioc_macros$run_state_machine
                                                 invokeStatic
                                                 "ioc_macros.clj"
                                                 978]
                                                [clojure.core.async.impl.ioc_macros$run_state_machine
                                                 invoke
                                                 "ioc_macros.clj"
                                                 977]
                                                [clojure.core.async.impl.ioc_macros$run_state_machine_wrapped
                                                 invokeStatic
                                                 "ioc_macros.clj"
                                                 982]
                                                [clojure.core.async.impl.ioc_macros$run_state_machine_wrapped
                                                 invoke
                                                 "ioc_macros.clj"
                                                 980]
                                                [clojure.core.async.impl.ioc_macros$take_BANG_$fn__8500
                                                 invoke
                                                 "ioc_macros.clj"
                                                 991]
                                                [clojure.core.async.impl.channels.ManyToManyChannel$fn__2385$fn__2386
                                                 invoke
                                                 "channels.clj"
                                                 95]
                                                [clojure.lang.AFn run "AFn.java" 22]
                                                [java.util.concurrent.ThreadPoolExecutor
                                                 runWorker
                                                 "ThreadPoolExecutor.java"
                                                 1136]
                                                [java.util.concurrent.ThreadPoolExecutor$Worker
                                                 run
                                                 "ThreadPoolExecutor.java"
                                                 635]
                                                [clojure.core.async.impl.concurrent$counted_thread_factory$reify__2254$fn__2255
                                                 invoke
                                                 "concurrent.clj"
                                                 29]
                                                [clojure.lang.AFn run "AFn.java" 22]
                                                [java.lang.Thread run "Thread.java" 833]]}}

https://github.com/cognitect-labs/aws-api/blob/1fb906ac0552051f7e6297279f24b738d087101b/src/cognitect/aws/util.clj#L87

dchelimsky commented 1 year ago

Have you actually seen this surface? I don't know that we have any code paths that can possibly get to that function with a ByteBuffer with no backing array.

zerg000000 commented 1 year ago

@dchelimsky updated with example and detailed stacktrace.

dchelimsky commented 1 year ago

aws-api supports either a byte array or an input stream for the AWS "blob" type, but not ByteBuffer. This is documented in https://github.com/cognitect-labs/aws-api/blob/main/doc/types.md. I'll add something to the README to make sure that doc is more visible.

dchelimsky commented 1 year ago

FYI:

(def s3 (aws/client {:api :s3}))

(aws/validate-requests s3 true)

(aws/invoke s3 {:op :PutObject
                :request {:Body (-> (RandomAccessFile. "deps.edn" "r") 
                                    (.getChannel) 
                                    (.map FileChannel$MapMode/READ_ONLY 0 10))
                          :Bucket "example-bucket"
                          :Key    "sample.txt"}})
;; produces
{:clojure.spec.alpha/problems
 ({:path [:Body :byte-array],
   :pred clojure.core/bytes?,
   :val #object[java.nio.DirectByteBufferR 0x2662413f "java.nio.DirectByteBufferR[pos=0 lim=10 cap=10]"],
   :via [:cognitect.aws.s3/PutObjectRequest :cognitect.aws.s3/Body],
   :in [:Body]}
  {:path [:Body :input-stream],
   :pred (clojure.core/fn [%] (clojure.core/instance? java.io.InputStream %)),
   :val #object[java.nio.DirectByteBufferR 0x2662413f "java.nio.DirectByteBufferR[pos=0 lim=10 cap=10]"],
   :via [:cognitect.aws.s3/PutObjectRequest :cognitect.aws.s3/Body],
   :in [:Body]}),
 :clojure.spec.alpha/spec :cognitect.aws.s3/PutObjectRequest,
 :clojure.spec.alpha/value
 {:Body #object[java.nio.DirectByteBufferR 0x2662413f "java.nio.DirectByteBufferR[pos=0 lim=10 cap=10]"],
  :Bucket "example-bucket",
  :Key "sample.txt"},
 :cognitect.anomalies/category :cognitect.anomalies/incorrect}
dchelimsky commented 1 year ago

Added to README and docstrings for invoke and doc. The README changes are visible now, and the docstrings will be visible in the API docs and locally in a REPL after the next release.