Skip to content

Accurately reflect the canonical casing of API-Version and OS-Type headers#49054

Merged
thaJeztah merged 1 commit intomoby:masterfrom
maggie44:apiversion
Dec 16, 2024
Merged

Accurately reflect the canonical casing of API-Version and OS-Type headers#49054
thaJeztah merged 1 commit intomoby:masterfrom
maggie44:apiversion

Conversation

@maggie44
Copy link
Copy Markdown
Contributor

@maggie44 maggie44 commented Dec 8, 2024

Go automatically canonicalises HTTP headers, meaning the string API-Version passed as a header has always been returned as Api-Version. Similarly, OSType is returned as Ostype.

This commit updates the documentation to reflect this behaviour and modifies the codebase to ensure that input strings are aligned with their canonical output values.

Update docs and code to reflect Go’s automatic canonicalisation of Api-Version and Ostype headers.

Closes: #49046 (comment)

@maggie44 maggie44 force-pushed the apiversion branch 2 times, most recently from f84d374 to 510b8b4 Compare December 8, 2024 22:20
@maggie44 maggie44 changed the title Update documentation to reflect the canonical casing of API-Version and OS-Type headers Accurately reflect the canonical casing of API-Version and OS-Type headers Dec 8, 2024
…` headers

Go automatically canonicalises HTTP headers, meaning the string `API-Version` passed as a header has always been returned as `Api-Version`. Similarly, `OSType` is returned as `Ostype`.

This commit updates the documentation to reflect this behaviour and modifies the codebase to ensure that input strings are aligned with their canonical output values.

Signed-off-by: maggie44 <[email protected]>
@maggie44 maggie44 changed the title Accurately reflect the canonical casing of API-Version and OS-Type headers Accurately reflect the canonical casing of API-Version and OS-Type headers Dec 8, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Dec 9, 2024
Comment on lines -70 to +71
w.Header().Set("API-Version", v.defaultAPIVersion)
w.Header().Set("OSType", runtime.GOOS)
w.Header().Set("Api-Version", v.defaultAPIVersion)
w.Header().Set("Ostype", runtime.GOOS)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For others reviewing; I suggested changing these to match the normalised version. Effectively these parts of the change are a "no-op", but explicitly setting the headers in the normalised form (instead of them implicitly being normalised by Go stdlib) reduces confusion.

As a follow-up to this PR I want to have a peek if we could define consts for this (on which we could document why they're capitalised as they are), but I have not yet looked at the impact of that (i.e.; where to best put the const without that resulting in cumbersome imports / inter-package dependencies).

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API docs incorrectly suggest the api version header is not canonicalised: API-Version

3 participants