Skip to content

Fix JWT::JWK::Set sharing its key collection when copied - #751

Open
anakinj wants to merge 1 commit into
jwt:mainfrom
anakinj:fix/jwk-set-key-array-ownership
Open

Fix JWT::JWK::Set sharing its key collection when copied#751
anakinj wants to merge 1 commit into
jwt:mainfrom
anakinj:fix/jwk-set-key-array-ownership

Conversation

@anakinj

@anakinj anakinj commented Sep 2, 2026

Copy link
Copy Markdown
Member

Description

Fixes #750.

JWT::JWK::Set.new(other_set) returned the source set's keys array by reference, despite the # Simple duplication comment on that branch. The class also defined no initialize_copy, so dup and clone shared the array as well — a case not covered in the issue.

Mutating a copy through add, <<, delete, select!, reject! or uniq! therefore mutated the original too:

original = JWT::JWK::Set.new([jwk])
JWT::JWK::Set.new(original).reject! { true }
original.size # => 0

This matters because key sets are commonly cached and shared between consumers, so one caller filtering its own copy can silently empty another's.

union was unaffected, since merge rebinds @keys with += rather than mutating in place. There is a regression test for that too.

Both copy paths now dup the array. The JWT::JWK objects themselves are still shared — only the collection is copied.

Checklist

Before the PR can be merged be sure the following are checked:

  • There are tests for the fix or feature added/changed
  • A description of the changes and a reference to the PR has been added to CHANGELOG.md. More details in the CONTRIBUTING.md

@anakinj
anakinj force-pushed the fix/jwk-set-key-array-ownership branch from 4ddf558 to d4b2413 Compare September 2, 2026 13:49
JWT::JWK::Set.new(other_set) returned the source set's `keys` array by
reference, despite the "Simple duplication" comment on that branch. The
class also defined no `initialize_copy`, so `dup` and `clone` shared the
array as well.

Mutating a copy through `add`, `<<`, `delete`, `select!`, `reject!` or
`uniq!` therefore mutated the original too, which matters because key
sets are commonly cached and shared between consumers. `union` was
unaffected, as `merge` rebinds `@keys` with `+=` instead of mutating.

Both paths now copy the array. The JWK objects themselves stay shared.

Fixes jwt#750
@anakinj
anakinj force-pushed the fix/jwk-set-key-array-ownership branch from d4b2413 to ce143f6 Compare September 2, 2026 14:01
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.

Constructing a JWK::Set from another set aliases its key array

1 participant