Bug 40423

Summary: Handling x-koha-request-id relies on wrong data structure
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Pushed to main --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, martin.renvoize, matt.blenkinsop, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.11.00
Circulation function:
Bug Depends on:    
Bug Blocks: 40424    
Attachments: Bug 40423: Unit tests
Bug 40423: Add helper to stash x-koha-request-id header value
Bug 40423: Unit tests
Bug 40423: Add helper to stash x-koha-request-id header value
Bug 40423: Unit tests
Bug 40423: Add helper to stash x-koha-request-id header value

Description Tomás Cohen Arazi (tcohen) 2025-07-17 02:30:24 UTC
Processing the `x-koha-request-id` header relies on the fact the header is added to the 'reserved_params' earlier by the use of `$c->validation->output`.

This is fragile as it relies on an internal data structure from the `Mojolicious::Plugin::OpenAPI` plugin that is not safe or intended to be accessed.

Bug 40417 deals with removing that last use in `objects.search_rs` but highlights this situation. In order to keep things clear, I file this bug for dealing with this particular situation.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-07-17 02:56:18 UTC
Created attachment 184179 [details] [review]
Bug 40423: Unit tests
Comment 2 Tomás Cohen Arazi (tcohen) 2025-07-17 02:56:21 UTC
Created attachment 184180 [details] [review]
Bug 40423: Add helper to stash x-koha-request-id header value

This patch aligns handling of the header with how other headers are
managed.

* A helper is added which reads the header and stashes the value
* The helper is called in the same place similar helpers are called
  (authenticate_api_request)
* The `objects.search_rs` helper is adjusted to use the stashed value
  instead of picking it from the reserved params.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ prove t/Koha/REST/Plugin/Query.t \
           t/db_dependent/api/v1/*.t \
           t/db_dependent/Koha/REST/Plugin/*
=> SUCCESS: Tests pass! No behavior change!
3. Sign off :-D
Comment 3 Matt Blenkinsop 2025-07-17 15:53:44 UTC
Created attachment 184216 [details] [review]
Bug 40423: Unit tests

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 4 Matt Blenkinsop 2025-07-17 15:53:47 UTC
Created attachment 184217 [details] [review]
Bug 40423: Add helper to stash x-koha-request-id header value

This patch aligns handling of the header with how other headers are
managed.

* A helper is added which reads the header and stashes the value
* The helper is called in the same place similar helpers are called
  (authenticate_api_request)
* The `objects.search_rs` helper is adjusted to use the stashed value
  instead of picking it from the reserved params.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ prove t/Koha/REST/Plugin/Query.t \
           t/db_dependent/api/v1/*.t \
           t/db_dependent/Koha/REST/Plugin/*
=> SUCCESS: Tests pass! No behavior change!
3. Sign off :-D

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 5 Martin Renvoize (ashimema) 2025-07-23 14:20:52 UTC
Created attachment 184552 [details] [review]
Bug 40423: Unit tests

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 6 Martin Renvoize (ashimema) 2025-07-23 14:20:54 UTC
Created attachment 184553 [details] [review]
Bug 40423: Add helper to stash x-koha-request-id header value

This patch aligns handling of the header with how other headers are
managed.

* A helper is added which reads the header and stashes the value
* The helper is called in the same place similar helpers are called
  (authenticate_api_request)
* The `objects.search_rs` helper is adjusted to use the stashed value
  instead of picking it from the reserved params.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ prove t/Koha/REST/Plugin/Query.t \
           t/db_dependent/api/v1/*.t \
           t/db_dependent/Koha/REST/Plugin/*
=> SUCCESS: Tests pass! No behavior change!
3. Sign off :-D

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 7 Lucas Gass (lukeg) 2025-07-23 22:43:12 UTC
Nice work everyone!

Pushed to main for 25.11