Skip to content

fix(storage): use code-point ordering for canonical headers - #9841

Open
itsrayeedscodespace wants to merge 1 commit into
googleapis:mainfrom
itsrayeedscodespace:fix/9840-storage-v4-header-order
Open

itsrayeedscodespace wants to merge 1 commit into
googleapis:mainfrom
itsrayeedscodespace:fix/9840-storage-v4-header-order

Conversation

@itsrayeedscodespace

Copy link
Copy Markdown

Description

This change sorts canonical signing headers by character code.

Impact

Signed URLs use matching canonical and signed header orders across locales.

Changes

  • Replace locale-dependent localeCompare with direct string comparison after lowercasing header names in URLSigner.getCanonicalHeaders.
  • Match the native string ordering already used by V4 signedHeaders, as required by the GCS canonical request specification. Valid HTTP header names are ASCII, so JavaScript code-unit ordering agrees with code-point ordering here.
  • Preserve value normalization and public APIs. V2 shares this method and also requires code-point ordering.

Testing

  • Add eight V4 cases covering hyphens versus underscores, reverse insertion order, mixed-case names, normalized values, and Lithuanian-sensitive letters under default and explicit Lithuanian collation (when supported).
  • Verify the canonical headers block, canonical request signed header list, returned URL's X-Goog-SignedHeaders, and exact string passed to mocked signing credentials, including the canonical request SHA-256 digest.
  • Add a V2 regression checking the complete signing input with punctuation, mixed case, and normalized values.
  • Red: unmodified implementation plus regression tests: 74 passing, 8 failing. The default punctuation cases fail without requiring a particular process locale.
  • Green: node ../../bin/run-test.cjs --no-c8 --no-parallel build/cjs/test/signer.js: 82 passing; also 82 passing with LANG=lt_LT.UTF-8.
  • pnpm install --frozen-lockfile --filter @google-cloud/storage..., pnpm run compile (CJS and ESM), and pnpm test: 1,112 passing.
  • pnpm run check, pnpm exec prettier --check src/signer.ts test/signer.ts, and git diff --check: passed.
  • Verified with Node.js v26.13.0 and pnpm 10.0.0; no live GCS access or credentials required.

Alternatives

Keeping localeCompare produces signatures inconsistent with the advertised signed header list for valid header names. Locale-specific collation options still do not express the protocol's character-code ordering. Direct comparison of the already-lowercased names is sufficient, requires no helper or API changes, and compares names independently of header values.

  • Existing issue and approved coordination comment precede submission.
  • Tests and linter pass.
  • Regression tests exercise the changed comparison.
  • No public documentation changes are needed.

Fixes #9840

Sort lowercased header names by character code to match the V4 signed header list and avoid locale-dependent signature mismatches. Add V4 signing-input and V2 regression coverage.

Fixes googleapis#9840
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Nov 8, 2026
@google-cla

google-cla Bot commented Nov 8, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the header sorting logic in URLSigner to use code-point order instead of localeCompare for canonical headers, and adds corresponding test cases for V2 and V4 header ordering. The feedback suggests using a regular expression like /\r?\n/ instead of a literal newline character when splitting strings in tests to ensure cross-platform compatibility with Windows line endings.

'host:storage.googleapis.com\n' + testCase.canonical;
const signedHeaders = headers
.trim()
.split('\n')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When splitting string content by lines, use a regular expression like /\r?\n/ instead of '\n' to ensure cross-platform compatibility with Windows line endings (\r\n), and include test cases with \r\n line endings to verify this behavior.

                .split(/\r?\n/)
References
  1. When splitting string content by lines, use a regular expression like /\r?\n/ instead of '\n' to ensure cross-platform compatibility with Windows line endings (\r\n), and include test cases with \r\n line endings to verify this behavior.

@itsrayeedscodespace
itsrayeedscodespace marked this pull request as ready for review November 8, 2026 02:57
@itsrayeedscodespace
itsrayeedscodespace requested review from a team as code owners November 8, 2026 02:57

This branch has not been deployed

No deployments
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

storage: V4 getSignedUrl sorts canonical headers with localeCompare, so they disagree with X-Goog-SignedHeaders

1 participant