Skip to content

Storage ReadChannel can read blob after 416-range exception #3016

@mhl-b

Description

@mhl-b

When ReadChannel tries to read slice first time and it's out of range it returns -1, because internally hit 416 range exception. But if blob is changed after this first unsuccessful call, then following read operation on the same ReadChannel succeeds with new blob that satisfies the range.

It does violate API contract for the java.nio.channels.ReadableByteChannel#read. If read() returns -1 once, all following read operations should return -1 consistently.

Returns:
The number of bytes read, possibly zero, or -1 if the channel has reached end-of-stream

Minimal reproduction set:

       Storage storage = StorageOptions.http().setCredentials(creds).build().getService();
       String bucket = "bucket";
       String blob = "blob";
       BlobId blobid =  BlobId.of(bucket, blob);
       BlobInfo blobInfo = ...

       ReadChannel reader = storage.reader(BlobId.of(bucket, blob));
       ByteBuffer buf = ByteBuffer.allocate(10);

        // create 2-byte blob
        byte[] content1 = new byte[]{1, 2};
        storage.create(blobInfo, content1);

        // move reader out of range
        reader.seek(2);
        int read1 = reader.read(buf);
        assert read1 == -1; // expect 416 code and read -1

        // create 4-byte blob
        byte[] content2 = new byte[]{1, 2, 3, 4};
        storage.create(blobInfo, content2);

        // now can read data
        int read2 = reader.read(buf);
        assert read2 == 2;

I assume https://github.com/googleapis/java-storage/blob/v2.50.0/google-cloud-storage/src/main/java/com/google/cloud/storage/BaseStorageReadChannel.java#L135-L141 needs to remember result of 416 error?

      if (eof) return -1;
      ...
      } catch (StorageException e) {
        if (e.getCode() == 416) {
          // HttpStorageRpc turns 416 into a null etag with an empty byte array, leading
          // BlobReadChannel to believe it read 0 bytes, returning -1 and leaving the channel open.
          // Emulate that same behavior here to preserve behavior compatibility, though this should
          // be removed in the next major version.
          eof = true;
          return -1;

Metadata

Metadata

Assignees

Labels

api: storageIssues related to the googleapis/java-storage API.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions