Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] - RequestBody supports InputStream #3585

Closed
jaredsburrows opened this issue Sep 5, 2017 · 13 comments
Closed

[Feature Request] - RequestBody supports InputStream #3585

jaredsburrows opened this issue Sep 5, 2017 · 13 comments

Comments

@jaredsburrows
Copy link
Contributor

jaredsburrows commented Sep 5, 2017

I would like to submit a PR to add support for InputStreams in the RequestBody class. As of now, RequestBody supports the following:

  • create(MediaType contentType, String content)
  • create(MediaType contentType, ByteString content)
  • create(MediaType contentType, byte[] content)
  • create(MediaType contentType, byte[] content, final int offset, final int byteCount)
  • create(MediaType contentType, File file)

I would like to go one step further and add support for InputStream:

  • create(MediaType contentType, Inputream inputStream)

Similar to https://stackoverflow.com/a/25384793:

  public static RequestBody create(final @Nullable MediaType contentType, final InputStream inputStream) {
    if (inputStream == null) throw new NullPointerException("inputStream == null");

    return new RequestBody() {
      @Override public @Nullable MediaType contentType() {
        return contentType;
      }

      @Override public long contentLength() {
        return inputStream.available() == 0 ? -1 : inputStream.available();
      }

      @Override public void writeTo(BufferedSink sink) throws IOException {
        Source source = null;
        try {
          source = Okio.source(inputStream);
          sink.writeAll(source);
        } finally {
          Util.closeQuietly(source);
        }
      }
    };
  }

This would be nice to have since Okio.source support both File and InputStream.

@JakeWharton
Copy link
Member

JakeWharton commented Sep 5, 2017 via email

@jaredsburrows
Copy link
Contributor Author

Are RequestBodys written multiple times on retries?

What would you suggest for contentResolver.openInputStream(uri)? Save it to a file first then use create(MediaType contentType, File file)? Even that uses Okio.source when uses a FileInputStream.

@JakeWharton
Copy link
Member

JakeWharton commented Sep 5, 2017 via email

@swankjesse
Copy link
Member

Also, request bodies may be written zero times, in which case this would leak the input stream.

@jaredsburrows
Copy link
Contributor Author

@swankjesse Maybe I could pass a Uri instead of InputStream in the class to prevent leaking using.

@JakeWharton If I call contentResolver.openInputStream(uri) inside of writeTo, how do I update the long contentLength() before the request is made?

@JakeWharton
Copy link
Member

JakeWharton commented Sep 5, 2017 via email

@jaredsburrows
Copy link
Contributor Author

@JakeWharton I know it is not an Android library. I figured after the first comment, I would have to keep this code locally and not be able to make a PR. Something like this:

public class InputStreamRequestBody extends RequestBody {
    private final InputStream inputStream;
    private final MediaType contentType;

    public InputStreamRequestBody(MediaType contentType, InputStream inputStream) {
        if (inputStream == null) throw new NullPointerException("inputStream == null");
        this.contentType = contentType;
        this.inputStream = inputStream;
    }

    @Nullable
    @Override
    public MediaType contentType() {
        return contentType;
    }

    @Override
    public long contentLength() throws IOException {
        return inputStream.available() == 0 ? -1 : inputStream.available();
    }

    @Override
    public void writeTo(@NonNull BufferedSink sink) throws IOException {
        Source source = null;
        try {
            source = Okio.source(inputStream);
            sink.writeAll(source);
        } finally {
            Util.closeQuietly(source);
        }
    }
}

Based on your changes, @JakeWharton, to be changed to handle Uri:

public class InputStreamRequestBody extends RequestBody {
    private final MediaType contentType;
    private final ContentResolver contentResolver;
    private final Uri uri;

    public InputStreamRequestBody(MediaType contentType, ContentResolver contentResolver, Uri uri) {
        if (uri == null) throw new NullPointerException("uri == null");
        this.contentType = contentType;
        this.contentResolver = contentResolver;
        this.uri = uri;
    }

    @Nullable
    @Override
    public MediaType contentType() {
        return contentType;
    }

    @Override
    public long contentLength() throws IOException {
        return -1;
    }

    @Override
    public void writeTo(@NonNull BufferedSink sink) throws IOException {
        sink.writeAll(Okio.source(contentResolver.openInputStream(uri)));
    }
}

@JakeWharton
Copy link
Member

JakeWharton commented Sep 5, 2017 via email

@jaredsburrows
Copy link
Contributor Author

jaredsburrows commented Sep 6, 2017

@JakeWharton Based on your first comment, I guess I will close this because using TypeInputStream directly does not work very well with the current implementation of RequestBody.

guillaume-tgl added a commit to thegrizzlylabs/sardine-android that referenced this issue Mar 5, 2020
We can't use InputStream directly because of square/okhttp#3585. Instead of being specific and pass a ContentResolver and Uri, the new method takes an InputStreamProvider, which is an interface providing an InputStream. This way, it can be called several times if needed by OkHttp.
@neiljaywarner
Copy link

@JakeWharton @jaredsburrows 3 years later. what's the best way to go from a android uri to okhttp "put"?

@Miha-x64
Copy link

@neiljaywarner

val upload = POST("/updatePhoto",
    Field("id", uuid),
    Part("photo", Stream("image/*"), { "photo.jpg" }),
    Response<Boolean>())

val doUpload = okHttpClient.template(baseUrl, upload,
    blocking { body?.close(); isSuccessful })

doUpload(id) { contentResolver.openInputStream(uri) }

https://github.com/Miha-x64/Lychee#http

@jemshit
Copy link

jemshit commented Apr 25, 2021

My take on this.

@yschimke
Copy link
Collaborator

@jemshit Thanks, we can possibly make this some samples for documentation inside the project.

@swankjesse had the most conniving argument for why we don't support this inbuilt

Also, request bodies may be written zero times, in which case this would leak the input stream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants