clean up nsMsgAttachmentHandler.cpp, fix GenerateFileNameFromURI() to use NS_MsgHashIfNecessary()

VERIFIED FIXED in mozilla0.9.6

Status

MailNews Core
Composition
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: Jean-Francois Ducarroz)

Tracking

Trunk
mozilla0.9.6

Firefox Tracking Flags

(Not tracked)

Details

here's the code as it stands now:

  tempName = GenerateFileNameFromURI(mURL); // Make it a sane name
#ifdef XP_WIN
  if (tempName && PL_strchr(tempName, '|'))
  {
    PR_Free(tempName);
    tempName = nsnull;
  }
#endif
#ifdef XP_MAC
  if (tempName && PL_strlen(tempName) >= 31)
    PR_DELETE(tempName)
#endif

basically, you are hacking it so if GenerateFileNameFromURI() doesn't generate 
a sane name, you throw it away and use "nsmail-X.tmp".

my issue with the code is it isn't XP (what about OS/2?)  about about other 
characters?

instead of tacking on changes using #ifdef XP_foo, you should fix 
GenerateFileNameFromURI() to really generate a "sane name"

I'd fix GenerateFileNameFromURI() to use NS_MsgHashIfNecessary(), which is how 
we've been turning a given string into something safe for the underlying OS.

it will take care of long file names and file names with illegal chars.

Comment 1

17 years ago
Let me reassign to ducarroz, this is a generic mail compose code.
Assignee: nhotta → ducarroz

Comment 2

17 years ago
reassign based on bug summary
QA Contact: sheelar → esther
OS: other → All
QA Contact: esther → stephend
Hardware: Other → All
(Assignee)

Comment 3

17 years ago
this problem should goes away once bug 86089 get fixed.
Status: NEW → ASSIGNED
Depends on: 86089
Target Milestone: --- → mozilla0.9.7
(Assignee)

Updated

17 years ago
Whiteboard: Have fix
(Assignee)

Comment 4

17 years ago
Fixed and checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Whiteboard: Have fix
Target Milestone: mozilla0.9.7 → mozilla0.9.6
Fix was checked in this morning, verified that it made it (had to repull it,
myself).

I think this is LXR-verifiable, if not, please do re-open.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.