Bug 40423 - Handling x-koha-request-id relies on wrong data structure
Summary: Handling x-koha-request-id relies on wrong data structure
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks: 40424
  Show dependency treegraph
 
Reported: 2025-07-17 02:30 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-07-23 22:43 UTC (History)
4 users (show)

See Also:
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:


Attachments
Bug 40423: Unit tests (1.55 KB, patch)
2025-07-17 02:56 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 40423: Add helper to stash x-koha-request-id header value (4.04 KB, patch)
2025-07-17 02:56 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 40423: Unit tests (1.61 KB, patch)
2025-07-17 15:53 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 40423: Add helper to stash x-koha-request-id header value (4.10 KB, patch)
2025-07-17 15:53 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 40423: Unit tests (1.67 KB, patch)
2025-07-23 14:20 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 40423: Add helper to stash x-koha-request-id header value (4.16 KB, patch)
2025-07-23 14:20 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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