Incorrect Image URL can cause Loop

[Paul Gammans]

If you have open an inkscape/svg file with a image URL pointing to a folder it cause inkscape to loop trying to process the image.

This can also be triggered by editing the image url from both the image properties dialogue. As you delete the URL path if you match a folder name inkscape enter an endless loop.

This is repeatable. To do this using the GUI.
0 Close or save any unsaved work in Inkscape...
1 Open any Inkscape Document
2 Drag / Drop image into document
3 Right Click image and select 'Image properties'
4 Click in URL field of the new popup window
5 Delete leaf name of URL till path is folder ....

Version : 0.47
Distribution: Debian squeeze

[su_v (suv-lp)]

Reproduced with Inkscape 0.48.1 and 0.48+devel r10188 on OS X 10.5.8 (i386)

The hang occurs when trying to backspace/delete the last remaining letter of the file name.
Attached backtraces have been created by interrupting the hanging Inkscape process while running from gdb

[jazzynico (jazzynico)]

Not reproduced on Windows XP, Inkscape 0.48.1 and trunk revision 10194.

[su_v (suv-lp)]

Information provided in new duplicate bug #1071034:

<quote>
When editing the properties of an image (linked to, not embedded) and changing the image path such that it refers to a directory (such as easily happens when you strip off the old filename in order to insert a new one), pixbuf_new_from_file() will go into an endless loop. See

http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/view/head:/src/sp-image.cpp#L470

There, feof(fp) will always be false and youme.readmore() will also always be false and not advance the fp position.
</quote>

[Kris (kris-degussem)]

Could not reproduce the bug on windows either, although the patch could fix the issue on other systems.
The patch drops some comments and unused code and checks at the beginning of pixbuf_new_from_file whether the the given filename is a directory.

[su_v (suv-lp)]

> (…) although the patch could fix the issue on other systems.

AFAICT the patch works ok with current trunk on OS X 10.7.4 (GTK+/X11 2.24.13, glib 2.32.4; GTK+/Quartz 2.24.13, glib 2.34.2).
Anything special (possible regressions) to watch out for?

[Kris (kris-degussem)]

Re: comment 6:
> Anything special (possible regressions) to watch out for?
No. The patch essentially adds code to check whether the URL given is a valid path to a file. It checks whether the file exists (and whether the path is not the entry of a directory).

[jazzynico (jazzynico)]

Patch tested successfully on Ubuntu 11.10, and committed revision 11885.
Thanks, Kris!

[su_v (suv-lp)]

@Kris, JazzyNico - would this be save for backporting to the stable release branch (0.48.x)?

[su_v (suv-lp)]

Diff for lp:inkscape/0.48.x

[Kris (kris-degussem)]

Seems safe. But are we still going to release 0.48.4, given the release plan for 0.49?

[su_v (suv-lp)]

> But are we still going to release 0.48.4, given the release plan for 0.49?

Bug #944077 IMHO is reason enough to keep 0.48.4 as target (also because AFAIK 0.48.4 more likely to get backported for various linux distros which might not provide packages for 0.49). And there's one (still open) CVE bug which by itself might warrant another bug fix release of the current stable series (once we have a fix for it).

[jazzynico (jazzynico)]

The diff for 0.48.x is exactly the same, and thus it should be ok.

[jazzynico (jazzynico)]

Backported to the branch, revision 9920.