Opened 13 years ago

Closed 13 years ago

#675 closed bug (fixed)

Dragging (zip) file onto StyleEdit window hangs it, 100%cpu useage

Reported by: kutspam@… Owned by: jackburton
Priority: normal Milestone: R1
Component: Kits/Interface Kit Version:
Keywords: Cc: diver, Niels.Reedijk@…, boolaiku@…
Blocked By: Blocking:
Has a Patch: no Platform: All

Description (last modified by axeld)

Drag a zip file onto StyledEdit window and it will hang and 100% cpu time is being used.

Attachments (2)

StyledEditFileChecker.diff (2.6 KB) - added by boolaiku@… 13 years ago.
checks the mime type before loading a file
StyledEditFileChecker.2.diff (2.6 KB) - added by boolaiku@… 13 years ago.
check the type of the file

Download all attachments as: .zip

Change History (19)

comment:1 Changed 13 years ago by diver

Cc: diver added

comment:2 Changed 13 years ago by jackburton

Also opening ppm files hangs StyledEdit.

comment:3 Changed 13 years ago by boolaiku@…

attachments.isobsolete: 01

Changed 13 years ago by boolaiku@…

Attachment: StyledEditFileChecker.diff added

checks the mime type before loading a file

Changed 13 years ago by boolaiku@…

check the type of the file

comment:4 Changed 13 years ago by axeld

Owner: changed from sikosis to axeld

comment:5 Changed 13 years ago by axeld

Status: newassigned

comment:6 Changed 13 years ago by boolaiku@…

Cc: boolaiku@… added

comment:7 Changed 13 years ago by Niels.Reedijk@…

Hi Alexandre,

Since no one has responded to your patch, I've taken the liberty of looking at it. And I have some suggestions/questions.

  1. You define "char fileTypeStr[256];". The maximum length of the MIME-type is

defined in StorageDefs.h as "#define B_MIME_TYPE_LENGTH (B_ATTR_NAME_LENGTH - 15)", so I think you should use that one instead of 256.

  1. The title of your alerts "StyledEdit Load Failed" sounds rather spartan and

harsh. I haven't checked the other alert titles, so please check if this is a convention that's common (I'd opt for 'Load Failed', or better, 'Failed loading bla.png").

  1. What happens to files that don't have a MIME-type yet (like files just

checked out from SVN)? I'm not sure on standard procedure, but does the OS set the mimetype automatically? Please test this by dragging over an unmimetyped file. If your load fails, you might want to try to work 'update_mime_info' from Mime.h into your patch (perhaps extra if your mimetype is invalid - but I don't know if empty mime types are invalid).

Anyway, the code looks clean and good. Please have a look at the remaining issues, and then I'll check in the patch for you.

comment:8 Changed 13 years ago by Niels.Reedijk@…

Cc: Niels.Reedijk@… added

comment:9 Changed 13 years ago by axeld

I didn't accidently assigned this bug to me (it's not new, it's assigned). I've also spoken to Alexandre via IRC already; please don't check in that code, as it's not the correct solution to the problem.

comment:10 Changed 13 years ago by boolaiku@…

(In reply to comment #4) Thanks for looking at my proposal and for your remarks. I just made a new patch following suggestions 1 and 3. Not 2 because anyway the title is not shown on BAlert's. Anyway, i didn't post it. I'll rather wait for new directions from Axel since it

seems i touched a problem that goes deeper than i thought 8)

(In reply to comment #5) In fact it seems we're not yet ready for a fully typed os. What if the file has no mime type?, like on a fat32 partition. I'm sure we'll have solution for this

post R1. I'm supposing Axel prefers going more oldschool and do some other kind

of checking for StyledEdit. Please enlighten me :)

ps:What the best place for this kind of talk? mailing list? seems better

thanks both, going to bed...

comment:11 Changed 13 years ago by Niels.Reedijk@…

(In reply to comment #5)

I've also spoken to Alexandre via IRC already; please don't check in that code, as it's not the correct solution to the problem.

I didn't see any comments and I assumed this was a bug with a patch at which no one looked. That would be a waste. If you've got things under control, I'll take my hands of it.

OT:

I didn't accidently assigned this bug to me (it's not new, it's assigned).

Yeah, I noticed that. I also noticed that nowhere in the comments there was a reference to when you actually ASSIGNED it to you. My experience with other Bugzillas are that you were to eager in thinking you could fix it and assigned it too soon.

So please document that convention somewhere, as there are other projects that use different ones.

comment:12 Changed 13 years ago by axeld

When a back has been assigned and accepted (ie. it's status is not new anymore), a developer has indeed accepted to look at it. If that's the case, you could of course still ask to take it over, but you at least can assume that the developer in question looked into it. In fact, I wanted to have solved this issue already, but I got distracted, and didn't find the time to complete that yet. So thanks for the reminder, anyway :-)

comment:13 Changed 13 years ago by diver

Also if you go to /boot/beos/etc, select all and press enter (repeat again if needed) StyledEdit will eat 100% cpu.

comment:14 Changed 13 years ago by axeld

Component: ApplicationsUser Interface/InterfaceKit
Description: modified (diff)
Owner: changed from axeld to jackburton
Platform: All
Status: assignednew

I don't know if the last comment has anything to do with this bug, but the future will tell :-) I've investigated the issue, and prepared a fix in STXTTranslator such that it will only accept actual text documents. However, the actual problem lies in BTextView; it is called with a buffer full of invalid characters, and falls into an infinite loop as the result. Stefano, if that helps you, the BTextView::Insert() method is called from BTranslationUtils::GetStyledText() line 423.

comment:15 in reply to:  14 Changed 13 years ago by jackburton

Status: newassigned

Replying to axeld:

I don't know if the last comment has anything to do with this bug, but the future will tell :-) I've investigated the issue, and prepared a fix in STXTTranslator such that it will only accept actual text documents. However, the actual problem lies in BTextView; it is called with a buffer full of invalid characters, and falls into an infinite loop as the result. Stefano, if that helps you, the BTextView::Insert() method is called from BTranslationUtils::GetStyledText() line 423.

Ok, thank you very much, I'll have a look, shouldn't be hard to find out and fix.

comment:16 Changed 13 years ago by jackburton

Last commit of mine (18967) seems to have fixed that problem. Can you confirm ?

comment:17 Changed 13 years ago by axeld

Resolution: fixed
Status: assignedclosed

That's indeed fixed now. However, it's very different from how R5's BTextView handles invalid characters. I'm not sure which one I'd prefer, but if R5's can handle NULL bytes correctly, we probably should be able to do that as well some day. But maybe it's not that important.

Note: See TracTickets for help on using tickets.