Regression in storage url signing

[mickeyreiss]

@ostronom @mziccard It seems #1277 has caused a regression in my code. I use / characters in the blob names, and they are now being unexpectedly encoded as %2F in the signed path.

It seems that object naming requirements are not equivalent to URL encoding.

When I round-trip a signed url, Storage#get now fails to find the successfully uploaded file (confirmed via gsutil and Cloud Console).

My understanding is that /s are valid characters for blob names. From the same page quoted above:

The canonical resource is everything that follows the host name. For example, if the Cloud Storage URL is https://storage.googleapis.com/example-bucket/cat-pics/tabby.jpeg, then the canonical resource is /example-bucket/cat-pics/tabby.jpeg.

It seems like this library needs to use a different escape function for object names. (Also, I noticed that the new test testSignUrlForBlobWithSpecialChars, does not cover slashes.)

Does this make sense? What is the intended behavior for blob names containing slashes?

---

repro test:

    BlobInfo blobInfo1 = BlobInfo.builder(bucketName, "foo/bar/baz #%20other cool stuff.txt").build();
    storage.create(blobInfo1, "content".getBytes());
    URL url = storage.signUrl(
        blobInfo1,
        1,
        TimeUnit.HOURS,
        httpMethod(HttpMethod.PUT),
        httpMethod(HttpMethod.GET)
    );
    final String[] split = url.getPath().split("/");
    Blob notFound = storage.get(split[1], split[2]); // unexpectedly null!
    Blob found = storage.get(split[1], URLDecoder.decode(split[2])); // found, surprisingly

[ostronom]

Yes, i do confirm it is a bug. I will make PR it ASAP.

Haven't you considered Content-Disposition as a workaround?

[mickeyreiss]

Thanks, @ostronom.

The Content-Disposition trick is an interesting idea. I'll consider that!

[mickeyreiss]

@mziccard any chance you can cut a point release that includes this fix?

[mziccard]

Ehi @mickeyreiss this is already out as part of google-cloud 0.5.0, see release notes

[mickeyreiss]

@mziccard Sorry for the noise. Thanks!