Select a font in a PDF file#1
Select a font in a PDF file#1coolwanglu wants to merge 2 commits intofontforge:masterfrom coolwanglu:pull
Conversation
|
Can you elaporate on how to test this? I can get indices to work, something like |
|
Font name should be used, like "a.pdf(BFRJLG+Calibri)", where the font name is obtained using pdffonts. Can you send me a link of the pdf causing crash? |
|
It turned out the crash was not related to your patch, selecting the same font in the GUI chooser caused a crash too, so applied. Selecting fonts by index works for TTC file, so if you can get this to work for PDF files would be even better to keep things consistent. |
|
Yes, I've thought about selection by index. |
|
PostScript font names can't be a number (or even start with a number), but I'm not sure about TrueType fonts, so I think it is OK to leave it as it is for now. |
I don't know why it is crshing now (may be the resources?) as this code have
been like that since the dawn of time.
Traceback:
0xb78289b0 in check_image_buffers (gdisp=0x84d5c78, neww=<optimized out>, newh=7, is_bitmap=0) at gimagexdraw.c:1705
1705 if ( width > gdisp->gg.iwidth || depth!=gdisp->gg.img->depth ) {
(gdb) bt
#0 0xb78289b0 in check_image_buffers (gdisp=0x84d5c78, neww=<optimized out>, newh=7, is_bitmap=0) at gimagexdraw.c:1705
#1 0xb782b595 in gximage_to_ximage (image=0x8527ff0, src=0xbfffa0a0, gw=<optimized out>) at gimagexdraw.c:1778
#2 0xb782e8c2 in _GXDraw_Image (_w=0x9bf4bb0, image=<optimized out>, src=0xbfffa0a0, x=8250, y=7) at gimagexdraw.c:1981
#3 0xb7807a98 in GDrawDrawScaledImage (w=0x9bf4bb0, img=0x8527ff0, x=8250, y=7) at gdraw.c:488
#4 0xb781b513 in GListMarkDraw (pixmap=0x9bf4bb0, x=8250, y=1, height=19, state=gs_enabled) at ggadgets.c:482
#5 0xb788c493 in GMatrixEdit_SubExpose (gme=0x9bd9448, pixmap=0x9bf4bb0, event=0xbfffa400) at gmatrixedit.c:1691
#6 0xb788c6b1 in matrixeditsub_e_h (gw=0x9bf4bb0, event=0xbfffa400) at gmatrixedit.c:1723
#7 0xb78040da in _GWidget_Container_eh (gw=0x9bd9178, event=0xbfffa400) at gcontainer.c:269
#8 0xb7878001 in dispatchEvent (gdisp=0x84d5c78, event=0xbfffa5ec) at gxdraw.c:3959
#9 0xb7878162 in GXDrawProcessOneEvent (gdisp=0x84d5c78) at gxdraw.c:3991
#10 0xb7808606 in GDrawProcessOneEvent (gdisp=0x84d5c78) at gdraw.c:748
#11 0x080c77ff in ContextChainEdit (sf=0x9aa3c68, fpst=0x8ad982c, gfi=0x9afbfc0, newname=0x0, layer=1) at contextchain.c:3207
#12 0x0818f14a in _LookupSubtableContents (sf=0x9aa3c68, sub=0x8a9d43c, sd=0x0, def_layer=1) at lookupui.c:5531
#13 0x0813138d in LookupSubtableContents (gfi=0x9afbfc0, isgpos=0) at fontinfo.c:5941
#14 0x08136a16 in LookupMouse (gfi=0x9afbfc0, isgpos=0, event=0xbfffe930) at fontinfo.c:7336
#15 0x08136ccc in lookups_e_h (gw=0x9b98230, event=0xbfffe930, isgpos=0) at fontinfo.c:7380
#16 0x08136d78 in gsublookups_e_h (gw=0x9b98230, event=0xbfffe930) at fontinfo.c:7399
#17 0xb788f3b0 in drawable_e_h (pixmap=0x9b98230, event=0xbfffe930) at gdrawable.c:219
#18 0xb78048a7 in _GWidget_Container_eh (gw=0x9b98230, event=0xbfffe930) at gcontainer.c:393
#19 0xb7878001 in dispatchEvent (gdisp=0x84d5c78, event=0xbfffeb1c) at gxdraw.c:3959
#20 0xb7878480 in GXDrawEventLoop (gd=0x84d5c78) at gxdraw.c:4058
#21 0xb78086c1 in GDrawEventLoop (gdisp=0x84d5c78) at gdraw.c:766
#22 0x0822457c in main (argc=1, argv=0xbffff134) at startui.c:1501
Coverity found that values read from external data sources were not checked for 'goodness'. More specifically the upper/lower bounds were not being checked. Since a somewhat similar scenario passing through this area crashed FontForge with bogus values, it seems reasonable to satisfy Coverity and protect ourselves from this (unlikely) danger. The starting object number and count of objects is read into variables 'start' and 'num' and used to populate a dynamically allocated array. Protect against trouble by requiring that start and num are positive integers less than a (very high) upper bound. Also that the sum of those is less than that same bound. (bound derived as highest observed object number rounded up to power of 10 and again multipled by 10) The inserted code protects against values from both fscanf() calls at top and bottom of routine FindObjects(). The Coverity reports were: > CID 1083582: Untrusted value as argument (TAINTED_SCALAR) [select issue] > 6. overflow: Multiply operation overflows on operands start + num + 1L and 8UL. Example value for operand: start + num + 1L = 4611686018427387904. > CID 1083582 (fontforge#1 of 7): Untrusted loop bound (TAINTED_SCALAR) > 8. tainted_data: Using tainted variable start + num as a loop boundary. Tested against PDF files with the 'xref' table, including the tutorial file doc/html/fontforge-tutorial.pdf
A mix of minor fixes against Coverity report items. All of these are for code in fontforge/parsepdf.c === Routine pdf_loadfont() line ~ 1933 CID 1226268 (fontforge#1 of 1): Dereference null return value (NULL_RETURNS) Coverity reported that a returned pointer could be NULL, but that error return value wasn't being handled by calling code in pdf_loadfont(). Unlikely to happen (tmpfile() failing?) but I checked that pdf_loadfont() callers could handle error return of NULL, and saw example use just above area, so added guard against return of NULL from _ReadPSFont(). Coverity report was > CID 1226268 (fontforge#1 of 1): Dereference null return value (NULL_RETURNS) > 15. dereference: Dereferencing a pointer that might be null fd when calling SplineFontFromPSFont. [show details] > 1956 fd = _ReadPSFont(file); Tested against PDF containing type 1 font ("/FontFile") both with and without forcing the returned value to NULL. === Routine pdf_getinteger() line ~ 636 Coverity complained that the return value from ftell() was being used with fseek() without first checking for the error return value -1. Added an "if(here<0) return(0)" emulating the several other error returns in the routine pdf_getinteger(). Coverity report was > CID 1083667 (fontforge#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS) > 11. negative_returns: here is passed to a parameter that cannot be negative. > 648 fseek(pc->pdf,here,SEEK_SET); Could not test definitively as no available PDF had data that passed through this code path. === Routine pdf_getcmap() line ~ 1749 Coverity complained that the string variable 'prevtok' was being used before being initialized, which was very true. Later in the code variable 'tok' would be copied into it, but no initial value was set. The surrounding code made mistakes unlikely but... Coverity report was > CID 1225176 (fontforge#2 of 2): Uninitialized scalar variable (UNINIT) > 8. uninit_use_in_call: Using uninitialized element of array prevtok when calling sscanf. Tested with PDF having CMap and verified 'prevtok' was uninitialized, and initialized after code change. > char tok[200], *ccval, prevtok[200]; > char tok[200], *ccval, prevtok[200]=""; Coverity complained that dynamic calloc() into 'mappings' was not being released, which was true. Coverity report was > CID 1083111 (fontforge#4-1 of 5): Resource leak (RESOURCE_LEAK) > 50. leaked_storage: Variable mappings going out of scope leaks the storage it points to. Tested with PDF having CMap. === Routine pdf_findfonts() line ~ 556 Coverity complained about an allocation leak. Code needed to make a copy of a transient value 'pt' (the font name) as a following call would erase the value. But then the copy was only used inside a conditional, and if false then no one released the memory. Coverity report was > CID 1083101 (fontforge#2 of 3): Resource leak (RESOURCE_LEAK) > 73. leaked_storage: Variable tpt going out of scope leaks the storage it points to. Tested the true path (and found I'd coded it wrong and fixed to this version). Haven't found a PDF passing through the false path. === Routine add_mapping() line ~ 1687 Coverity complained about an allocation leak. Code works very hard to create a name of many parts, but then only uses that within a conditional. The memory was not released if the conditional was false. Coverity report was > CID 1083105 (fontforge#1-2 of 3): Resource leak (RESOURCE_LEAK) > 13. leaked_storage: Variable name going out of scope leaks the storage it points to. Tested the true path, but not able to find a PDF that tests the false path. === Routine pdf_readdict() line ~ 385 Coverity complained about a memory leak. A copy of a string was being made to save in a data structure, but that assignment was inside a conditional. The memory would be lost if the conditional went the other way. Coverity report was > CID 1083585 (fontforge#1 of 2): Resource leak (RESOURCE_LEAK) > 16. leaked_storage: Variable value going out of scope leaks the storage it points to. Tested with various PDFs, as all would pass through this routine.
==10627==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00010e2239c1 at pc 0x000111258c3d bp 0x7ffee286c210 sp 0x7ffee286b988
WRITE of size 4 at 0x00010e2239c1 thread T0
#0 0x111258c3c in scanf_common(void*, int, bool, char const*, __va_list_tag*) (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x27c3c)
#1 0x111258d6d in wrap_vsscanf (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x27d6d)
#2 0x11125902c in wrap_sscanf (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x2802c)
fontforge#3 0x10de70b21 in PrefsUI_LoadPrefs prefs.c:1230
fontforge#4 0x10e02e0ce in fontforge_main startui.c:1109
fontforge#5 0x10d654b11 in main main.c:33
fontforge#6 0x7fff62d7b3d4 in start (libdyld.dylib:x86_64+0x163d4)
0x00010e2239c1 is located 63 bytes to the left of global variable 'fvhintingneededcol' defined in '../fontforgeexe/fontview.c:123:14' (0x10e223a00) of size 4
0x00010e2239c1 is located 0 bytes to the right of global variable 'warn_script_unsaved' defined in '../fontforgeexe/fontview.c:83:6' (0x10e2239c0) of size 1
SUMMARY: AddressSanitizer: global-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x27c3c) in scanf_common(void*, int, bool, char const*, __va_list_tag*)
warn_script_unsaved is declared as bool, but prefs.c:1230 casts its
pointer to int *, leading the issue above. Prefs of type pr_bool should
be int as well, FontForge is pre-C99 and does not know bool.
Bookmarks in File Chooser UI
Selecting font by name using the FILENAME(FONTNAME) scheme has been implemented in SFReadPdfFont but not _SFReadPdfFont
However the latter is the one used by script, python and UI, which is weird.
I just move the font selection part from SF... to _SF...