If an OPAC message is pasted into a Koha patron's record which includes a carriage return, it breaks the line and SIP. A user at the Kiosk may find they can no longer log in to the SIP kiosk. The SIP2 specification uses the carriage return as the message terminator.
Created attachment 185583 [details] [review] Bug 40675 Removes carriage returns from messages to prevent breaking SIP Test 1. Find a user and click "Add message". 2. Choose "Add message for OPAC ... " from dropdown 3. In Word (or similar, type a message adding a carriage return in the middle of it. 4. Copy / paste this text into the message box and save. 5. Run the SIP emulator tool sip_cli_emulator.pl and observe that after the carriage return, the message has broken onto a new line. 6. Install patch 7. restart_all 8. Repeat above 1-5 9. Observe that runaway line is now joined to the previous line (correctly)
Created attachment 185791 [details] [review] Bug 40675 Removes carriage returns from messages to prevent breaking SIP Test 1. Find a user and click "Add message". 2. Choose "Add message for OPAC ... " from dropdown 3. In Word (or similar, type a message adding a carriage return in the middle of it. 4. Copy / paste this text into the message box and save. 5. Run the SIP emulator tool sip_cli_emulator.pl and observe that after the carriage return, the message has broken onto a new line. 6. Install patch 7. restart_all 8. Repeat above 1-5 9. Observe that runaway line is now joined to the previous line (correctly) Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Created attachment 185792 [details] [review] Bug 40675 Removes carriage returns from messages to prevent breaking SIP - new regex to prevent regression. I am told my original patch could cause a regression and should instead be $value =~ s/\r\n|\r|\n/ /g; "We probably really want to normalise all options to a space as opposed to switching options. My original change switches us from Mac CR characters to Windows combination of CRLF (and unit tends to just stick to LF in the olden days).. so to really fix the bug we should look for all three as above." Test 1. Find a user and click "Add message". 2. Choose "Add message for OPAC ... " from dropdown 3. In Word (or similar, type a message adding a carriage return in the middle of it. 4. Copy / paste this text into the message box and save. 5. Run the SIP emulator tool sip_cli_emulator.pl and observe that after the carriage return, the message has broken onto a new line. 6. Install patch 7. restart_all 8. Repeat above 1-5 9. Observe that runaway line is now joined to the previous line (correctly)
Created attachment 185793 [details] [review] Bug 40675 Removes carriage returns from messages to prevent breaking SIP Test 1. Find a user and click "Add message". 2. Choose "Add message for OPAC ... " from dropdown 3. In Word (or similar, type a message adding a carriage return in the middle of it. 4. Copy / paste this text into the message box and save. 5. Run the SIP emulator tool sip_cli_emulator.pl and observe that after the carriage return, the message has broken onto a new line. 6. Install patch 7. restart_all 8. Repeat above 1-5 9. Observe that runaway line is now joined to the previous line (correctly)
Created attachment 185794 [details] [review] Bug 40675 Removes carriage returns from messages to prevent breaking SIP - new regex to prevent regression. I am told my original patch could cause a regression and should instead be $value =~ s/\r\n|\r|\n/ /g; "We probably really want to normalise all options to a space as opposed to switching options. My original change switches us from Mac CR characters to Windows combination of CRLF (and unit tends to just stick to LF in the olden days).. so to really fix the bug we should look for all three as above." Test 1. Find a user and click "Add message". 2. Choose "Add message for OPAC ... " from dropdown 3. In Word (or similar, type a message adding a carriage return in the middle of it. 4. Copy / paste this text into the message box and save. 5. Run the SIP emulator tool sip_cli_emulator.pl and observe that after the carriage return, the message has broken onto a new line. 6. Install patch 7. restart_all 8. Repeat above 1-5 9. Observe that runaway line is now joined to the previous line (correctly)
Created attachment 185802 [details] [review] Bug 40675: Added new regex to prevent regression I am told my original patch could cause a regression and should instead be $value =~ s/\r\n|\r|\n/ /g; "We probably really want to normalise all options to a space as opposed to switching options. My original change switches us from Mac CR characters to Windows combination of CRLF (and unit tends to just stick to LF in the olden days).. so to really fix the bug we should look for all three as above."
Created attachment 185833 [details] [review] Bug 40675: Fix SIP message corruption from embedded line endings Patron messages containing carriage returns, line feeds, or CRLF combinations can break SIP protocol communication since CR is used as the SIP message terminator. This causes message parsing failures and protocol errors in self-check systems. The previous regex only handled single carriage returns (\r), missing Windows CRLF (\r\n) and Unix LF (\n) sequences. This patch normalizes all line ending variations to spaces to prevent SIP message corruption while preserving message content. Test plan: 1. Go to a patron record and click "Add message" 2. Select "Add message for OPAC..." from dropdown 3. In a text editor (e.g., Word), create a message with line breaks: "This is line one[Enter]This is line two[Enter]This is line three" 4. Copy and paste this multi-line text into the message box and save 5. Run the SIP emulator: misc/sip_cli_emulator.pl 6. Execute a patron information request for this patron 7. Before patch: Observe message breaks at line endings, corrupting the SIP response 8. Apply patch and restart SIP server 9. Repeat steps 5-6 10. After patch: Verify the message appears as single line with spaces replacing line breaks: "This is line one This is line two This is line three" 11. Test with different line ending types (Windows, Mac, Unix) to ensure all variations are handled correctly Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Please add a test for a module change :)
Created attachment 186456 [details] [review] Bug 40675: Fix SIP message corruption from embedded line endings Patron messages containing carriage returns, line feeds, or CRLF combinations can break SIP protocol communication since CR is used as the SIP message terminator. This causes message parsing failures and protocol errors in self-check systems. The previous regex only handled single carriage returns (\r), missing Windows CRLF (\r\n) and Unix LF (\n) sequences. This patch normalizes all line ending variations to spaces to prevent SIP message corruption while preserving message content. Test plan: 1. Go to a patron record and click "Add message" 2. Select "Add message for OPAC..." from dropdown 3. In a text editor (e.g., Word), create a message with line breaks: "This is line one[Enter]This is line two[Enter]This is line three" 4. Copy and paste this multi-line text into the message box and save 5. Run the SIP emulator: misc/sip_cli_emulator.pl 6. Execute a patron information request for this patron 7. Before patch: Observe message breaks at line endings, corrupting the SIP response 8. Apply patch and restart SIP server 9. Repeat steps 5-6 10. After patch: Verify the message appears as single line with spaces replacing line breaks: "This is line one This is line two This is line three" 11. Test with different line ending types (Windows, Mac, Unix) to ensure all variations are handled correctly Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 186457 [details] [review] Bug 40675: Add unit tests for SIP line ending normalization This patch adds comprehensive unit tests for the add_field function to verify that all types of line endings (Unix \n, Windows \r\n, Mac \r, and mixed) are properly normalized to spaces to prevent SIP message corruption. Tests cover: - Unix line endings (\n) - Windows line endings (\r\n) - Mac line endings (\r) - Mixed line ending combinations - Normal text without line endings - Empty values
Looking here
Created attachment 186608 [details] [review] Bug 40675: Fix SIP message corruption from embedded line endings Patron messages containing carriage returns, line feeds, or CRLF combinations can break SIP protocol communication since CR is used as the SIP message terminator. This causes message parsing failures and protocol errors in self-check systems. The previous regex only handled single carriage returns (\r), missing Windows CRLF (\r\n) and Unix LF (\n) sequences. This patch normalizes all line ending variations to spaces to prevent SIP message corruption while preserving message content. Test plan: 1. Go to a patron record and click "Add message" 2. Select "Add message for OPAC..." from dropdown 3. In a text editor (e.g., Word), create a message with line breaks: "This is line one[Enter]This is line two[Enter]This is line three" 4. Copy and paste this multi-line text into the message box and save 5. Run the SIP emulator: misc/sip_cli_emulator.pl 6. Execute a patron information request for this patron 7. Before patch: Observe message breaks at line endings, corrupting the SIP response 8. Apply patch and restart SIP server 9. Repeat steps 5-6 10. After patch: Verify the message appears as single line with spaces replacing line breaks: "This is line one This is line two This is line three" 11. Test with different line ending types (Windows, Mac, Unix) to ensure all variations are handled correctly Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Adjusted the comment in Sip.pm
Created attachment 186609 [details] [review] Bug 40675: Add unit tests for SIP line ending normalization This patch adds comprehensive unit tests for the add_field function to verify that all types of line endings (Unix \n, Windows \r\n, Mac \r, and mixed) are properly normalized to spaces to prevent SIP message corruption. Tests cover: - Unix line endings (\n) - Windows line endings (\r\n) - Mac line endings (\r) - Mixed line ending combinations - Normal text without line endings - Empty values
Created attachment 186610 [details] [review] Bug 40675: Add unit tests for SIP line ending normalization This patch adds comprehensive unit tests for the add_field function to verify that all types of line endings (Unix \n, Windows \r\n, Mac \r, and mixed) are properly normalized to spaces to prevent SIP message corruption. Tests cover: - Unix line endings (\n) - Windows line endings (\r\n) - Mac line endings (\r) - Mixed line ending combinations - Normal text without line endings - Empty values Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Nice work everyone! Pushed to main for 25.11
Nice work everyone! Pushed to 25.05.x
Pushed to 24.11.x for 24.11.10