Description: On pc Debian x86-64 with master sources updated today, I got a crash when trying to save a form using an image bigger than length of Binary fix field. Steps to Reproduce: 1. Launch Base 2. Create Firebird embedded file (enable experimental features for prerequisite) or open an existing Firebird embedded file (no need to enable experimental features in this case) 3. Create a simple table TABLE1 with: - id/INTEGER primary key - f1/Binary (fix) BINARY (put 100 for length) 4. Create a form with TABLE1 with wizard: 4.1 : select the brand new table and use both fields + Next 4.2 : "Decide if you want to set up a subform" => Next 4.3 : "Arrange the controls on your form" => Select the second one from left of the first row + Finish => the form displays and you can enter data 5. type "1" in first field 6. right click on second field => "Insert Image from..." appears, select it 7. select an image of a length > 100 8. close form => dialog box asks to save 9. click "Yes" Actual Results: Crash Expected Results: No crash Reproducible: Always User Profile Reset: No Additional Info: With HSQLDB embedded, it's more difficult to reproduce this since by default length for Binary (fix) BINARY is 2147483647 and you can't change it compared with Firebird embedded database.
Created attachment 167864 [details] bt with debug symbols
Robert: would you have some minutes to reproduce this? Lionel: taking a look at the bt, the pb is here: 1847 case DataType::BINARY: 1848 case DataType::VARBINARY: 1849 case DataType::LONGVARBINARY: 1850 case DataType::BLOB: 1851 { 1852 Any x(_rValue.makeAny()); 1853 Sequence< sal_Int8> aBytes; 1854 if(x >>= aBytes) 1855 _xParams->setBytes(parameterIndex,aBytes); See https://opengrok.libreoffice.org/xref/core/connectivity/source/commontools/dbtools.cxx?r=d6d80c4e#1847 after x >>= aBytes, aBytes is just a sequence of 0 because we get over max size. So setBytes is called with a 0 size sequence and it crashes. A simple fix would be to change "if" condition to also test that aBytes is not a 0 length sequence. Pb is the user isn't warned about the pb. Another fix would be to add a specific test length and create a new string message here: connectivity/inc/strings.hrc like: "The image is too big compared for the field." (it can be greatly improved I suppose, it's just to give an idea). I'm pretty sureit's not specific to Firebird but with HSQLDB, as indicated in initial description, length of the Binary (fix) BINARY is 2147483647 and it's readonly (I didn't investigate why). Remark: I don't know how "Any" => "Sequence" conversion takes length into account to retrieve 0 size when the image is too big.
Sorry Robert, you won't be able to test this (at least with Firebird) for the moment, https://gerrit.libreoffice.org/c/core/+/107280 is a prerequisite.
(In reply to Julien Nabet from comment #2) > after x >>= aBytes, aBytes is just a sequence of 0 because we get over max > size. > So setBytes is called with a 0 size sequence and it crashes. I think that for robustness of LibreOffice, that would be useful to fix that (namely, that it doesn't crash when called with empty sequence). > Remark: I don't know how "Any" => "Sequence" conversion takes length into > account to retrieve 0 size when the image is too big. Does the "Any" actually contain something not of zero size?
Created attachment 167865 [details] gdb bt Argh, it still crashes but now it's not an empty sequence. Still, since the image size I chose on purpose is bigger than the length of the field, the crash was expected. I hate this kind of "Heisenberg" behavior but now at least, the error is understandeable. Now I suppose a check of size (0 size or size bigger than length of the field) should be done in common part and not Firebird specific.
I kept on the tests and I finally could reproduce the empty sequence too: Here are the steps: - create a table following steps for initial description but with length 10000 (instead of 100) - create a form the same way as initial description - in the form - type "1" for field 1 - choose image with a length < 10000 - close form and save - reopen the form - choose image with a length > 10000 => crash with an empty sequence. So not Heisenberg at all! :-) Now let's find the possible and most appropriate location to fix (check+warning) this between: - forms/source/runtime/formoperations.cxx - forms/source/component/DatabaseForm.cxx - dbaccess/source/core/api/RowSet.cxx - dbaccess/source/core/api/RowSetCache.cxx - dbaccess/source/core/api/KeySet.cxx - dbaccess/source/core/api/CacheSet.cxx - connectivity/source/commontools/dbtools.cxx - dbaccess/source/core/api/preparedstatement.cxx I removed connectivity/source/drivers/firebird/PreparedStatement.cxx since we want a generic fix (not Firebird specific).
Since we must deal with update + insert case, I think connectivity/source/commontools/dbtools.cxx should be used. Now the problem is how to retrieve precision (aka max length) of a column... I tested with a try catch (Exception or RuntimeException), LO doesn't care and crashes also in this case. => I'm stuck now
I think it's fair to move it to NEW
Just thinking loudly, perhaps we should check image size just after having selected it? It's in OImageControlControl::implInsertGraphics (see https://opengrok.libreoffice.org/xref/core/forms/source/component/ImageControl.cxx?r=1875b3d9&mo=23636&fi=762#762) If the image size is > field size => error message and the action isn't done.
(In reply to Julien Nabet from comment #9) > Just thinking loudly, perhaps we should check image size just after having > selected it? > It's in OImageControlControl::implInsertGraphics (see > https://opengrok.libreoffice.org/xref/core/forms/source/component/ > ImageControl.cxx?r=1875b3d9&mo=23636&fi=762#762) > If the image size is > field size => error message and the action isn't done. I would prefer this solution.
Mike Kaganski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/94ba3770ffe31bd26e0c67a5609c8935994b808a tdf#138691: avoid buffer overflow It will be available in 7.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(In reply to Robert Großkopf from comment #10) > (In reply to Julien Nabet from comment #9) > > perhaps we should check image size just after having selected it? > > It's in OImageControlControl::implInsertGraphics ... > > If the image size is > field size => error message and the action isn't done. > > I would prefer this solution. But can't forms pre-process data before inserting into tables? Then isn't checking something at entry time too soon?
(In reply to Mike Kaganski from comment #12) > (In reply to Robert Großkopf from comment #10) > > (In reply to Julien Nabet from comment #9) > > > perhaps we should check image size just after having selected it? > > > It's in OImageControlControl::implInsertGraphics ... > > > If the image size is > field size => error message and the action isn't done. > > > > I would prefer this solution. > > But can't forms pre-process data before inserting into tables? Then isn't > checking something at entry time too soon? Forms will only link from the path of the image to the table of the database. It works like this: oSimpleFileAccess = createUnoService("com.sun.star.ucb.SimpleFileAccess") oStream = oSimpleFileAccess.openFileRead(stUrl) oField.BoundField.updateBinaryStream(oStream, oStream.getLength()) There is no possibility to limit the content for this field in the form.
(In reply to Robert Großkopf from comment #13) I'm afraid I don't quite understand you. There are form events, including "Before Submitting"; do you say I can't replace whatever value I want with some processed data, e.g. with a processed (reduced) thumbnail of what I loaded into the control?
(In reply to Mike Kaganski from comment #14) > (In reply to Robert Großkopf from comment #13) > > I'm afraid I don't quite understand you. There are form events, including > "Before Submitting"; do you say I can't replace whatever value I want with > some processed data, e.g. with a processed (reduced) thumbnail of what I > loaded into the control? The form control doesn't support any limit for the content, which should be read. But: If I will control it by macro I could say: oStream.getLength() is too big for the field. So I could stop submitting. I haven't tested much with images and forms, special with images, which should be saved in a table of the database. I won't do this, because it expands the database too much. I prefer to link images by the image control to the database.
(In reply to Robert Großkopf from comment #15) Ok, so this means: 1. You shouldn't implement comment 12, since there may be a different workflow compared to what you prefer. 2. The control could benefit from a new optional property "max size". That could be possibly set automatically upon creation of the control on a fixed-size database field.