On Sat, Dec 31, 2011 at 2:58 PM, Stephen J. Turnbull <stephen(a)xemacs.org&gt; wrote: We do use DGifSlurp, and that's what's telling us that the GIF file is invalid, causing us to signal an error. I'd expect us to then return 'nil', but it appears that somehow a literal '0' is escaping to lisp. I'm currently lost in a maze of unwind-protect and call_with_suspended_errors to see how this could be fixed. I don't think we need look into that just yet. Yes, it's similar, except that now we don't crash. Robert _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

On Mon, Jan 2, 2012 at 3:53 PM, Stephen J. Turnbull <stephen(a)xemacs.org&gt; wrote: They're from when you call the _instantiate routine. No. That works fine (and there's no crash when it does fail, just an error). I don't think make-glyph actually causes the gif to be read until later. Robert _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

On Sat, 31 Dec 2011 12:14:39 +0100 Robert Pluim <rpluim(a)gmail.com&gt; wrote: It looks like the gif image is good... except it is missing the terminator byte (0x3b). It is such a HUGE gif, they are probably trying to save space ;) All the readers I tried handled this gracefully, except XEmacs. Even Emacs handled it. I looked quickly at XEmacs 21.4 and DGifSlurp basically does this: do { GetRecordType switch (type) process } while (RecordType != TERMINATE_RECORD_TYPE); Since there is no terminate byte, GetRecordType tries to read the next byte and fails. I have some gif knowledge, but I am no expert so take this with a lump of salt (unless you are on a salt reduced diet). But it looks like this is done to get around not keeping track of the gif version. So instead of doing what I would consider normal: read header if gif89a read extension header read image block header read image blocks read terminator XEmacs does this: read header Slurp (as shown above) It seems that the loop is only to handle the fact that the extension header may or may not exist. One solution is to check for EOF before calling GetRecordType, since it looks like the lack of a terminator is not considered fatal in practice. Cheers, Sean _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

On Sun, 1 Jan 2012 13:25:55 +0100 Robert Pluim <rpluim(a)gmail.com&gt; wrote: Ok, here is another idea. I tested with with XEmacs 21.5.31. We change gif_read_from_memory to handle this specific case: static int gif_read_from_memory (GifFileType *gif, GifByteType *buf, int size) { gif_memory_storage *mem = (gif_memory_storage *) gif->UserData; if (size > (mem->len - mem->index)) { /* Hack to handle a gif file that is missing the terminator byte. */ if (size == 1 && mem->len > 1 && mem->bytes[mem->len - 1] != 0x3b) { *buf = 0x3b; mem->len = 0; return 1; } else return -1; } memcpy (buf, mem->bytes + mem->index, size); mem->index = mem->index + size; return size; } Since it is short, I added the entire routine. I just added the if block. Since the UserData is setup once in gif_instantiate and only used in gif_read_from_memory, it is safe to mess with the len. I did a quick check to see that it handled the case where the terminator was missing, but still caught some other random truncations as errors. Cheers, Sean _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

Robert Pluim writes: Are you guys seriously proposing that we special-case this one particular bogus file? By simply silently ignoring the error? (AFAICS the statement "mem->len = 0;" tells giflib that there's no data there, no?) We really need to fix the error stuff that Robert mentioned (which should give the same result, or perhaps the calling code will need to do a condition-case to ignore the GIF error). Presumably that could happen on a real corrupted GIF. I wonder if this comment at the top of gif_instantiate() might not have something to do with it: /* It is OK for the unwind data to be local to this function, because the unwind-protect is always executed when this stack frame is still valid. */ _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

On Mon, Jan 2, 2012 at 5:26 PM, Robert Pluim <rpluim(a)gmail.com&gt; wrote: In fact, text-modes/image-mode.el probably should be patched like this, since it matches the expressed intent. (did make-image-instance change signature?) diff --git a/image-mode.el b/image-mode.el index e9dd55a..a6959dd 100644 --- a/image-mode.el +++ b/image-mode.el ＠＠ -82,11 +82,11 ＠＠ (let ((image (and type (make-image-instance (vector type :data (buffer-string start end)) - nil nil 'no-error)))) + nil nil 't)))) (unless image (setq image (make-image-instance (vector 'string :data "format is not supported!\n") - nil nil 'no-error))) + nil nil 't))) (set-extent-property (make-extent start end) 'invisible t) (let ((glyph (make-glyph image))) (set-extent-end-glyph (make-extent end end) glyph)) It does mask a bug though, I think. Robert _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

Robert Pluim writes: OK. I'm happy to hear that. :-) Hm. I just got around to trying it, and I only get 43 lines. Maybe we should blame larsi for the other 250 lines you get? ;-) I don't think returning is the right thing to do: Errors should never pass silently. Unless explicitly silenced. The obvious application of the Zen is for the calling code to do the explicit silencing. In the case of VM, (condition-case nil (set-extent-begin-glyph (make-extent (point) (point)) (make-glyph (vector 'gif :data data))) (image-conversion-error (insert "[invalid GIF: " file "]\n"))) or similar. Unfortunately, there are times when we do want to see such errors. It would be horrible for somebody to spend a few days trying to track down why some image doesn't appear, only to find out that the image is broken (according to giflib) and got swallowed in make-image-instance. I agree that we do *not* (normally) want to see a backtrace for a problem that XEmacs thinks is external. `make-image-instance' seems to agree, it has a fourth argument NOERROR with this documentation: NOERROR controls what happens with the image cannot be generated. If nil, an error message is generated. If t, no messages are generated and this function returns nil. If anything else, a warning message is generated and this function returns nil. But callers of make-image-instance don't generally seem to have a way to control that. :-( It doesn't say what's in the error message. Maybe we can suppress the backtrace part in make-image-instance. _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

Sean MacLennan writes: Sure. Lots of standards have redundancy. Allowing nonstandard, less redundant formats makes things more fragile. While you can probably prove that there is no risk to your change if giflib is perfectly implemented, all of the image libraries have suffered from critical bugs in recent memory, and I suppose they have their share of minor bugs that only show up if the app that calls them is buggy or lax, too. I personally don't want additional fragility in my mission- critical application, and I would argue XEmacs core (which for this purpose includes xemacs-base and xemacs-devel but not the loadable modules) should push for more simplicity and strictness in base functionality that all XEmacs depends on, while providing hooks that allow Lisp or modules to provide more powerful functionality. If you could tell me that Wikipedia distributes and the GIMP produces such GIFs (of course I'm exaggerating, but you get the flavor), I might hold my nose and let it pass. But seriously, what do we lose by fixing make-image-instance to warn less obtrusively, instead of going out of our way to display something that's invisible anyway? (I wouldn't be surprised to find out that single pixel is transparent!) viewer developer I would find your arguments attractive. Nevertheless, XEmacs is neither an image viewer nor a browser. The dedicated image viewers and editors mostly provide libraries we could link to. If you want to go that direction, especially if it were modularized (and therefore optional at runtime), that would be great. Really! Heck, I'll even go find Jerry and physically lean on him until he agrees to help with the modularization! :-) But I don't see a need for repairing broken GIFs in core XEmacs, and dealing with the stupid backtrace from the error is far more important, because I think that's wrong even with genuinely busted images. _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

Jerry James writes: AFA image libraries go, according to what I've read, GraphicsMagick has a more stable API than ImageMagick. OTOH, I'm not sure we care about that since we have no backwards to compat. Besides, Emacs is using ImageMagick, so we can get some useful tips and possibly apps from them. I don't know if there are other libraries around. Actually, basic modularization isn't all that hard. I've done a bunch (both from former core code -- Canna -- and from scratch). But there are some tricky aspects I haven't mastered (I'd like some help with modules that need to load other modules and/or Lisp libraries as prerequisites, but I'll get back to you on that), and if we're going to hand GIF to Imagemagick, it *really* should be usable on Windows (where GIF is a lot more prevalent than on Linux) which I know nothing about modularizing beyond depending on ellcc. There may also be some tricky aspects to image processing since it's low-level and maybe gets called from redisplay? Actually, I think it's Gnusies who want gnutls, which is going to be pretty pervasive in Emacs 24. I suspect VM would be happy if we went with the same TLS implementation as GNU, too. (If I had my druthers, I think I'd go with openssl as the Gnubies have been through a fair amount of pain with gnutls, especially w.r.t. Windows.) _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

Didier Verna writes: Erm, that's precisely where a lot of the pain with gnutls was! It didn't work with Gmail at first. ISTR they fixed it by requiring an unreleased version of gnutls, which presumably will be released before Emacs 24 is. There were other problems, too, but I don't remember exactly what. _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

Didier Verna writes: OK, we'll take it on faith that gnutls has its advantages. It doesn't really matter; Emacs is using gnutls (for political reasons, but there's nothing we can do about that), so that's where we should start. gnutls has an OpenSSL compatibility API, and if that's not good enough for somebody's OpenSSL-based application, they can implement an OpenSSL module or configure option, with the advantage of having a (hopefully mostly working) OpenSSL API already integrated and with a Lisp API as reference for design. _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

Robert Pluim writes: I'll veto that. Does that answer the implied question? (1) AFAIK, GIF is obsolete but well-defined technology. People who distribute broken GIFs should be considered a hazard to the health of the Internet. (2) I've only ever heard of this particular bug with respect to this pheedo situation. Let's remember what this is for: to provide an invisible place to attach privacy-invading stuff to a web page. I see no need to do anything to pander to them. Show me some legitimate GIFs broken in this way that people actually *want*, and I'll reconsider. _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta

Sean MacLennan <seanm(a)seanm.ca&gt; writes: These are tracker images. Someone claimed that they were constructed that way because (some) browsers would keep the TCP socket open until they received the complete image, which they never would, of course. But this allows the tracker company to see how long time each user spends reading the page. I don't know whether this is true, though... -- (domestic pets only, the antidote for overdose, milk.) bloggy blog http://lars.ingebrigtsen.no/ _______________________________________________ XEmacs-Beta mailing list XEmacs-Beta(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-beta