Issue 125359 - PDF Export crashes for Source Han Sans / Noto CJK fonts
Summary: PDF Export crashes for Source Han Sans / Noto CJK fonts
Status: RESOLVED FIXED
Alias: None
Product: General
Classification: Code
Component: code (show other issues)
Version: 3.3.0 or older (OOo)
Hardware: All All
: P3 Major with 2 votes (vote)
Target Milestone: 4.1.8
Assignee: AOO issues mailing list
QA Contact:
URL:
Keywords: crash
: 128392 (view as issue list)
Depends on:
Blocks:
 
Reported: 2014-08-05 18:18 UTC by Audrey Tang
Modified: 2020-07-01 17:24 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: 4.2.0-dev
Developer Difficulty: Easy


Attachments
Patch to cff.cxx (383 bytes, patch)
2014-08-05 18:18 UTC, Audrey Tang
no flags Details | Diff
GDB backtrace (10.53 KB, text/plain)
2014-08-05 18:58 UTC, Ariel Constenla-Haile
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description Audrey Tang 2014-08-05 18:18:46 UTC
Created attachment 83778 [details]
Patch to cff.cxx

Steps to reproduce:

1. Download and install Source Han Sans:
http://sourceforge.net/adobe/source-han-sans/wiki/Home/

Or: Noto T Chinese / Noto S Chinese / Noto Japanese from:

http://www.google.com/get/noto/

They are the same font.

2. New Writer or Impress file, type in any text, and apply the above font from font dropdown list.

3. Export to PDF.
--> Crashes.

The attached patch fixes this issue as explained by @kenlunde, main coordinator of Source Han Sans CJK at https://github.com/adobe-fonts/source-han-sans/issues/27#issuecomment-51055950 :

Source Han Sans (and thus Noto Sans CJK) include 19 FDArray elements. The maximum number of FDArray elements is 256. For testing fodder, please grab one or most fonts that are provided in the following CJK Type Blog article that @kenlunde published over two years ago: http://blogs.adobe.com/CCJKType/2012/05/all-unicode-cfr.html

The same patch has been vetted and applied as a hot-fix in LibreOffice at https://bugs.freedesktop.org/show_bug.cgi?id=81516#c14 .

As for copyright status of the patch, here is my standard disclaimer:

To the extent possible under law, I waive all copyright and related or neighboring rights to my past & future contributions to OpenOffice.

http://creativecommons.org/publicdomain/zero/1.0

Cheers,
Audrey
Comment 1 Ariel Constenla-Haile 2014-08-05 18:58:26 UTC
Created attachment 83779 [details]
GDB backtrace
Comment 2 Ariel Constenla-Haile 2014-08-05 18:59:20 UTC
Already reproducible with 3.3.
Comment 3 Arrigo Marchiori 2020-06-30 12:08:40 UTC
I just kind-of duplicated this bug as #128392.
Please merge this patch into 4-devel branches!
Comment 4 Matthias Seidel 2020-06-30 13:15:44 UTC
*** Issue 128392 has been marked as a duplicate of this issue. ***
Comment 5 Matthias Seidel 2020-06-30 13:16:48 UTC
Patch was committed to trunk in 2016:
https://github.com/apache/openoffice/commit/30928f5456b84e4f631e6ab589da3fe977b55cd5
Comment 6 Arrigo Marchiori 2020-06-30 13:18:27 UTC
Thank you Matthias for your quick reply to bug #128392 !

I checked out branch AOO418 and the patch does not seem to have been integrated there.

Apparently, it is also missing from branch AOO420.

Am I looking in the wrong places? I just want to point out that this patch must absolutely (IMHO of course) be integrated into next release.
Comment 7 Arrigo Marchiori 2020-06-30 13:27:09 UTC
I apologize, I was wrong about branch AOO42X. The patch is there.
Sorry for the noise.

So, please, have it included into AOO418, if a release 4.1.8 is going to be published.
Comment 8 Matthias Seidel 2020-06-30 13:28:48 UTC
Yes, it is in AOO42X
https://github.com/apache/openoffice/commits/AOO42X/main/vcl/source/fontsubset/cff.cxx

I plan to cherry-pick it for AOO418 now.
Comment 9 Matthias Seidel 2020-06-30 15:30:56 UTC
Cherry-picked to AOO418 now:
https://github.com/apache/openoffice/commit/971776bc74f50cdd618873d0419126920f937128

Thank you for the reminder!

It would be great if you could test if that fixes your problem.
Comment 10 Arrigo Marchiori 2020-07-01 11:49:02 UTC
Thank you very much Matthias, it works!

I understand that this patch consists of changing a somewhat-small fixed array into a somewhat-big fixed array. To me, this looks like a possible waste of memory, mostly due the fact that many people probably never bumped into this problem.

Do you think it would be worth the effort to change that fixed array into a variable-sized one? 

I would be available to make a patch, but please tell me:

 1- if it is worth this extra effort,

 2- how I should implement it, to be consistent with OpenOffice's coding standards:
   - std::vector?
   - new[] and delete[]?
   - malloc(), realloc() and free()?
   - ...?
Comment 11 Peter 2020-07-01 12:14:20 UTC
Hi  Arrigo Marchiori

Please see the Wiki entry here:
https://cwiki.apache.org/confluence/display/OOOUSERS/Development+Improvement

We want to remove all c-arrays over time. Currently no volunteer looking into this.

A patch is welcome. However we prefer a pull request on github.

We are still bound by C++ std GNU98, due to set supported Operating Sysytems. But it is a good thing to go for modern Programming style as much as possible.

please use std::vector. If you can avoid new and delete, please do so.
Comment 12 Arrigo Marchiori 2020-07-01 14:49:49 UTC
Just opened PR #89: https://github.com/apache/openoffice/pull/89

I hope it helps.
Comment 13 Matthias Seidel 2020-07-01 17:24:00 UTC
(In reply to Arrigo Marchiori from comment #12)
> Just opened PR #89: https://github.com/apache/openoffice/pull/89
> 
> I hope it helps.

Thanks!

Someone more knowledgeable than me will have a look.

According to comment 10, I will close this issue as resolved.

Looking forward for a more elegant solution.