Skip to content

fix(storage): SignedURL v4 allows headers with colons in value#7543

Merged
BrennaEpp merged 6 commits intogoogleapis:mainfrom
BrennaEpp:v4-headers
Mar 17, 2023
Merged

fix(storage): SignedURL v4 allows headers with colons in value#7543
BrennaEpp merged 6 commits intogoogleapis:mainfrom
BrennaEpp:v4-headers

Conversation

@BrennaEpp
Copy link
Contributor

Previously, this:

opts := &storage.SignedURLOptions{  
       Scheme:  storage.SigningSchemeV4,  
       Headers: []string{  
            "x-goog-meta-start-time:2023-02-10T03:",   
        },  
        ...
}
url, err := client.Bucket(bucket).SignedURL(object, opts)  

would return a URL that would return a 403 (signature mismatch) with the header x-goog-meta-start-time:2023-02-10T03:;
you would have to specify the header as x-goog-meta-start-time:2023-02-10T03 to get a 200 with the provided URL.

Based on our public docs, values should be able to use any US-ASCII characters, which includes colons.

This PR fixes the splitting of the headers so that colons (and anything after them) are preserved in the header values.

@BrennaEpp BrennaEpp requested review from a team March 13, 2023 22:58
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: storage Issues related to the Cloud Storage API. labels Mar 13, 2023
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Mar 14, 2023
@BrennaEpp BrennaEpp requested a review from tritone March 14, 2023 21:35
Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix. Can you look into adding a test case for this to the cross-lang signed URL conformance tests as well? https://github.com/googleapis/conformance-tests/blob/main/storage/v1/v4_signatures.json

@BrennaEpp
Copy link
Contributor Author

Nice fix. Can you look into adding a test case for this to the cross-lang signed URL conformance tests as well? https://github.com/googleapis/conformance-tests/blob/main/storage/v1/v4_signatures.json

Sure thing, I'll open a PR there as discussed offline.

@BrennaEpp BrennaEpp enabled auto-merge (squash) March 17, 2023 20:21
@BrennaEpp BrennaEpp merged commit 602014d into googleapis:main Mar 17, 2023
@BrennaEpp BrennaEpp deleted the v4-headers branch March 17, 2023 23:25
tritone added a commit that referenced this pull request Mar 20, 2023
gcf-merge-on-green bot pushed a commit that referenced this pull request Mar 20, 2023
…e" (#7592)

This causes a build failure in Go 1.17 because `strings.Cut` was not added until Go 1.18. Need to investigate why the presubmit did not catch this.

Reverts #7543

Fixes #7588
@BrennaEpp BrennaEpp restored the v4-headers branch March 20, 2023 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants