Bug 31104 - Pagination generates HTTP "Link:" header which is over 8192 bytes apache's limit
Summary: Pagination generates HTTP "Link:" header which is over 8192 bytes apache's limit
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Keywords: rel_22_05_candidate
Depends on:
Blocks:
 
Reported: 2022-07-05 14:29 UTC by Andrii Nugged
Modified: 2023-06-15 17:43 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00


Attachments
Bug 31104: Put each link on a separate header (2.23 KB, patch)
2022-07-06 19:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31104: Put each link on a separate header (2.28 KB, patch)
2022-07-08 16:31 UTC, Michal Urban
Details | Diff | Splinter Review
Bug 31104: Put each link on a separate header (2.34 KB, patch)
2022-07-11 14:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31104: Add a selenium test (1.56 KB, patch)
2022-07-12 08:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 31104: Add a selenium test (2.21 KB, patch)
2022-07-12 13:36 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrii Nugged 2022-07-05 14:29:49 UTC
to repeat:

1. Go to /cgi-bin/koha/members/member.pl

2. put this test into the "Search for patron" "Search:" field:
   "some long test to search in patron fields"

3. Observe "nothing found" but in the server log, it will say

(28)No space left on device: [client 127.0.0.1:60330] AH10124: header size is over the limit allowed by ResponseFieldSize (8192 bytes). Bad response header: 'Link: <https://...

this of course happens when you searching for something long but seems we came to so long URL which fails to fit into the "forward" (Link: ... header) field, 

if to debug, it creates a "search" API request URL that is already 5219 bytes long, then this URL is re-encoded and put two times inside the "Link:" header, so it quickly jumps over the 8192 bytes header limit.

Note: as we found together that solution in Koha/REST/Plugin/Pagination.pm to split @links in separate "Link:" headers.

SIDENOTE: considering that we already have URL 5129 long in this example, we might anyway even without header doubling bump into the 8192 limits (!). We might need re-think later how in general this formed up. Not related to the current shorter fix though.
Comment 1 Tomás Cohen Arazi 2022-07-06 19:37:35 UTC
Created attachment 137240 [details] [review]
Bug 31104: Put each link on a separate header

This patch makes the Pagination helper put each Link entry on the
response headers on a separate header. In practice, this leads to
shorter headers and will avoid apache default/reasonable limits.

