Skip to content

fix: components to be included in the signatureHeaders#77

Merged
thibmeu merged 1 commit intocloudflare:mainfrom
bcotrim-cloudflare:fix-components-to-be-included-in-the-signature
Mar 6, 2026
Merged

fix: components to be included in the signatureHeaders#77
thibmeu merged 1 commit intocloudflare:mainfrom
bcotrim-cloudflare:fix-components-to-be-included-in-the-signature

Conversation

@bcotrim-cloudflare
Copy link
Contributor

Fix: custom components crash in getSigningOptions()

Problem

Passing custom components to signatureHeaders() crashes with:

TypeError: Cannot read properties of undefined (reading 'indexOf')

This happens because getSigningOptions() has two bugs on the same line:

if (signatureAgent && components.indexOf("SIGNATURE_AGENT_HEADER") === -1) {
  1. components is an uninitialized local variable — should be params.components
  2. "SIGNATURE_AGENT_HEADER" is a string literal — should be the variable SIGNATURE_AGENT_HEADER ("signature-agent")
    The bug was never caught because the default code path (no custom components) skips this branch entirely.

Fix

- if (signatureAgent && components.indexOf("SIGNATURE_AGENT_HEADER") === -1) {
+ if (signatureAgent && params.components.indexOf(SIGNATURE_AGENT_HEADER) === -1) {

Tests

Added tests covering custom components:

  • Signing with extra headers (e.g. crawler-max-price) includes them in Signature-Input
  • Rejects missing signature-agent when the header is present on the request
  • Allows its absence when the header is not set

Copy link
Collaborator

@thibmeu thibmeu left a comment

Choose a reason for hiding this comment

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

looks good overall. one suggestion to make it work for components with parameters, and another suggestion to use a generic name for headers in test instead of being tied to a product

@bcotrim-cloudflare bcotrim-cloudflare force-pushed the fix-components-to-be-included-in-the-signature branch from 5b81ffd to ef97940 Compare March 6, 2026 14:32
@bcotrim-cloudflare
Copy link
Contributor Author

looks good overall. one suggestion to make it work for components with parameters, and another suggestion to use a generic name for headers in test instead of being tied to a product

Thanks for the review, I squash committed all you improvements :)

@thibmeu thibmeu merged commit 0b4723f into cloudflare:main Mar 6, 2026
3 checks passed
@bcotrim-cloudflare bcotrim-cloudflare deleted the fix-components-to-be-included-in-the-signature branch March 6, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants