tats
Fix potential null dereference with gopher
guessContentType will return NULL if it can determine a ContentType. This might happen e.g. if you insert a partial URL where the file extension is missing.
Segfaulting is the worst of all options, so fall back to the default ContentType of "image/png" in that case. Still leads to bad UX as the process will continue without an obvious error besides not image being displayed.
Ah, nice catch.
Still leads to bad UX as the process will continue without an obvious error besides not image being displayed.
So the original code just set image/gopher for I in all cases. I
guess I changed it because it's not a real MIME type. image/gopher
does have advantages though; you could set a special mailcap entry that
autodetects the image based on magic numbers, or runs a script where the
user can interactively select the image type.
OTOH I'm unsure whether anybody would spend this much effort configuring w3m to fix this incredibly niche issue. "It's probably PNG if it has no extension" may be a good enough guess for pretty much all purposes...
I wonder what Lynx does.
guessContentType will return NULL if it can determine a ContentType
Pretty sure you meant "can't" here instead of "can".
On Fri, Apr 12, 2024 at 04:05:11PM -0700, NRK wrote:
guessContentType will return NULL if it can determine a ContentType
Pretty sure you meant "can't" here instead of "can".
Of course. I updated the commit message. Thanks
On Fri, Apr 12, 2024 at 02:48:54PM -0700, bptato wrote:
Ah, nice catch.
I pasted only part of a URL to an image and got a crash. ;)
Still leads to bad UX as the process will continue without an obvious error besides not image being displayed.
[...]
OTOH I'm unsure whether anybody would spend this much effort configuring w3m to fix this incredibly niche issue. "It's probably PNG if it has no extension" may be a good enough guess for pretty much all purposes...
In my case, it had no extension because I accidentally copied a line break in the URL. Currently w3m just crashes, with this patch it at least stays alive, but the user get's no error message. That's why I put the comment there.
I wonder what Lynx does.
Good idea, will check this when I have time to come back to this issue.
I consider the current patch more as a hot fix. I just don't like losing my session.
I pasted only part of a URL to an image and got a crash. ;)
Apologies :P
Currently w3m just crashes, with this patch it at least stays alive, but the user get's no error message.
Oh, now I see what you mean. e.g. gopher://gopher.floodgap.com/I/asdf will open the error page in an image viewer.
Gopher does not have protocol-level error messages, so this seems hard to handle. Three approaches come to mind:
-
Run parseImageHeader on the file, loadGopherDir on failure. This only supports a few formats (JPEG, GIF and PNG), so probably a bad idea.
-
Check for a "3" menu item in the response, run loadGopherDir if found. I don't think there is any guarantee it will be sent, but at least the item type is defined in the standard, so it may work.
-
Check for the return code after running mailcap entries. I suppose we'd have to print an error message to the status line from SIGCHLD. This would help with other cases too when the external command fails.
Lynx
Just checked; it fails silently as well :/
On Sat, Apr 13, 2024 at 07:58:30AM -0700, bptato wrote: [...]
Currently w3m just crashes, with this patch it at least stays alive, but the user get's no error message.
Oh, now I see what you mean. e.g. gopher://gopher.floodgap.com/I/asdf will open the error page in an image viewer.
Gopher does not have protocol-level error messages, so this seems hard to handle. Three approaches come to mind: [...] 3. Check for the return code after running mailcap entries. I suppose we'd have to print an error message to the status line from SIGCHLD. This would help with other cases too when the external command fails.
I have a commit that does that already. It's part of PR #278 Replace deprecated MD5 function and other unrelated changes.
Commit 874bef6f Handle errors in mySystem
It's been a while and I have so many patches, I forgot about it.
I will create a branch in my repo where I integrate all open PRs and other stuff I have laying around.
Lynx
Just checked; it fails silently as well :/
Well, with my other patch applied: Take that, lynx! ;-)