To test:
1. Open the patron search page
2. Open the koha logs:
   $ tail -f /var/log/koha/kohadev/*.log
3. On the 'Search for patron' input, use a really long string, like
   'superlongstringthatistoolongforapacheyeahsuperlong'
=> SUCCESS: There's no user with an attribute with that content :-D
=> FAIL: You get an error like (28)No space left on device: [client 127.0.0.1:60330] AH10124: header size is over the limit allowed by ResponseFieldSize (8192 bytes). Bad response header: 'Link:...'
4. Apply this patch
5. Restart all:
   $ restart_all
6. Repeat 2 and 3
=> SUCCESS: No results
=> SUCCESS: No error in the logs
7. Run:
   $ kshell
  k$ prove t/Koha/REST/Plugin/Pagination.t \
           t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass!

Note 1: The Link header is expected to be in CSV format, which is
usually the way libraries treat repeated headers anyway.

Note 2: Apache is not rejecting the response, just stripping out the
'bad header'. So this is not a critical issue.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 David Nind 2022-07-07 09:16:12 UTC
I'm not seeing the error described in step 3.

Are there some other settings required to generate the error?

Testing on master with koha-testing-docker.
Comment 3 Tomás Cohen Arazi 2022-07-07 18:17:30 UTC
(In reply to David Nind from comment #2)
> I'm not seeing the error described in step 3.

I had to put a longer and longer string to reach the limit. Just add some random chars.
Comment 4 Michal Urban 2022-07-08 16:31:08 UTC
Created attachment 137433 [details] [review]
Bug 31104: Put each link on a separate header

This patch makes the Pagination helper put each Link entry on the
response headers on a separate header. In practice, this leads to
shorter headers and will avoid apache default/reasonable limits.

To test:
1. Open the patron search page
2. Open the koha logs:
   $ tail -f /var/log/koha/kohadev/*.log
3. On the 'Search for patron' input, use a really long string, like
   'superlongstringthatistoolongforapacheyeahsuperlong'
=> SUCCESS: There's no user with an attribute with that content :-D
=> FAIL: You get an error like (28)No space left on device: [client 127.0.0.1:60330] AH10124: header size is over the limit allowed by ResponseFieldSize (8192 bytes). Bad response header: 'Link:...'
4. Apply this patch
5. Restart all:
   $ restart_all
6. Repeat 2 and 3
=> SUCCESS: No results
=> SUCCESS: No error in the logs
7. Run:
   $ kshell
  k$ prove t/Koha/REST/Plugin/Pagination.t \
           t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass!

Note 1: The Link header is expected to be in CSV format, which is
usually the way libraries treat repeated headers anyway.

Note 2: Apache is not rejecting the response, just stripping out the
'bad header'. So this is not a critical issue.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Michal Urban <michalurban177@gmail.com>
Comment 5 Martin Renvoize 2022-07-11 14:31:37 UTC
Created attachment 137517 [details] [review]
Bug 31104: Put each link on a separate header

This patch makes the Pagination helper put each Link entry on the
response headers on a separate header. In practice, this leads to
shorter headers and will avoid apache default/reasonable limits.

To test:
1. Open the patron search page
2. Open the koha logs:
   $ tail -f /var/log/koha/kohadev/*.log
3. On the 'Search for patron' input, use a really long string, like
   'superlongstringthatistoolongforapacheyeahsuperlong'
=> SUCCESS: There's no user with an attribute with that content :-D
=> FAIL: You get an error like (28)No space left on device: [client 127.0.0.1:60330] AH10124: header size is over the limit allowed by ResponseFieldSize (8192 bytes). Bad response header: 'Link:...'
4. Apply this patch
5. Restart all:
   $ restart_all
6. Repeat 2 and 3
=> SUCCESS: No results
=> SUCCESS: No error in the logs
7. Run:
   $ kshell
  k$ prove t/Koha/REST/Plugin/Pagination.t \
           t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass!

Note 1: The Link header is expected to be in CSV format, which is
usually the way libraries treat repeated headers anyway.

Note 2: Apache is not rejecting the response, just stripping out the
'bad header'. So this is not a critical issue.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Michal Urban <michalurban177@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2022-07-11 14:35:11 UTC
Nice clear fix Tomas, thanks.

All works as expected.. perhaps it might have been nice to see a unit test for it, but otherwise, I'm happy here.

Passing QA.
Comment 7 Tomás Cohen Arazi 2022-07-11 21:24:05 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 8 Jonathan Druart 2022-07-12 08:25:47 UTC
Created attachment 137610 [details] [review]
Bug 31104: Add a selenium test
Comment 9 Jonathan Druart 2022-07-12 08:26:07 UTC
Will still pass after bug 31138.
Comment 10 Tomás Cohen Arazi 2022-07-12 13:06:22 UTC
Thanks, Jonathan!
Comment 11 Jonathan Druart 2022-07-12 13:36:42 UTC
Created attachment 137628 [details] [review]
Bug 31104: Add a selenium test
Comment 12 Jonathan Druart 2022-07-25 11:57:01 UTC
No sure what's going on here, but the selenium tests are failing on jenkins.

They are passing with and without commit "Bug 31104: Put each link on a separate header"

They are failing after bug 31138.

And actually this patch does not fix the problem, I am still getting "414
Request-URI Too Long".
Comment 13 Tomás Cohen Arazi 2022-08-08 18:49:18 UTC
Tests reverted.