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

Resuming a resumable upload from GCS C++ client does not work: fake-gcs-server rejects “Content-Range: bytes */*” #1149

Open
QrczakMK opened this issue May 5, 2023 · 4 comments
Labels

Comments

@QrczakMK
Copy link

QrczakMK commented May 5, 2023

When resuming a resumable upload using gcs::RestoreResumableUploadSession(), the C++ client sends Content-Range: bytes */*:
https://github.com/googleapis/google-cloud-cpp/blob/1178ef9bbb0257e940dc2d9af475802d8bb37038/google/cloud/storage/internal/rest_client.cc#L699

fake-gcs-server rejects this:

return parsed, invalidErr

AFAIK this syntax is valid.

@fsouza fsouza added the bug label May 5, 2023
fsouza added a commit that referenced this issue May 6, 2023
I spent some time reading about Content-Range and it does look like this
is valid (though it's kinda equivalent to omitting the value?)

Fixes #1149.
@fsouza
Copy link
Owner

fsouza commented May 6, 2023

@QrczakMK thanks for reporting, can you give #1150 a shot?

@QrczakMK
Copy link
Author

QrczakMK commented May 7, 2023

It helps in the sense that the request syntax is no longer rejected, but a resumable upload still fails. The same test succeeds against the real GCS server.

I don’t have that test open sourced (it is in Google), but here is what happens. I begin writing to a new object with gcs::NewResumableUploadSession():

… \"POST /upload/storage/v1/b/test-bucket/o?uploadType=resumable&name=riegeli-test-thcivcvtyawjewoe-object HTTP/1.1\" 200 423"

I write the first piece, capture resumable_session_id, suspend writing, and open a new writing session with gcs::RestoreResumableUploadSession(resumable_session_id) and gcs::DisableCrc32cChecksum(true):

… \"PUT /upload/resumable/9e63eeafa4ee28ae17ac7ce5a84cff9a HTTP/1.1\" 200 598"

But gcs::ObjectWriteStream::IsOpen() is false and gcs::ObjectWriteStream::metadata().ok() is true, which according to the documentation indicates that the previous upload was finished: https://github.com/googleapis/google-cloud-cpp/blob/1178ef9bbb0257e940dc2d9af475802d8bb37038/google/cloud/storage/object_write_stream.h#L175-L192.

@fsouza
Copy link
Owner

fsouza commented May 7, 2023

Gotcha, thanks for providing additional info. I'll try to create a reproducer.

@QrczakMK
Copy link
Author

QrczakMK commented May 7, 2023

Here is a strace log of socket communication (of a similar but different run — this is a randomized test):

sendto(12, "POST /storage/v1/b?project= HTTP/1.1\r\nHost: localhost:27454\r\nUser-Agent: gcloud-cpp/v2.11.0-rc.g3+piper (Clang-9999.0.0; noex) libcurl/8.0.1-DEV BoringSSL zlib/1.2.13 brotli/1.0.9 c-ares/1.19.0 nghttp2/1.41.0\r\nAccept: */*\r\nx-goog-api-client: gl-cpp/Clang-9999.0.0-noex-2017 gccl/v2.11.0-rc.g3+piper\r\ncontent-type: application/json\r\nContent-Length: 52\r\n\r\n", 354, MSG_NOSIGNAL, NULL, 0) = 354
sendto(12, "{\"defaultEventBasedHold\":false,\"name\":\"test-bucket\"}", 52, MSG_NOSIGNAL, NULL, 0) = 52
recvfrom(12, "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nDate: Sun, 07 May 2023 16:29:46 GMT\r\nContent-Length: 156\r\n\r\n{\"kind\":\"storage#bucket\",\"id\":\"test-bucket\",\"name\":\"test-bucket\",\"versioning\":{},\"timeCreated\":\"2023-05-07T09:29:46.703865-07:00\",\"location\":\"US-CENTRAL1\"}\n", 131072, 0, NULL, NULL) = 265
sendto(12, "POST /upload/storage/v1/b/test-bucket/o?uploadType=resumable&name=riegeli-test-ihnwgelmfdmhgvgu-object HTTP/1.1\r\nHost: localhost:27454\r\nUser-Agent: gcloud-cpp/v2.11.0-rc.g3+piper (Clang-9999.0.0; noex) libcurl/8.0.1-DEV BoringSSL zlib/1.2.13 brotli/1.0.9 c-ares/1.19.0 nghttp2/1.41.0\r\nAccept: */*\r\nx-goog-api-client: gl-cpp/Clang-9999.0.0-noex-2017 gccl/v2.11.0-rc.g3+piper\r\ncontent-type: application/json; charset=UTF-8\r\nContent-Length: 0\r\n\r\n", 443, MSG_NOSIGNAL, NULL, 0) = 443
recvfrom(12, "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nLocation: http://localhost:27454/upload/resumable/353fee57963c299593b3504a00aecdef\r\nDate: Sun, 07 May 2023 16:29:46 GMT\r\nContent-Length: 423\r\n\r\n{\"kind\":\"storage#object\",\"name\":\"riegeli-test-ihnwgelmfdmhgvgu-object\",\"id\":\"test-bucket/riegeli-test-ihnwgelmfdmhgvgu-object\",\"bucket\":\"test-bucket\",\"size\":\"0\",\"acl\":[{\"bucket\":\"test-bucket\",\"entity\":\"projectOwner\",\"object\":\"riegeli-test-ihnwgelmfdmhgvgu-object\",\"projectTeam\":{},\"role\":\"OWNER\"}],\"timeCreated\":\"0001-01-01T00:00:00Z\",\"timeDeleted\":\"0001-01-01T00:00:00Z\",\"updated\":\"0001-01-01T00:00:00Z\",\"generation\":\"0\"}\n", 65536, 0, NULL, NULL) = 616
sendto(12, "PUT /upload/resumable/353fee57963c299593b3504a00aecdef HTTP/1.1\r\nHost: localhost:27454\r\nUser-Agent: gcloud-cpp/v2.11.0-rc.g3+piper (Clang-9999.0.0; noex) libcurl/8.0.1-DEV BoringSSL zlib/1.2.13 brotli/1.0.9 c-ares/1.19.0 nghttp2/1.41.0\r\nAccept: */*\r\nx-goog-api-client: gl-cpp/Clang-9999.0.0-noex-2017 gccl/v2.11.0-rc.g3+piper\r\ncontent-type: application/octet-stream\r\ncontent-range: bytes */*\r\nContent-Length: 0\r\n\r\n", 414, MSG_NOSIGNAL, NULL, 0) = 414
recvfrom(12, "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nRange: bytes=0-0\r\nDate: Sun, 07 May 2023 16:29:46 GMT\r\nContent-Length: 598\r\n\r\n{\"kind\":\"storage#object\",\"name\":\"riegeli-test-ihnwgelmfdmhgvgu-object\",\"id\":\"test-bucket/riegeli-test-ihnwgelmfdmhgvgu-object\",\"bucket\":\"test-bucket\",\"size\":\"0\",\"contentType\":\"application/octet-stream\",\"crc32c\":\"AAAAAA==\",\"acl\":[{\"bucket\":\"test-bucket\",\"entity\":\"projectOwner\",\"object\":\"riegeli-test-ihnwgelmfdmhgvgu-object\",\"projectTeam\":{},\"role\":\"OWNER\"}],\"md5Hash\":\"1B2M2Y8AsgTpgAmY7PhCfg==\",\"etag\":\"\\\"1B2M2Y8AsgTpgAmY7PhCfg==\\\"\",\"timeCreated\":\"2023-05-07T09:29:46.712809-07:00\",\"timeDeleted\":\"0001-01-01T00:00:00Z\",\"updated\":\"2023-05-07T09:29:46.712811-07:00\",\"generation\":\"1683476986712812\"}\n", 65536, 0, NULL, NULL) = 725

I am not familiar with the protocol so I am not sure what it should be there.

No data were actually written because the length did not reach 256 KiB. Still it should resume starting from 0.

I don’t have an easy comparison with a real GCS server because in that case the communication goes via https.

fsouza added a commit that referenced this issue May 9, 2023
I spent some time reading about Content-Range and it does look like this
is valid (though it's kinda equivalent to omitting the value?)

Fixes #1149.
fsouza added a commit that referenced this issue May 9, 2023
I spent some time reading about Content-Range and it does look like this
is valid (though it's kinda equivalent to omitting the value?)

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

No branches or pull requests

2 participants