RequestBodyUtil.create uses inputStream.available() as contentLength

[dantman]

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

n/a

Bug

See: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/modules/network/RequestBodyUtil.java#L132

RequestBodyUtil.create(MediaType mediaType, InputStream inputStream) uses inputStream.available() to set the contentLength of the RequestBody which is wrong.

https://developer.android.com/reference/java/io/InputStream.html#available()

Returns an estimate of the number of bytes that can be read (or skipped over) from this input stream without blocking by the next invocation of a method for this input stream. The next invocation might be the same thread or another thread. A single read or skip of this many bytes will not block, but may read or skip fewer bytes.

Note that while some implementations of InputStream will return the total number of bytes in the stream, many will not. It is never correct to use the return value of this method to allocate a buffer intended to hold all data in this stream.

[janicduplessis]

Do you know what method should be used instead? I PR would be appreciated :)

[dantman]

InputStreams do not have a defined length. If you're uploading a local file within the app's container, you use file.length() on the file object you pass to new FileInputStream. If you are uploading an openable file from a document provider outside your app you can ask the content resolver for the openable size though it does not have to be defined, there are a few document providers that can give you documents without a known length until you read the full input stream And if you're uploading a virtual file by asking for an alternative mime type you can use assetFileDescriptor.getLength() though this can also return UNKNOWN_LENGTH.

[tyricec]

I want to look into this one.

I'm not sure how I should verify testing this. I was thinking of creating an app that creates request with different types of data and seeing what input streams are created from this request body. Not sure if this is overkill or not.

[tyricec]

Actually, I'm not sure if I'm the right person to look at this. I think I would need to learn more about Android. I will leave this available to anyone else that would like to work on this.

[rohanramaswamy]

The available() method returns the the total number of bytes in the stream when certain implementations of InputStream is used. https://docs.oracle.com/javase/7/docs/api/java/io/InputStream.html#available()

For example, FileInputStream returns an estimate of the number of remaining bytes that can be read (or skipped over) from the inputstream.

As for other types of inputstream it is not possible to get the stream size this would result in an unpredictable behaviour, this method should return -1 by default to indicate unknown size.
https://square.github.io/okhttp/3.x/okhttp/okhttp3/RequestBody.html#contentLength--

[hotchemi]

So the summary is like:

1. if InputStream is an instance of FileInputStream we rely on available method
2. if InputStream is not an instance of FileInputStream:

• returns -1 to indicate file size is unknown
• Or copy stream data to Buffer of okio or ByteArrayOutputStream like what OKHttp is doing in a few RequestBody extended classes or for example snippet below:

ByteArrayOutputStream xxx = new ByteArrayOutputStream();
byte[] buf = new byte[32768];
int size = 0;
while((size = inputStream.read(buf, 0, buf.length)) != -1) {
    xxx.write(buf, 0, size);
}   
return xxx.size();

3. If IOException is occurred returns -1

2 is the discussion point presumably...🤔 I dug into RequestBodyUtil a bit and seemingly all passed InputStream is FileInputStream though.

@dantman @janicduplessis What do you think about above? I'd like to send PR but before that let me clarify to being on the same page🙇

[dantman]

FileInputStream.available is still not supposed to be used to figure out the file size.

RequestBodyUtil.create(MediaType, InputStream) should just be deprecated and replaced. The caller should be passing a size along with the media type and body. The callers upstream of this helper function have much better access to the file size.

If a FileInputStream is being passed to RBU.create then somewhere close upstream it was created from a File instance that can just have .size() called on it.

If the InputStream comes from the content resolver like in getFileInputStream, that same code has the ability to ask Android's document provider for the size column of the fileContentUri (which may be unknown so more work will need to be done, but often is not giving you the size you need).

[Salakar]

👋 hey everyone. Thank you for raising this issue and the discussion of potential solutions here. Given the age of this issue and time since the last activity, I'm going to go ahead and close this issue for now.

However; should you still like this to remain open please let me know and I'll re-open - I would suggest though tackling this via a PR instead (if someone would like to volunteer that'd be great ❤️) and having the discussion there on the various ways of resolving this issue.

Thank you