blob/gcsblob: Ensure driver sets Content-Type auto-detection properly #3381

stanhu · 2024-02-23T01:48:55Z

#3371 made it possible to disable Go Cloud's Content-Type auto-detection via the
DisableContentTypeDetection option. However, the Google Cloud Storage (GCS) driver still performed auto-detection even if this option were enabled, so it was previously not possible to keep Content-Type blank. This commit adds the
DisableContentTypeDetection option to the the driver and passes along the value to make it possible to keep Content-Type blank in the GCS object metadata. This enables the use of the Response-Content-Type override in a signed URL.

This commit needs cloud.google.com/go/storage v1.39 (googleapis/google-cloud-go#9431).

stanhu · 2024-02-23T04:25:16Z

@vangent I think the GCS tests need to be re-recorded to handle the cloud.google.com/go/storage version bump first, and then perhaps again to handle the latest changes. If I substitute my bucket, then I see different recorded replays. Is there a way for the average person to this? UPDATE: I see that CONTRIBUTING.md already mentions this, so I have to let someone else do the upgrade and regenerate the replay files.

I'm not sure why the Ubuntu test is failing with this, but macOS seems to want github.com/mitchellh/go-homedir:

Ensuring that there are no dependencies not listed in ./internal/testing/alldeps...
--- ./internal/testing/alldeps	2024-02-23 03:26:16.879296954 +0000
+++ -	2024-02-23 03:41:52.978626415 +0000
@@ -100,7 +100,6 @@
 github.com/klauspost/compress
 github.com/kylelemons/godebug
 github.com/lib/pq
-github.com/mitchellh/go-homedir
 github.com/mitchellh/mapstructure
 github.com/montanaflynn/stats
 github.com/nats-io/nats.go
FAIL: dependencies changed; run: internal/testing/listdeps.sh > internal/testing/alldeps

I've regenerated the list on Linux and checked that in.

stanhu · 2024-02-23T04:53:57Z

Here's a diff between ContentType_is_left_empty_if_not_provided_and_DisableContentTypeDetection_is_true.replay:

 % diff new/ContentType_is_left_empty_if_not_provided_and_DisableContentTypeDetection_is_true.replay newpatch/ContentType_is_left_empty_if_not_provided_and_DisableContentTypeDetection_is_true.replay
2c2
<   "Initial": "AQAAAA7dahh3OKHswP4g",
---
>   "Initial": "AQAAAA7dahj1FFqpsP4g",
40c40
<       "ID": "cbec847fefac6811",
---
>       "ID": "d807be34f7fd3105",
71c71
<             "695"
---
>             "654"
77c77
<             "Fri, 23 Feb 2024 04:39:20 GMT"
---
>             "Fri, 23 Feb 2024 04:41:25 GMT"
80c80
<             "CNHm0aXSwIQDEAE="
---
>             "CLaBtuHSwIQDEAE="
96c96
<             "ABPtcPrmZkac9Ivz6JM0oa_MRPdert1V6QDwMmYv14dyiVgjakwetkodrvUPogTCrU1XKdPApw"
---
>             "ABPtcPpfV9FCLjT5Tf88GqdHuPQKdMKrAALRoYnHeBB7zxAMvPhaPNaqr4UUfOQNgtIY6KHDAeCSSFOBPQ"
99c99
<         "Body": "eyJraW5kIjoic3RvcmFnZSNvYmplY3QiLCJpZCI6InN0YW5odS10ZXN0L2Jsb2ItZm9yLXJlYWRpbmcvMTcwODY2MzE2MDIwNjE2MSIsInNlbGZMaW5rIjoiaHR0cHM6Ly93d3cuZ29vZ2xlYXBpcy5jb20vc3RvcmFnZS92MS9iL3N0YW5odS10ZXN0L28vYmxvYi1mb3ItcmVhZGluZyIsIm1lZGlhTGluayI6Imh0dHBzOi8vc3RvcmFnZS5nb29nbGVhcGlzLmNvbS9kb3dubG9hZC9zdG9yYWdlL3YxL2Ivc3Rhbmh1LXRlc3Qvby9ibG9iLWZvci1yZWFkaW5nP2dlbmVyYXRpb249MTcwODY2MzE2MDIwNjE2MSZhbHQ9bWVkaWEiLCJuYW1lIjoiYmxvYi1mb3ItcmVhZGluZyIsImJ1Y2tldCI6InN0YW5odS10ZXN0IiwiZ2VuZXJhdGlvbiI6IjE3MDg2NjMxNjAyMDYxNjEiLCJtZXRhZ2VuZXJhdGlvbiI6IjEiLCJjb250ZW50VHlwZSI6InRleHQvaHRtbDsgY2hhcnNldD11dGYtOCIsInN0b3JhZ2VDbGFzcyI6IlNUQU5EQVJEIiwic2l6ZSI6IjYzNTQiLCJtZDVIYXNoIjoiRDJwOG4vUFQxM0M0OS9vWndxNFowQT09IiwiY3JjMzJjIjoidGR3cmdRPT0iLCJldGFnIjoiQ05IbTBhWFN3SVFERUFFPSIsInRpbWVDcmVhdGVkIjoiMjAyNC0wMi0yM1QwNDozOToyMC4yNTZaIiwidXBkYXRlZCI6IjIwMjQtMDItMjNUMDQ6Mzk6MjAuMjU2WiIsInRpbWVTdG9yYWdlQ2xhc3NVcGRhdGVkIjoiMjAyNC0wMi0yM1QwNDozOToyMC4yNTZaIn0="
---
>         "Body": "eyJraW5kIjoic3RvcmFnZSNvYmplY3QiLCJpZCI6InN0YW5odS10ZXN0L2Jsb2ItZm9yLXJlYWRpbmcvMTcwODY2MzI4NTU3OTk1OCIsInNlbGZMaW5rIjoiaHR0cHM6Ly93d3cuZ29vZ2xlYXBpcy5jb20vc3RvcmFnZS92MS9iL3N0YW5odS10ZXN0L28vYmxvYi1mb3ItcmVhZGluZyIsIm1lZGlhTGluayI6Imh0dHBzOi8vc3RvcmFnZS5nb29nbGVhcGlzLmNvbS9kb3dubG9hZC9zdG9yYWdlL3YxL2Ivc3Rhbmh1LXRlc3Qvby9ibG9iLWZvci1yZWFkaW5nP2dlbmVyYXRpb249MTcwODY2MzI4NTU3OTk1OCZhbHQ9bWVkaWEiLCJuYW1lIjoiYmxvYi1mb3ItcmVhZGluZyIsImJ1Y2tldCI6InN0YW5odS10ZXN0IiwiZ2VuZXJhdGlvbiI6IjE3MDg2NjMyODU1Nzk5NTgiLCJtZXRhZ2VuZXJhdGlvbiI6IjEiLCJzdG9yYWdlQ2xhc3MiOiJTVEFOREFSRCIsInNpemUiOiI2MzU0IiwibWQ1SGFzaCI6IkQycDhuL1BUMTNDNDkvb1p3cTRaMEE9PSIsImNyYzMyYyI6InRkd3JnUT09IiwiZXRhZyI6IkNMYUJ0dUhTd0lRREVBRT0iLCJ0aW1lQ3JlYXRlZCI6IjIwMjQtMDItMjNUMDQ6NDE6MjUuNjE1WiIsInVwZGF0ZWQiOiIyMDI0LTAyLTIzVDA0OjQxOjI1LjYxNVoiLCJ0aW1lU3RvcmFnZUNsYXNzVXBkYXRlZCI6IjIwMjQtMDItMjNUMDQ6NDE6MjUuNjE1WiJ9"
103c103
<       "ID": "cde3e48f8ec60e44",
---
>       "ID": "d6df0cec23facc54",
139c139
<             "text/html; charset=utf-8"
---
>             "application/octet-stream"
142c142
<             "Fri, 23 Feb 2024 04:39:20 GMT"
---
>             "Fri, 23 Feb 2024 04:41:25 GMT"
151c151
<             "Fri, 23 Feb 2024 04:39:20 GMT"
---
>             "Fri, 23 Feb 2024 04:41:25 GMT"
157c157
<             "1708663160206161"
---
>             "1708663285579958"
176c176
<             "ABPtcPp9XU3hLfwVwoC1vJdjJn7zRorTQU2HOwTeCBzWVYLPRNjPJr345BBGYYhwF2L1NETKgQ"
---
>             "ABPtcPrchhnqFBhXE0m-ouetvWbS7oum-Yb6fgfFr5SNHrAj7ZJHCXNWTSg10IEFyO-FwHihi_wxYRRDfw"
183c183
<       "ID": "d020bd5d188ff050",
---
>       "ID": "93892b86d06d0e26",
222c222
<             "Fri, 23 Feb 2024 04:39:20 GMT"
---
>             "Fri, 23 Feb 2024 04:41:26 GMT"
238c238
<             "ABPtcPr4C4qeJPyF1Hs355ngmuBi86yohwEJAjwNpot2D32-YoWdjJw1MFzIPpH05tJd7MOZRg"
---
>             "ABPtcPo4ozm6fchsDbEgNQzPavrkXLk314QaszAfZssPn9pGFsQAOM8Rrwm1Zyn39NKjIOX6xtIagqF3qg"

The two key differences are in Body and the response Content-Type. It's interesting that application/octet-stream is now being returned instead of text/html; charset=utf-8, but the Sadly we can't really verify this comment still applies.

Body (before)

{
   "kind":"storage#object",
   "id":"stanhu-test/blob-for-reading/1708663160206161",
   "selfLink":"https://www.googleapis.com/storage/v1/b/stanhu-test/o/blob-for-reading",
   "mediaLink":"https://storage.googleapis.com/download/storage/v1/b/stanhu-test/o/blob-for-reading?generation=1708663160206161&alt=media",
   "name":"blob-for-reading",
   "bucket":"stanhu-test",
   "generation":"1708663160206161",
   "metageneration":"1",
   "contentType":"text/html; charset=utf-8",
   "storageClass":"STANDARD",
   "size":"6354",
   "md5Hash":"D2p8n/PT13C49/oZwq4Z0A==",
   "crc32c":"tdwrgQ==",
   "etag":"CNHm0aXSwIQDEAE=",
   "timeCreated":"2024-02-23T04:39:20.256Z",
   "updated":"2024-02-23T04:39:20.256Z",
   "timeStorageClassUpdated":"2024-02-23T04:39:20.256Z"
}

Body (after)

{
  "kind": "storage#object",
  "id": "stanhu-test/blob-for-reading/1708663285579958",
  "selfLink": "https://www.googleapis.com/storage/v1/b/stanhu-test/o/blob-for-reading",
  "mediaLink": "https://storage.googleapis.com/download/storage/v1/b/stanhu-test/o/blob-for-reading?generation=1708663285579958&alt=media",
  "name": "blob-for-reading",
  "bucket": "stanhu-test",
  "generation": "1708663285579958",
  "metageneration": "1",
  "storageClass": "STANDARD",
  "size": "6354",
  "md5Hash": "D2p8n/PT13C49/oZwq4Z0A==",
  "crc32c": "tdwrgQ==",
  "etag": "CLaBtuHSwIQDEAE=",
  "timeCreated": "2024-02-23T04:41:25.615Z",
  "updated": "2024-02-23T04:41:25.615Z",
  "timeStorageClassUpdated": "2024-02-23T04:41:25.615Z"
}

You can see contentType has been omitted in the second case.

stanhu · 2024-02-23T05:34:38Z

I also see that googleapis/google-cloud-go#9211 introduced the includeFoldersAsPrefixes=false query string in the URLs, so this is the main reason why the GCS suite is not working with the updated version.

This is needed in preparation for google#3381. googleapis/google-cloud-go#9211 introduced the `includeFoldersAsPrefixes=false` query string in the URLs.

vangent · 2024-02-25T02:29:06Z

As noted on the other PR, let's wait until there is a released version with the change you need, then ping me here and I'll update the dependency version.

vangent

I've submitted a PR to update dependencies, so you should be able to go ahead with this now.

blob/blob.go

blob/driver/driver.go

codecov · 2024-02-29T22:38:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.15%. Comparing base (6ca965e) to head (71bf492).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3381   +/-   ##
=======================================
  Coverage   73.15%   73.15%           
=======================================
  Files         113      113           
  Lines       14846    14848    +2     
=======================================
+ Hits        10860    10862    +2     
  Misses       3213     3213           
  Partials      773      773

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

stanhu · 2024-02-29T22:45:46Z

@vangent Thanks for your help! I think this is ready to go.

vangent

Can you update the PR to drop all of the go.mod and go.sum changes? IIUC those aren't necessary anymore.

stanhu · 2024-03-01T18:35:22Z

@vangent cloud.google.com/go/storage v1.39.0 is necessary for the ForceEmptyContentType flag:

vangent · 2024-03-01T18:48:01Z

Hmm, I thought you said that the change you needed was released, and then I updated dependencies to the latest. Did something else change, or ...? I'd rather have the dependency updates separate from code changes.

stanhu · 2024-03-01T18:54:39Z

@vangent Sorry, I should have clarified. I proposed updating to cloud.google.com/go/storage v1.38.0 first in #3382 because all the replay files needed to be regenerated for v1.38.0.

v1.39.0 was just released yesterday with the change this pull request needs with the ForceEmptyContentType flag. No replay files need to be regenerated.

vangent · 2024-03-01T19:05:23Z

Ah, OK. I will submit a separate PR to update that dependency, after which you should be able to rebase this one and have it just be the code change.

Do Azure/AWS not have any way to disable any content-type detection they are doing?

stanhu · 2024-03-01T19:16:40Z

Ah, OK. I will submit a separate PR to update that dependency, after which you should be able to rebase this one and have it just be the code change.

Thanks.

Do Azure/AWS not have any way to disable any content-type detection they are doing?

Azure appears to set the Content-Type to application/octet-stream if ContentType is left empty. I'm not sure if the client or server is doing that.

I'm going to test with AWS, but a cursory scan did not show any Content-Type detection in the Go client.

google#3371 made it possible to disable Go Cloud's Content-Type auto-detection via the `DisableContentTypeDetection` option. However, the Google Cloud Storage (GCS) driver still performed auto-detection even if this option were enabled, so it was previously not possible to keep `Content-Type` blank. This commit adds the `DisableContentTypeDetection` option to the the driver and passes along the value to make it possible to keep Content-Type blank in the GCS object metadata. This enables the use of the `Response-Content-Type` override in a signed URL. This commit needs cloud.google.com/go/storage v1.39 (googleapis/google-cloud-go#9431).

stanhu · 2024-03-01T20:19:05Z

I confirmed that AWS leaves the Content-Type blank. I applied this diff to examine the HTTP traffic:

diff --git a/aws/aws.go b/aws/aws.go
index 55dc3864..478d11fc 100644
--- a/aws/aws.go
+++ b/aws/aws.go
@@ -83,6 +83,7 @@ func (co ConfigOverrider) ClientConfig(serviceName string, cfgs ...*aws.Config)
 //   - s3ForcePathStyle: A value of "true" forces the request to use path-style addressing; sets aws.Config.S3ForcePathStyle.
 func ConfigFromURLParams(q url.Values) (*aws.Config, error) {
 	var cfg aws.Config
+	cfg.LogLevel = aws.LogLevel(aws.LogDebug)
 	for param, values := range q {
 		value := values[0]
 		switch param {
diff --git a/samples/gocdk-blob/main.go b/samples/gocdk-blob/main.go
index 2d0449e9..b92978e4 100644
--- a/samples/gocdk-blob/main.go
+++ b/samples/gocdk-blob/main.go
@@ -192,7 +192,8 @@ func (*uploadCmd) Execute(ctx context.Context, f *flag.FlagSet, _ ...interface{}
 	defer bucket.Close()
 
 	// Open a *blob.Writer for the blob at blobKey.
-	writer, err := bucket.NewWriter(ctx, blobKey, nil)
+	opts := blob.WriterOptions{DisableContentTypeDetection: true}
+	writer, err := bucket.NewWriter(ctx, blobKey, &opts)
 	if err != nil {
 		log.Printf("Failed to write %q: %v\n", blobKey, err)
 		return subcommands.ExitFailure

Then I used gocdk-blob to upload a file:

% cat main.go | ./gocdk-blob upload "s3://redacted?region=us-west-2" main.go
2024/03/01 12:16:51 DEBUG: Request s3/PutObject Details:
---[ REQUEST POST-SIGN ]-----------------------------
PUT /main.go HTTP/1.1
Host: redacted.s3.us-west-2.amazonaws.com
User-Agent: aws-sdk-go/1.50.29 (go1.21.6; darwin; arm64) S3Manager
Content-Length: 5797
Authorization: AWS4-HMAC-SHA256 Credential=REDACTED/20240301/us-west-2/s3/aws4_request, SignedHeaders=content-length;content-md5;content-type;host;x-amz-content-sha256;x-amz-date, Signature=37e7099dfb6c02fa2555d9d508513536a91d8faf04cd437a433b0394ac3e3643
Content-Md5: l6jCdv1TY7ep5kuifTwxVA==
Content-Type:
X-Amz-Content-Sha256: 8700113c483b76e7175caedbd5fc2d6bb2b08f8914c4c520b0efa893120d3c4a
X-Amz-Date: 20240301T201651Z
Accept-Encoding: gzip


-----------------------------------------------------
2024/03/01 12:16:51 DEBUG: Response s3/PutObject Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 200 OK
Content-Length: 0
Date: Fri, 01 Mar 2024 20:16:52 GMT
Etag: "97a8c276fd5363b7a9e64ba27d3c3154"
Server: AmazonS3
X-Amz-Id-2: n95qpqGNvGT+GWYs/5aTI+o7jx0I1QyT9nXHpdjtxOFh1QKfDkximTvyY67hCK//X2tbB+wyoIA=
X-Amz-Request-Id: EC0NM5CJ05CH09D6
X-Amz-Server-Side-Encryption: AES256


-----------------------------------------------------

The download confirmed a blank Content-Type:

% ./gocdk-blob download "s3://redacted?region=us-west-2" main.go
2024/03/01 12:17:45 DEBUG: Request s3/GetObject Details:
---[ REQUEST POST-SIGN ]-----------------------------
GET /main.go HTTP/1.1
Host: redacted.s3.us-west-2.amazonaws.com
User-Agent: aws-sdk-go/1.50.29 (go1.21.6; darwin; arm64)
Authorization: AWS4-HMAC-SHA256 Credential=REDACTED/20240301/us-west-2/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=f2dfc90a65eaa66a5d63d220198cc4e0ab6c92f088d1f0ddd09ce32ab08e49fa
X-Amz-Content-Sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
X-Amz-Date: 20240301T201745Z
Accept-Encoding: gzip


-----------------------------------------------------
2024/03/01 12:17:45 DEBUG: Response s3/GetObject Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 200 OK
Content-Length: 5797
Accept-Ranges: bytes
Content-Type:
Date: Fri, 01 Mar 2024 20:17:46 GMT
Etag: "97a8c276fd5363b7a9e64ba27d3c3154"
Last-Modified: Fri, 01 Mar 2024 20:16:52 GMT
Server: AmazonS3
X-Amz-Id-2: rIHHlRV+xYFlIxDe0/7FlwEP5b4eJF5e7vPsZIc8Hj4fvDGumNBva45GqBQaIYaGHHo2kNXqHRM=
X-Amz-Request-Id: 2Z3KQM9GET3JDAMS
X-Amz-Server-Side-Encryption: AES256

stanhu force-pushed the sh-gcsblob-disable-content-type-detection branch 3 times, most recently from fff9db4 to 53a0705 Compare February 23, 2024 03:26

stanhu force-pushed the sh-gcsblob-disable-content-type-detection branch from 53a0705 to 74b5319 Compare February 23, 2024 05:07

stanhu added a commit to stanhu/go-cloud that referenced this pull request Feb 23, 2024

all: update deps for gcsblob

9f369e8

This is needed in preparation for google#3381. googleapis/google-cloud-go#9211 introduced the `includeFoldersAsPrefixes=false` query string in the URLs.

stanhu mentioned this pull request Feb 23, 2024

all: update deps for gcsblob #3382

Closed

stanhu added a commit to stanhu/go-cloud that referenced this pull request Feb 23, 2024

all: update deps for gcsblob

d4457dd

This is needed in preparation for google#3381. googleapis/google-cloud-go#9211 introduced the `includeFoldersAsPrefixes=false` query string in the URLs.

vangent requested changes Feb 25, 2024

View reviewed changes

blob/blob.go Show resolved Hide resolved

blob/driver/driver.go Outdated Show resolved Hide resolved

stanhu added a commit to stanhu/go-cloud that referenced this pull request Feb 29, 2024

all: update deps (google#3381)

bfbcaa1

stanhu force-pushed the sh-gcsblob-disable-content-type-detection branch from 74b5319 to 2132176 Compare February 29, 2024 22:32

stanhu requested a review from vangent February 29, 2024 23:05

vangent requested changes Mar 1, 2024

View reviewed changes

stanhu force-pushed the sh-gcsblob-disable-content-type-detection branch from 2132176 to 71bf492 Compare March 1, 2024 19:17

vangent approved these changes Mar 1, 2024

View reviewed changes

vangent merged commit 7750aa6 into google:master Mar 1, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

blob/gcsblob: Ensure driver sets Content-Type auto-detection properly #3381

blob/gcsblob: Ensure driver sets Content-Type auto-detection properly #3381

stanhu commented Feb 23, 2024 •

edited

Loading

stanhu commented Feb 23, 2024 •

edited

Loading

stanhu commented Feb 23, 2024 •

edited

Loading

stanhu commented Feb 23, 2024

vangent commented Feb 25, 2024

vangent left a comment

codecov bot commented Feb 29, 2024 •

edited

Loading

stanhu commented Feb 29, 2024

vangent left a comment

stanhu commented Mar 1, 2024 •

edited

Loading

vangent commented Mar 1, 2024

stanhu commented Mar 1, 2024

vangent commented Mar 1, 2024

stanhu commented Mar 1, 2024

stanhu commented Mar 1, 2024

blob/gcsblob: Ensure driver sets Content-Type auto-detection properly #3381

blob/gcsblob: Ensure driver sets Content-Type auto-detection properly #3381

Conversation

stanhu commented Feb 23, 2024 • edited Loading

stanhu commented Feb 23, 2024 • edited Loading

stanhu commented Feb 23, 2024 • edited Loading

Body (before)

Body (after)

stanhu commented Feb 23, 2024

vangent commented Feb 25, 2024

vangent left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 29, 2024 • edited Loading

Codecov Report

stanhu commented Feb 29, 2024

vangent left a comment

Choose a reason for hiding this comment

stanhu commented Mar 1, 2024 • edited Loading

vangent commented Mar 1, 2024

stanhu commented Mar 1, 2024

vangent commented Mar 1, 2024

stanhu commented Mar 1, 2024

stanhu commented Mar 1, 2024

stanhu commented Feb 23, 2024 •

edited

Loading

stanhu commented Feb 23, 2024 •

edited

Loading

stanhu commented Feb 23, 2024 •

edited

Loading

codecov bot commented Feb 29, 2024 •

edited

Loading

stanhu commented Mar 1, 2024 •

edited

Loading