Description
Joonas Kylmälä
2021-05-31 08:15:31 UTC
Relating to this is another bug but it only happens with strict_sql_modes = 1, so if we come up with a fix for this we should make sure it handles this case as well.
The other issue is:
> CGI::Session::Driver::mysql::store(): DBI Exception: DBD::mysql::db do
> failed: Incorrect string value: '\xE4iskap...' for column
> `koha_kohadev`.`sessions`.`a_session` at row 1 [for Statement "INSERT INTO
> sessions (id, a_session) VALUES(?, ?) ON DUPLICATE KEY UPDATE a_session =
> ?"] at /usr/share/perl5/CGI/Session.pm line 251
> at /usr/share/perl5/DBIx/Class/Exception.pm line 77
This happens when trying to to set on the page /cgi-bin/koha/circ/set-library.pl the active branch another time to what is selected already AND the branch display contains an "ä" character.
The problem is fixed by enabling the Mariadb/Mysql STRICT_TRANS_TABLES mode. (In reply to Joonas Kylmälä from comment #2) > The problem is fixed by enabling the Mariadb/Mysql STRICT_TRANS_TABLES mode. Even though this fixes it I'm not convinved it is the right fix. Maybe we our output is not UTF8 to the database: https://metacpan.org/pod/CGI::Session#Influencing-the-encoding-of-the-program's-output Also here more on this: https://stackoverflow.com/questions/31049125/perl-cgisession-mysql-thaw-utf-8-data (In reply to Joonas Kylmälä from comment #0) > 1) Set strict_sql_modes = 1 in koha-conf.xml (depending on your test > environment variables you might also just wanna uncomment the relevant code I typoed this, in fact the strict_sql_modes need to be set to 0 to reproduce this. I found another workaround: $datastr = decode_utf8(encode_utf8($datastr)); inside CGI::Session::Driver::mysql::store fixes the problem. The difference seems to be that $datastr is not marked as utf8 string. And after it is (decode_utf8 does it) the string is stored correctly to the DB. Data::Dumper, which the default serializer uses, seems to produce iso-latin-1 strings: https://www.perlmonks.org/?node_id=759457 Our database connection is configured for utf8mb4 so that could be the issue here. I don't know if we can fix this from koha code other than by changing the database connection to iso-latin-1 for the sessions table. Or maybe changing sessions table to iso-latin-1 could make this work automatically? Hm, I think we store some things like a user name in the sessions table, so not sure if changing the table to latin-1 is an option. The weird thing is that it works correctly with characters like ✓ It seems like the problem is that ä in Latin-1 is the same as the unicode code point (E4) which probably confuses some encoding auto-detection mechanism (In reply to Julian Maurice from comment #8) > The weird thing is that it works correctly with characters like ✓ > It seems like the problem is that ä in Latin-1 is the same as the unicode > code point (E4) which probably confuses some encoding auto-detection > mechanism It only confuses the C code implementation of Data::Dumper, if we enable local $Data::Dumper::Useperl = 1; or just put in the default serializer directly the following line > $d->Useperl( 1 ); it starts working. I just don't know if we can make it a workable solution. If I modify sessions.a_session from longtext to longblob I no longer have the bug. Can you test if it works for you too ? Created attachment 121539 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob This column contains serialized data, it should be a blob. And it fixes encoding issues. Test plan: 1. Apply patch and run updatedatabase 2. In $KOHA_CONF make sure that you have <strict_sql_modes>0</strict_sql_modes> 3. Create 2 libraries: a. TESTÄ/Testä b. TEST✓/Test✓ 4. Go to the staff interface, change your library to Testä, then navigate to a few other pages, make sure there is no encoding problem with the library name in the top right corner 5. Do the same with Test✓ Created attachment 121541 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob This column contains serialized data, it should be a blob. And it fixes encoding issues. Test plan: 1. Apply patch and run updatedatabase 2. In $KOHA_CONF make sure that you have <strict_sql_modes>0</strict_sql_modes> 3. Create 2 libraries: a. TESTÄ/Testä b. TEST✓/Test✓ 4. Go to the staff interface, change your library to Testä, then navigate to a few other pages, make sure there is no encoding problem with the library name in the top right corner 5. Do the same with Test✓ Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Yesterday I experimented with the same – I changed to "mediumblob" manually on my local test server to find a solution and it worked. What I can say that I found that Data::Dumper when converting Perl strings scalars acts "fuzzily" (better: weirdly), and if it meets real Unicode chars it stores them in the string in \xXX perl-form, so they stored in DB properly and restored properly, for example, I created branch "Čentervälly": it is stored in MySQL in a_session dumped in this way: 'branchname' => "\x{10c}enterv\x{e4}lly" so here it works and no error happens, but when it meets only single "ä" for example "Hämeenlinna" (and maybe few other characters like that which suspect can be represented in old ASCII latin-1 or how that fuzziness fuzzies :)...) chars, so it stores in such a way in MySQL longtext (broken): 'branchname' => 'H?meenlinna', because: see below hex dump of the blob, and when it saves to UTF-8-MySQL-field – it gets malformed, but yes, with blob it works: 'branchname' => 'Hämeenlinna', and when I looked inside blob in HEX, I see: 48 E4 6D 65 65 6E 6C 69 6E 6E 61 H‰meenlinna because it stores ä as E4 and it is then get returned "in the same way" into "ä" (seems). But oh that's silvertape everywhere :). This is just to say, that it weird but WORKS with blob, so – thanks Julian for the patch, and I say I support it, (yet I'd blame Data::Dumper "fuzziness" :) ) /me still thinks we should've stuck with YAML Julian, thanks for patch, can you also add DB changes to kohastructure.sql as well, not just atomic update, so we can then again Sign-off this? (In reply to Andrew Nugged from comment #15) > Julian, thanks for patch, > > can you also add DB changes to kohastructure.sql as well, not just atomic > update, > > so we can then again Sign-off this? Ah yes I forgot that. I'll write the patch ASAP Created attachment 121563 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob This patch modifies kohastructure.sql Created attachment 121564 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob This column contains serialized data, it should be a blob. And it fixes encoding issues. Test plan: 1. Apply patch and run updatedatabase 2. In $KOHA_CONF make sure that you have <strict_sql_modes>0</strict_sql_modes> 3. Create 2 libraries: a. TESTÄ/Testä b. TEST✓/Test✓ 4. Go to the staff interface, change your library to Testä, then navigate to a few other pages, make sure there is no encoding problem with the library name in the top right corner 5. Do the same with Test✓ Created attachment 121565 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob (kohastructure.sql) Created attachment 121570 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob This column contains serialized data, it should be a blob. And it fixes encoding issues. Test plan: 1. Apply patch and run updatedatabase 2. In $KOHA_CONF make sure that you have <strict_sql_modes>0</strict_sql_modes> 3. Create 2 libraries: a. TESTÄ/Testä b. TEST✓/Test✓ 4. Go to the staff interface, change your library to Testä, then navigate to a few other pages, make sure there is no encoding problem with the library name in the top right corner 5. Do the same with Test✓ Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Created attachment 121571 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob (kohastructure.sql) Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Created attachment 121584 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob This column contains serialized data, it should be a blob. And it fixes encoding issues. Test plan: 1. Apply patch and run updatedatabase 2. In $KOHA_CONF make sure that you have <strict_sql_modes>0</strict_sql_modes> 3. Create 2 libraries: a. TESTÄ/Testä b. TEST✓/Test✓ 4. Go to the staff interface, change your library to Testä, then navigate to a few other pages, make sure there is no encoding problem with the library name in the top right corner 5. Do the same with Test✓ Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Signed-off-by: Andrew Nugged <nugged@gmail.com> Created attachment 121585 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob (kohastructure.sql) Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Signed-off-by: Andrew Nugged <nugged@gmail.com> Futher tests. == Master == 1. Try to hold from the OPAC with delivery to TESTÄ/Testä or TEST✓/Test✓ => ERROR: Internal error: incomplete hold request. (odd, nothing in all the logs, just the HTTP request returning 200) 2. Staff interface set library: TESTÄ/Testä transfer item to any another library => Broken FK constraint at /usr/share/perl5/Exception/Class/Base.pm line 88 Oddly it works with TEST✓/Test✓ 3. Staff interface set library: TESTÄ/Testä checkout an item => Broken FK constraint at /usr/share/perl5/Exception/Class/Base.pm line 88 Oddly it works with TEST✓/Test✓ 4. Staff interface set library: NOT TESTÄ/Testä checkout an item set library: TESTÄ/Testä partron page => checkout tab => show checkouts try to check in => it's loads for ever => logs: Broken FK constraint at /usr/share/perl5/Exception/Class/Base.pm line 88 refresh page The item has actually been checked in. (not sure if DB is in a consistent state) 5. Same but use the check in tab of the search box, next to the Koha logo == Does anyone know why TEST✓/Test✓ doesn't cause issue? At least it should cause as much issues as TESTÄ/Testä. == With patches == 1. fails! 2. passes 3. fails! 4. passes 5. fails! (odd) So, QA fails! Does anyone have more ideas of tests? (In reply to Victor Grousset/tuxayo from comment #24) > == With patches == > 1. fails! > 2. passes > 3. fails! > 4. passes > 5. fails! (odd) > > So, QA fails! Looks like it works better with the patch, so why do you say that ? (In reply to Victor Grousset/tuxayo from comment #24) > Does anyone know why TEST✓/Test✓ doesn't cause issue? At least it should > cause as much issues as TESTÄ/Testä. see comment 8 and comment 13 Cannot recreate 1, but 3: - New library created TESTÄ/TESTÄ - Use it for logged in library - Checkout Invalid value passed, branchcode=\xC4 expected type is string at /usr/share/perl5/Exception/Class/Base.pm line 88 comment 28 > Cannot recreate 1 Ok, it needs also to have TESTÄ/Testä and TEST✓/Test✓ as the library code. The idea was {Asian, Arabic, Pakistanis, Cyrillic} writing libraries (and more) might not always transliterate. Is that actually correct? Or is that out of scope? Are the test plan also out of scope? Assuming one or more of them still fail with ascii library code. Comment 26 > > So, QA fails! > Looks like it works better with the patch, so why do you say that ? Indeed it works better. That was based on the assumption that these tests are in the scope of this bug. Which is uncertain now. I imagine cyrillic in library codes and library names to be a good test. It can be recreated easily using: use Koha::Cities; use CGI::Session::Serialize::default; #$Data::Dumper::Useperl = 1; use utf8; my $branchname = "TESTÄ/Testä"; my $hash = { branch => $branchname }; my $s = CGI::Session::Serialize::default->freeze($hash); $hash = CGI::Session::Serialize::default->thaw($s); Koha::City->new({city_name => $hash->{branch}})->store; It fails with Incorrect string value: '\xC4/Test...' for column 'city_name' at row 1 But inserts the city correctly if the Useperl line is uncommented. Created attachment 121624 [details] [review] Bug 28489: Fix issues with umlaut in session vars This last patch seems to fix the problem, can someone test or give their opinions on it please? > I imagine cyrillic in library codes and library names to be a good test.
I tested naming libraries with cyrillic symbols, like "Бібліотека" and it works
fine with and without the patch. Then, I tested a bunch of other special
characters from different languages, here's the list of them:
These ones work fine without the patch:
ааа
ААА
яяя
ЯЯЯ
ііі
ІІІ
ЄЄЄ
єєє
łłł
ŁŁŁ
ŽŽŽ
Meanwhile these ones get shown as "???" without the patch:
äää
ÄÄÄ
ööö
ÖÖÖ
ààà
ááá
æææ
ÆÆÆ
ååå
ÅÅÅ
ôôô
ÔÔÔ
üüü
ÜÜÜ
ëëë
ËËË
ççç
ććć
meanwhile with patch these symbols are displayed correctly.
> I imagine cyrillic in library codes and library names to be a good test.
I tested naming libraries with cyrillic symbols, like "Бібліотека" and it works
fine with and without the patch. Then, I tested a bunch of other special
characters from different languages, here's the list of them:
These ones work fine without the patch:
ааа
ААА
яяя
ЯЯЯ
ііі
ІІІ
ЄЄЄ
єєє
łłł
ŁŁŁ
ŽŽŽ
ććć
Meanwhile these ones get shown as "???" without the patch:
äää
ÄÄÄ
ööö
ÖÖÖ
ààà
ááá
æææ
ÆÆÆ
ååå
ÅÅÅ
ôôô
ÔÔÔ
üüü
ÜÜÜ
ëëë
ËËË
ççç
meanwhile with patch all these symbols are displayed correctly.
Ok, so with the patch all is good...? (In reply to Victor Grousset/tuxayo from comment #24) > Further tests. > == Master == ... > == With patches == ... Hey, I couldn't reproduce it, I didn't manage to get any errors like that neither on master, nor with two previous patches that were signed off by me previously. Everything seems to magically work on my local koha so I am not sure what the problem is. Wait a bit, Julian and me are trying to come with a better solution. My last patch is removing the different problems listed previously, but it's 1. ugly and 2. incorrect (we only fix the problem in set_userenv whereas it must be fixed at session level). I also don't like direct utf8 flag change on lower level: later it can bring more errors even if we manage for now them not to happen :P. But I also tried with Victor's findings and can't: Jonathan/Julian – have you able to reproduce what Victor said? Hint me how, for me it just worked (comment 24). Julian, Andrew and me worked on it today but we have not found a correct solution so far. The only viable solution would be: 1. For master: Replace CGI::Session with Data::Session using JSON serialization (see bug 17427 and note that there is still a problem there) 2. For 21.05: Reintroduce the YAML dependency and use YAML serialization with CGI::Session (we know it is a working solution and we are failing to provide a fix with Data::Dumper) 3. Developers will test Data::Session in master 4. Eventually backport the Data::Session move into 21.05 Note that the proposed solution on bug 17427 does not require a datatype change (sessions.a_session is still longtext). *** Bug 28517 has been marked as a duplicate of this bug. *** Thought on swimming 4K, thought on walking. Such idea: what if we will create own CGI::Session::Serialize::yamlxs and we're good to go then with all for now? Proof of concept: https://github.com/nugged/Koha/commit/2c62d311fba86576321337f2596283300013dd47 in: https://github.com/nugged/Koha/commits/cgi_session_serialize_yamlxs I there created "CGI::Session::Serialize::yamlxs" package just "inside" of C4::Auth file, to show that solution. Jonathan and Dave: and voila, we might have YAML::XS now for sessions! Thought on swimming 4K, thought on walking. Such idea: what if we will create own CGI::Session::Serialize::yamlxs and we're good to go then with all for now? Proof of concept: https://github.com/nugged/Koha/commit/2c62d311fba86576321337f2596283300013dd47 in: https://github.com/nugged/Koha/commits/cgi_session_serialize_yamlxs I there created "CGI::Session::Serialize::yamlxs" package just "inside" of C4::Auth file, to show that solution. Jonathan and David: and voila, we might have YAML::XS now for sessions! Created attachment 121637 [details] [review] Bug 28489: Add selenium tests Andrew, Yes, that could work as well. I forgot this possibility! I've written some selenium tests to help us testing the different patches/approach. I am running into the same problem I faced with Data::Session and using the file driver: YAML::XS::Load Error: The problem: control characters are not allowed was found at document: 0 Created attachment 121638 [details] [review] Bug 28489: Add selenium tests And those tests are passing on 20.11.x Created attachment 121639 [details] [review] Bug 28489: POD - CGI::Session::Serialize::yamlxs for CGI::Session Created attachment 121640 [details] [review] Bug 28489: Don't deal with encoding during the serialization Looks like there is a 100% working solution using longblob and removing the encode/decode from the YAML::XS serializer. Asking for a SO now, but we will need to provide a better solution to embed the serializer code (having it in C4::Auth is not nice). We discussed with Jonathan about "where better to place the file" but we both agreed (JD, you say) so for a moment we can keep the package in C4/Atuh as additional one for quick-est fix so we can provide working solution for backporting, but the question still open to decide for later (let's create a separate ticket after we accept this for not to forget, like TODO?): of course it's better to keep CGI::Session::Serialize::yamlxs in own file/path, .../CGI/Session/Serialize/yamlxs.pm but that pollutes Koha lib folder - Jonathan said he keeps in mind to create something separate, but that for later discussion. So, mates, for now – let's decide do we accept this 'yamlxs' serializer solution as a temporary but the full fix for this problem and backport, and keep development in master later towards whatever planned (JD said target is Data::Session). Jonathan, should we obsolete the first old two patches? Also, for me after you, Jonathan, removed encoders, I got this error: YAML::XS::Load Error: The problem:: /home/vagrant/kohaclone/mainpage.pl : /home/vagrant/kohaclone/mainpage.pl invalid trailing UTF-8 octet: /home/vagrant/kohaclone/mainpage.pl : /home/vagrant/kohaclone/mainpage.pl was found at document: 0: /home/vagrant/kohaclone/mainpage.pl but with "Encode::decode_utf8(YAML::XS::Dump($data));" so on as it was on beginning – it works. It works for me with longblob (so with all the patches attached on this bug report). ah, right. Longblob, not longtext, my bad then, then yes, it should work. (In reply to Andrew Nugged from comment #51) > We discussed with Jonathan about "where better to place the file" but we > both agreed (JD, you say) so for a moment we can keep the package in C4/Atuh > as additional one for quick-est fix so we can provide working solution for > backporting, > > but the question still open to decide for later (let's create a separate > ticket after we accept this for not to forget, like TODO?): of course it's > better to keep CGI::Session::Serialize::yamlxs in own file/path, > .../CGI/Session/Serialize/yamlxs.pm > but that pollutes Koha lib folder - Jonathan said he keeps in mind to create > something separate, but that for later discussion. Please no. This is how we get into trouble in Koha. We don't need more technical debt. The quickest fix is to revert Bug 28317, and to do this right. But I'll look at putting together a patch so that we can avoid this horror. Created attachment 121648 [details] [review] Bug 28489: Put CGI::Session::Serialize::yamlxs in lib directory This patch adds a "lib" directory to the source tree which gets mapped to the same directory as "C4" and "Koha" for single and standard installations. CGI::Session::Serialize::yamlxs is put into this "lib" directory. This patch also includes some changes so that dev/git installations work as well. Dev test plan: 1. vi /etc/apache2/sites-enabled/kohadev.conf 2. Change "SetEnv PERL5LIB "/kohadevbox/koha" to SetEnv PERL5LIB "/kohadevbox/koha:/kohadevbox/koha/lib" 2b. service apache2 reload 3. cp debian/scripts/koha-functions.sh /usr/share/koha/bin/koha-functions.sh 4. koha-plack --restart kohadev 5. Log into http://localhost:8081/cgi-bin/koha/mainpage.pl 6. Go to CGI script http://localhost:8081/cgi-bin/koha/tools/export.pl (Note that this is still run in CGI even when using Plack) NOTE: We'll need to update koha-gitify (https://gitlab.com/koha-community/koha-gitify) to get that Apache configuration. There's no harm in updating it, since this is a backwards compatible change. See https://gitlab.com/koha-community/koha-gitify/-/merge_requests/2. -- Deploy test plan: Dev: 1. perl Makefile.PL 1a. Installation mode: dev 1b. Configuration directory: /opt/koha-dev 2. make 3. ls blib/PERL_MODULE_DIR/ 4. Note the following: C4 CGI cpanfile Koha Koha.pm 5. make install 5b. Note the output says the following: In order to use Koha's command-line batch jobs, you should set the following environment variables: export KOHA_CONF=/opt/koha-dev/etc/koha-conf.xml export PERL5LIB=/kohadevbox/koha:/kohadevbox/koha/lib 6. vi /opt/koha-dev/etc/koha-httpd.conf 7. Note the following: SetEnv PERL5LIB "/kohadevbox/koha:/kohadevbox/koha/lib" 6. rm -rf /opt/koha-dev 7. make clean Single: 1. perl Makefile.PL 1a. Installation mode: single 1b. Please specify the directory in which to install Koha: /opt/koha 2. make 3. ls blib/PERL_MODULE_DIR/ 4. Note the following: C4 CGI cpanfile Koha Koha.pm 5. make install 5b. Note the output says the following: In order to use Koha's command-line batch jobs, you should set the following environment variables: export KOHA_CONF=/opt/koha/etc/koha-conf.xml export PERL5LIB=/opt/koha/lib 6. ls /opt/koha/lib 7. Note the following: C4 CGI cpanfile Koha Koha.pm 8. vi /opt/koha/etc/koha-httpd.conf 9. Note the following: SetEnv PERL5LIB "/opt/koha/lib" 10. rm -rf /opt/koha 11. make clean Standard: 1. perl Makefile.PL 1a. accept all the defaults 2. make 3. ls blib/PERL_MODULE_DIR/ 4. Note the following: C4 CGI cpanfile Koha Koha.pm 5. make install 5b. Note there is a set_koha_ownership failure because of a missing "koha" user, but that's OK since this is an overly intensive process within koha-testing-docker anyway, and the install has already succeeded at this point. 6. ls /usr/share/koha/lib 7. Note the following: C4 CGI cpanfile Koha Koha.pm An alternative to my patch would be to create a "PERL_MODULE_LOCAL_DIR" target/dirmap entry/token, and we could use "local_lib" or something like that. Another idea would be to keep PERL_MODULE_DIR as a target/dirmap, but change the token used in etc/misc files to something like "__PERL5LIB_DIRS__". I think that would be more labour intensive though. That said... it looks like "backend_directory" will be broken in etc/koha-conf.xml and misc/bin/sip_run.sh will be broken with my proposed dev install atm... I really dislike this whole session mess but... I'll move my follow-up to a separate ticket, as it is blowing out a bit... Created attachment 121650 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob This column contains serialized data, it should be a blob. And it fixes encoding issues. Test plan: 1. Apply patch and run updatedatabase 2. In $KOHA_CONF make sure that you have <strict_sql_modes>0</strict_sql_modes> 3. Create 2 libraries: a. TESTÄ/Testä b. TEST✓/Test✓ 4. Go to the staff interface, change your library to Testä, then navigate to a few other pages, make sure there is no encoding problem with the library name in the top right corner 5. Do the same with Test✓ Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Signed-off-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 121651 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob (kohastructure.sql) Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Signed-off-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 121652 [details] [review] Bug 28489: Add selenium tests Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 121653 [details] [review] Bug 28489: POD - CGI::Session::Serialize::yamlxs for CGI::Session Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 121654 [details] [review] Bug 28489: Don't deal with encoding during the serialization Signed-off-by: David Cook <dcook@prosentient.com.au> I don't like the hack of putting CGI::Session::Serialize::yamlxs, but signing off anyway - with the assumption that we'll also push Bug 28519 before doing a stable release. Forget my comment 24 stuff. Just tried test 1 on a sandbox and a local master and it fails. I will open a ticket. Created attachment 121831 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob This column contains serialized data, it should be a blob. And it fixes encoding issues. Test plan: 1. Apply patch and run updatedatabase 2. In $KOHA_CONF make sure that you have <strict_sql_modes>0</strict_sql_modes> 3. Create 2 libraries: a. TESTÄ/Testä b. TEST✓/Test✓ 4. Go to the staff interface, change your library to Testä, then navigate to a few other pages, make sure there is no encoding problem with the library name in the top right corner 5. Do the same with Test✓ Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Signed-off-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 121832 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob (kohastructure.sql) Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Signed-off-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 121833 [details] [review] Bug 28489: Add selenium tests Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 121834 [details] [review] Bug 28489: POD - CGI::Session::Serialize::yamlxs for CGI::Session Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 121835 [details] [review] Bug 28489: Don't deal with encoding during the serialization Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Note: The libraries added in the selenium tests are not removed during cleanup Created attachment 121848 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob This column contains serialized data, it should be a blob. And it fixes encoding issues. Test plan: 1. Apply patch and run updatedatabase 2. In $KOHA_CONF make sure that you have <strict_sql_modes>0</strict_sql_modes> 3. Create 2 libraries: a. TESTÄ/Testä b. TEST✓/Test✓ 4. Go to the staff interface, change your library to Testä, then navigate to a few other pages, make sure there is no encoding problem with the library name in the top right corner 5. Do the same with Test✓ Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Signed-off-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 121849 [details] [review] Bug 28489: Modify sessions.a_session from longtext to longblob (kohastructure.sql) Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com> Signed-off-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 121850 [details] [review] Bug 28489: Add selenium tests Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> JD amended patch: Remove libraries the tests created Created attachment 121851 [details] [review] Bug 28489: POD - CGI::Session::Serialize::yamlxs for CGI::Session Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 121852 [details] [review] Bug 28489: Don't deal with encoding during the serialization Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> (In reply to Victor Grousset/tuxayo from comment #69) > Forget my comment 24 stuff. Just tried test 1 on a sandbox and a local > master and it fails. > I will open a ticket. bug 28560 That's still a mystery that here I was to only one to trigger it. The same mystery should happen in the above bug. Pushed to master for 21.11, thanks to everybody involved! Pushed to 21.05.x for 21.05.01 Depends on Bug 28317 not in 20.11.x |