GNOME Bugzilla – Bug 169051
rsvg-view hangs on this file (and right-clicking on it hangs Nautilus and X)
Last modified: 2005-03-03 12:59:56 UTC
From http://live.gnome.org/MarketingTeam_2fMarketingMaterial?action=AttachFile&do=get&target=tshirt-plain.svg (I will attach as well) rsvg-view hangs on this file. Right-clicking on it hangs Nautilus, and because Nautilus has an input grab at the time it locks the X server. Running a strace gives: ... open("Desktop/Downloads/tshirt-plain.svg", O_RDONLY) = 12 fstat64(12, {st_mode=S_IFREG|0644, st_size=16814, ...}) = 0 mmap2(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7d5c000 read(12, "<?xml version=\"1.0\" encoding=\"UT"..., 8192) = 8192 read(12, "00,138.48300 5.6860000,140.72600"..., 8192) = 8192 read(12, "-mode:lr\"\n x=\"181.88387\"\n "..., 8192) = 430 read(12, "", 8192) = 0 close(12) = 0 munmap(0xb7d5c000, 8192) = 0 open("Desktop/Downloads", O_RDONLY) = 12 fstat64(12, {st_mode=S_IFDIR|0755, st_size=8192, ...}) = 0 mmap2(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7d5c000 read(12, 0xb7d5c000, 8192) = -1 EISDIR (Is a directory) read(12, 0xb7d5c000, 8192) = -1 EISDIR (Is a directory) read(12, 0xb7d5c000, 8192) = -1 EISDIR (Is a directory) read(12, 0xb7d5c000, 8192) = -1 EISDIR (Is a directory) read(12, 0xb7d5c000, 8192) = -1 EISDIR (Is a directory) ... ad nauseam. Here's a backtrace:
+ Trace 56367
I'll recompile with debugging symbols. I cannot reproduce this on the 2.8.x series. I think this is a showstopper in view of its potential severity - users will not know to go to a console vt and killall -9 nautilus and will be left with a locked X session.
Created attachment 38179 [details] Offending svg
Better bt:
+ Trace 56368
Evidently gcc has inlined a function. I'll recompile w/o optimisations, but I think the offending code is in rsvg-image.c: rsvg_acquire_file_resource() 282 : while (!feof (f)) { 283 : length = fread (buffer, 1, sizeof (buffer), f); 284 : if (length > 0) 285 : if (g_byte_array_append (array, buffer, length) == NULL) { 286 : fclose (f); 287 : g_byte_array_free (array, TRUE); 288 : return NULL; 289 : } 290 : } We need to check for length < 0 and if so error out. Also it would probably be a good idea to check it's a file we're opening in the first place.
Yep:
+ Trace 56369
Created attachment 38180 [details] [review] Patch
Oh, sorry - the patch is against released sources; in cvs rsvg-shapes.c has been split up. It should apply OK to rsvg-image.c.
Created attachment 38181 [details] [review] Correct patch Sorry, that's completely wrong - fread() is a ferror() function. Here's the correct patch.
variation on your patch applied to HEAD.
Created attachment 38182 [details] [review] Correct (2) patch Gah. How long have I programmed in C?
Yeah, sorry. Now we're getting segfaults in functions that don't handle NULL return from rsvg_acquire_file_resource and wrapper _rsvg_acquire_xlink_href_resource. Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1208420688 (LWP 9077)] 0xb7fbac5e in rsvg_defs_load_extern (defs=0x804e1b0, name=0x807f590 "") at rsvg-defs.c:81 Again, this is released tarball, not CVS. 63 : static int 64 : rsvg_defs_load_extern(const RsvgDefs *defs, const char *name) 65 : { 66 : RsvgHandle * handle; 67 : cmoore 1.14 gchar * filename, *base_uri; 68 : GByteArray * chars; 69 : 70 : 71 : filename = rsvg_get_file_path (name, defs->base_uri); 72 : 73 : chars = _rsvg_acquire_xlink_href_resource(name, defs->base_uri, NULL); 74 : cmoore 1.13 75 : cmoore 1.14 handle = rsvg_handle_new (); 76 : cmoore 1.13 77 : cmoore 1.14 base_uri = rsvg_get_base_uri_from_filename(filename); 78 : rsvg_handle_set_base_uri (handle, base_uri); 79 : g_free(base_uri); 80 : cmoore 1.13 81 : cmoore 1.14 rsvg_handle_write (handle, chars->data, chars->len, NULL);
Created attachment 38183 [details] [review] Follow-up patch Hopefully this'll work first time. Against release tarball (but it should apply); coding style, etc.
Well, that allows another bug to become visible. Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1208420688 (LWP 29702)] 0xb7fbad56 in rsvg_defs_extern_lookup (defs=0x804e1b0, filename=0x807f590 "", name=0x807f4c8 "Unnamed copy#2") at rsvg-defs.c:105 static RsvgDefVal * rsvg_defs_extern_lookup (const RsvgDefs *defs, const char *filename, const char *name) { RsvgHandle * file; file = (RsvgHandle *)g_hash_table_lookup (defs->externs, filename); if (file == NULL) { if (rsvg_defs_load_extern(defs, filename)) return NULL; file = (RsvgHandle *)g_hash_table_lookup (defs->externs, filename); } return (RsvgDefVal *)g_hash_table_lookup (file->defs->hash, name); } 105 is the return. Guess we need if (file == NULL) return NULL;
Created attachment 38186 [details] [review] Next patch Tested it this time; this allows rsvg-view to display the file OK and fixes nautilus. Note that when I tried the file in inkscape: $ inkscape Desktop/Downloads/tshirt-plain.svg ** (inkscape:26703): WARNING **: Malformed URI Some extra sanity checking, evidently.
re comment 8: shouldn't these patches be applied to gnome-2-10 as well? I really don't want to hit this bug again when librsvg-2.10.0 comes out; svgs that trigger this bug can't be that hard to come by.
btw, the offending fragment in the svg is this: <use x="0" y="0" xlink:href="#Unnamed copy#2" id="use1101" /> a child of the <svg> node.
dear sweet jesus, please stop replying to this bug so often. my inbox is exploding :) i'll apply the patches to the gnome 2.10 branch as well. only so many hours in the day. plus, this isn't a bug many are likely to encounter given today's SVG art and themes. it isn't as big a deal as you're making it out to be.
variants on the latest 2 above patches applied to HEAD. i caught your error on the "Correct (2) patch".
applied to the gnome-2-10 branch. thanks for the patches and the bug report.