Mozilla should recognize RFC2231 enconding of filename parameter

RESOLVED WORKSFORME

Status

--
major
RESOLVED WORKSFORME
18 years ago
10 years ago

People

(Reporter: m_kato, Assigned: nhottanscp)

Tracking

({intl})

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments)

(Reporter)

Description

18 years ago
The filename parameter of Content-Disposition must uses RFC2231.
But Mozilla cannot corrently recognize filename parameter which uses RFC2231.
(Reporter)

Comment 1

18 years ago
Created attachment 21248 [details]
sample mail
(Reporter)

Comment 2

18 years ago
Created attachment 21249 [details] [diff] [review]
patch

Comment 3

18 years ago
Adding RFE and patch keyword. Marking NEW, so someone will decide if this is a
good idea or not.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: patch
Summary: Mozilla should recognize RFC2231 enconding of filename parameter → [RFE] Mozilla should recognize RFC2231 enconding of filename parameter
(Assignee)

Comment 4

18 years ago
You need to get a review by rhp for the patch.

I have a couple questions. You changed to support parameter continuation but the
patch does not have the code to unfold continuations. Has that already existed
in mozilla?
The patch does not do the decoding and instead converts from UTF-8 to specified
charset. Does that mean the decoding and the unfolding are done somewhere else
already?

(Assignee)

Comment 5

18 years ago
Remove RFE and reassign to Kato san, please get r=/sr= and check in.
Assignee: nhotta → m_kato
Keywords: intl
Summary: [RFE] Mozilla should recognize RFC2231 enconding of filename parameter → Mozilla should recognize RFC2231 enconding of filename parameter

Comment 6

18 years ago
Can you tell us if your patch covers/parses the following
items in RFC 2231?

1. Encoding name
2. Language name
3. Lightweight encoding used in RFC 2231

I don't know if we use Language name info right now
but potentially it may be used to set fonts correctly
in some cases.
(Reporter)

Comment 7

18 years ago
Momoi-san,
Langauge paramter does not watch since Mozilla uses Unicode/UTF-8 only by 
internal.  And font should not set since filename is not releted to font.
Status: NEW → ASSIGNED

Comment 8

18 years ago
Kato-san,

Language name is optional and I don't think we should be
paying attention to it. So, yes, as long as we parse it
and ignore it, that is OK with me.

By the way, Keyser raised an issue if this is a good
thing. It is a good thing to implement because this
is the only acceptable way of transmitting non-ASCII
filenames.

Comment 9

18 years ago
QA contact to ji.
QA Contact: momoi → ji
(Reporter)

Comment 10

18 years ago
I forgot the answer for hotta-san.

> I have a couple questions. You changed to support parameter continuation
> but the patch does not have the code to unfold continuations. Has that
> already existed in mozilla?

RFC2231 encoding is already implemented.  But this isn't used for filename 
parameter.  Probably, netscape engineeers don't understand abount encoding of 
filename parameter of Content-Disposition.  I told keith (the author of RFC).

> The patch does not do the decoding and instead converts from UTF-8 to
> specified charset. Does that mean the decoding and the unfolding are
> done somewhere else already?

mime_decode_filename() function decodes from RFC2047's encoding to UTF-8.
So I converted to UTF-8.

(Assignee)

Comment 11

18 years ago
Okay, please get this reviewed by rhp or ducarroz.
(Reporter)

Comment 12

18 years ago
rhp, can you review??
FYI, I am replacing rhp now.

Instead of doing the following pattern 4 time:
+          if (charset)
+          {
+            // this is seem to be a mail which supports RFC2231.
+            MIME_ConvertString(charset, "UTF-8", tmp->real_name, &fname);
+            PR_Free(charset);
+            charset = nsnull;
+          }
+          else
+            fname = mime_decode_filename(tmp->real_name);

Can you pass the charset to mime_decode_filename() and change this function do
the right job. Something like:

...
fname = mime_decode_filename(tmp->real_name, charset);
PR_FREEIF(charset);
...

Thanks

(Reporter)

Comment 14

18 years ago
Created attachment 23035 [details] [diff] [review]
new fix at 2001/01/19
(Reporter)

Comment 15

18 years ago
ducarroz, could you review?
Twice you do:
+        PR_FREEIF(charset);
+        charset = nsnull;
as the macro PR_FREEIF will set the variable to 0, you don't need to do it
yourself. Appart that, R=ducarroz

Comment 17

18 years ago
you could use an XPIDLCString for the charset, along with getter_Copies - then
you wouldn't need to put the PR_FREEIF's in at all. If you do that, or
ducarroz's suggestion, then sr=bienvenu
(Reporter)

Comment 18

18 years ago
check in
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 19

18 years ago
This fix was designed poorly.  Had I been asked, I would have refused an r=.

The conversion to UTF-8 should be done in MimeHeaders_get_parameter().  It 
should not be the responsibility of every single caller to 
MimeHeaders_get_parameter(). The "charset" parameter of 
MimeHeaders_get_parameter() should be removed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 20

18 years ago
Do you mean the following change?

char *
MimeHeaders_get_parameter (const char *header_value, const char *parm_name, 
                           char **charset, char **language)
 ->

char *
MimeHeaders_get_parameter (const char *header_value, const char *parm_name, 
                           char **language)

(Reporter)

Updated

17 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 21

17 years ago
Reassign to nhotta.

Does this still happen?
Assignee: m_kato → nhotta
Status: ASSIGNED → NEW

Comment 22

17 years ago
This problem doesn't exist on 04/08 trunk build.
Marked it wfm. Kato, please feel free to reopen it if you still see this problem.
Status: NEW → RESOLVED
Last Resolved: 18 years ago17 years ago
Resolution: --- → WORKSFORME
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.