Google-cloud-node: @google-cloud/storage file getSignedUrl returns trailing dot in URL

[ShaharHD]

Upon updating @google-cloud/storage to the latest package version (1.1.0) introduced a trailing dot in the url.
For example: https://storage.googleapis.com./<bucket name>/<file name>...

I've traced the cause of this due to GoogleCloudPlatform/google-cloud-node#2214

It seems valid by FQDN, on iOS, curl and wget no issue, but on Android: HTTP FAILED: javax.net.ssl.SSLHandshakeException: java.lang.IllegalArgumentException: Server name value of host_name cannot have the trailing dot
(using OkHttp and Fresco)

Is this by design?

Environment details

• OS: Heroku and MacOS
• Node.js: 6.10.2
• yarn version: 0.23.2
• "@google-cloud/storage": "^1.1.0"

Steps to reproduce

1. generate a signedUrl for file

On Stackoverflow

[stephenplusplus]

@lukesneeringer should we:

1. Not revert the trailing . change in getSignedUrl()
2. Revert the trailing . change in getSignedUrl()
3. Revert the trailing . change in the Storage API
4. Revert the trailing . change in our whole API

[shaibt]

Can also confirm azure based services cannot use these trailing dot URLs provided by getSignedUrl - will fail to send an HTTP request

[ShaharHD]

@shaibt I've updated on SO that our solution for the problem was to add a temporary URI overwriter (to remove the trailing dot, might be done very efficiently using just a regex, but we choose to parse and reconstruct after fixing the the hostname trailing dot).

We were actually amazed that Alamofire and iOS handled it pretty good compare to android (and Java)

[stephenplusplus]

Thanks for sharing, and here's just a quick copy-paste for anyone who needs it:

file.getSignedUrl(..., function(err, url) {
  url = url.replace('storage.googleapis.com.', 'storage.googleapis.com')
})

[ShaharHD]

@stephenplusplus this is a work around for a problem which was introduced to solve (and please correct me if I'm wrong) an issue on google backend.

If all Java and C# based clients will simply overwrite the fix, could it be the fix is actually ignored by simply using more CPU time globally?

[stephenplusplus]

I'm honestly not sure-- I didn't investigate much into the reasoning behind the change, but it was discussed internally and decided by @lukesneeringer that it was something we wanted. My find/replace above is just a temporary workaround for anyone else that might need it, while we investigate a final solution.

[ShaharHD]

@stephenplusplus FYI GoogleCloudPlatform/google-cloud-node#2214 was IMHO an opinionated discussion with @lukesneeringer being a minority "No" person.

I think the decision was made without being aware to the limitations of other environments support for "Absolute domain name" (which I agree is lacking, but simply because it isn't widespread)

IMHO many will agree that this change needs to be reverted ASAP (as 1.1.1), as once more node.js based deployments will pull the 1.1.0 version, many remote clients which uses signed URLs will start failing on invalid URLs.
This isn't some some minor component failing, we're talking about all Android devices (using javax.net.ssl)

[stephenplusplus]

This comment is the basis of my earlier statement regarding why the change was introduced. The big decisions that affect our library go through @lukesneeringer, so when he's available, he will be able to address your feedback.

[ShaharHD]

@stephenplusplus Adding on your comment before, I'll add a 5.

1. Have a flag (defaulted to false) which add the trailing . to all requests (in whichever part of the API you see fit...)

[stephenplusplus]

cc @swcloud

[stephenplusplus]

We're going to revert this library-wide. Thanks for your patience while we sorted this out.

[ShaharHD]

@stephenplusplus is there an ETA for the npm package to be updated?

[lukesneeringer]

@stephenplusplus Can you release a 1.1.1 today to fix this on Android?

Ironically, I got a request from another customer asking for the addition of the trailing . in all seven languages. I think we should potentially consider a flag. It is increasingly clear that some people really do need this. My original reason for saying no in #2214 was essentially, "We should not opt everyone in to this -- it might not work everywhere." But giving folks a _way to_ get this behavior clearly has value.

[stephenplusplus]

@callmehiphop are you available for a release?

[stephenplusplus]

1.1.1 released.

[wyardley]

Would it be possible to set the Host: header without the trailing dot but use the hostname with it for the target?