Skip to content

Append newline to end of extensionHeaders if necessary#1317

Closed
aknuds1 wants to merge 2 commits intogoogleapis:masterfrom
aknuds1:extensionheaders
Closed

Append newline to end of extensionHeaders if necessary#1317
aknuds1 wants to merge 2 commits intogoogleapis:masterfrom
aknuds1:extensionheaders

Conversation

@aknuds1
Copy link
Copy Markdown
Contributor

@aknuds1 aknuds1 commented May 14, 2016

Fixes #1316.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 14, 2016
@stephenplusplus
Copy link
Copy Markdown
Contributor

Thank you for this! What do you think about accepting an object for headers, e.g.

file.getSignedUrl({
  extensionHeaders: {
    'x-goog-acl': 'public-read',
    'x-goog-meta-foo': 'bar'
  },
  // ...

File.prototype.getSignedUrl = function(options, callback) {
  ...

  var extensionHeadersString;

  if (options.extensionHeaders) {
    for (var headerName in options.extensionHeaders) {
      extensionHeadersString += format('{name}:{value}\n', {
        name: headerName,
        value: options.extensionHeaders[headerName]
      });
    }
  }

  ...
};

@stephenplusplus stephenplusplus added the api: storage Issues related to the Cloud Storage API. label May 14, 2016
@aknuds1
Copy link
Copy Markdown
Contributor Author

aknuds1 commented May 14, 2016

@stephenplusplus I also thought that an object would be better, but I thought about maintaining backwards compatibility and that it would be easier to fix the existing interface (extension headers represented by a string) if it's supposed to be supported in the future.

If backwards compatibility doesn't matter, I'd definitely prefer representing extension headers just as an object.

@stephenplusplus
Copy link
Copy Markdown
Contributor

Thank you for thinking about the backwards compatibility issue, but it's okay to break it if we've discovered an easier API. We will just bump the minor to follow along with semver.

@aknuds1
Copy link
Copy Markdown
Contributor Author

aknuds1 commented May 14, 2016

Do you want me to update the PR to represent extension headers as an object then, @stephenplusplus?

@stephenplusplus
Copy link
Copy Markdown
Contributor

If you're up for it, that would be great 👍

@aknuds1
Copy link
Copy Markdown
Contributor Author

aknuds1 commented May 14, 2016

Will do then.

@aknuds1
Copy link
Copy Markdown
Contributor Author

aknuds1 commented May 14, 2016

@stephenplusplus Done!

@stephenplusplus
Copy link
Copy Markdown
Contributor

Merged in via 17e510b. Thanks again @aknuds1!

miguelvelezsa pushed a commit that referenced this pull request Jul 23, 2025
Otherwise the generated `protos.d.ts` does not look good.
sofisl pushed a commit that referenced this pull request Feb 25, 2026
* fix(docs): explain manulal pagination

* chore: add sample

* chore: simplify the sample

* chore: address PR comments

* chore: lint

* chore: move the line

* chore: one more log
sofisl pushed a commit that referenced this pull request Feb 26, 2026
This update unblocks generating IAM v1 and several more libraries that
were previously blocked by their package names.
Committer: @alexander-fenster
PiperOrigin-RevId: 361273630

Source-Author: Google APIs <noreply@google.com>
Source-Date: Fri Mar 5 20:02:38 2021 -0800
Source-Repo: googleapis/googleapis
Source-Sha: 5477122b3e8037a1dc5bc920536158edbd151dc4
Source-Link: googleapis/googleapis@5477122

Co-authored-by: skuruppu <skuruppu@google.com>
GautamSharda pushed a commit that referenced this pull request Mar 5, 2026
because the tools are already installed in the docker image as of googleapis/testing-infra-docker#227
Source-Link: googleapis/synthtool@ab7384e
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:bb493bf01d28519e82ab61c490c20122c85a7119c03a978ad0c34b4239fbad15
thiyaguk09 pushed a commit to thiyaguk09/google-cloud-node-fork that referenced this pull request Mar 18, 2026
…#1317)

* fix(docs): explain manulal pagination

* chore: add sample

* chore: simplify the sample

* chore: address PR comments

* chore: lint

* chore: move the line

* chore: one more log
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. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants