7973 - [I18N] ENCTYPE=multipart form submission does not work w/ non ASCII [form sub]

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P3)

Description

[Frank Tang]

We need to do the unicode to charset conversion as we did for url-encoded
ENCTYPE.

Comment 1

[Frank Tang]

Mark it M7 as today.

Comment 2

[Frank Tang]

I have the fix. However, since I cannot find a single html page which
successfully perform HTTP file upload (multipart submission). I cannot verfiy my
fix. karnaze, please provide test cases.

Comment 3

[Frank Tang]

somehow I just cannot create attachment... Here is the diff
Index: nsFormFrame.cpp
===================================================================
RCS file: /m/pub/mozilla/layout/html/forms/src/nsFormFrame.cpp,v
retrieving revision 3.41
diff -c -r3.41 nsFormFrame.cpp
*** nsFormFrame.cpp     1999/05/26 22:49:06     3.41
--- nsFormFrame.cpp     1999/06/15 04:20:39
***************
*** 74,79 ****
--- 74,80 ----
  #include "prenv.h"

  #define SPECIFY_CHARSET_IN_CONTENT_TYPE
+ #define FIX_NON_ASCII_MULTIPART

  #if defined(ClientWallet) || defined(SingleSignon) ||
defined(CookieManagement)
  #include "nsIServiceManager.h"
***************
*** 1009,1014 ****
--- 1010,1018 ----

  void nsFormFrame::ProcessAsMultipart(nsString& aData, nsIFormControlFrame*
aFrame)
  {
+ #ifdef FIX_NON_ASCII_MULTIPART
+   nsIUnicodeEncoder *encoder=nsnull;
+ #endif
    aData.SetLength(0);
    char buffer[BUFSIZE];
    PRInt32 numChildren = mFormControls.Count();
***************
*** 1035,1040 ****
--- 1039,1049 ----
      return;
    }

+ #ifdef FIX_NON_ASCII_MULTIPART
+   if(NS_FAILED( GetEncoder(&encoder) ) )
+      encoder = nsnull;
+ #endif
+
    PRInt32 boundaryLen = PL_strlen(boundary);
        PRInt32 contDispLen = PL_strlen(CONTENT_DISP);
    PRInt32 crlfLen     = PL_strlen(CRLF);
***************
*** 1062,1069 ****
          contentLen += boundaryLen + crlfLen;
          contentLen += contDispLen;
                    for (int valueX = 0; valueX < numValues; valueX++) {
!           char* name  = names[valueX].ToNewCString();
!           char* value = values[valueX].ToNewCString();
            if ((0 == names[valueX].Length()) || (0 == values[valueX].Length()))
{
              continue;
            }
--- 1071,1092 ----
          contentLen += boundaryLen + crlfLen;
          contentLen += contDispLen;
                    for (int valueX = 0; valueX < numValues; valueX++) {
!
!           char* name = nsnull;
!           char* value = nsnull;
!
! #ifdef FIX_NON_ASCII_MULTIPART
!           if(encoder) {
!               name  = UnicodeToNewBytes(names[valueX].GetUnicode(),
names[valueX].Length(), encoder);
!               value  = UnicodeToNewBytes(values[valueX].GetUnicode(),
values[valueX].Length(), encoder);
!           }
! #endif
!
!           if(nsnull == name)
!               name  = names[valueX].ToNewCString();
!           if(nsnull == value)
!               value = values[valueX].ToNewCString();
!
            if ((0 == names[valueX].Length()) || (0 == values[valueX].Length()))
{
              continue;
            }
***************
*** 1131,1138 ****
            continue;
          }
                    for (int valueX = 0; valueX < numValues; valueX++) {
!           char* name  = names[valueX].ToNewCString();
!           char* value = values[valueX].ToNewCString();
            if ((0 == names[valueX].Length()) || (0 == values[valueX].Length()))
{
              continue;
            }
--- 1154,1174 ----
            continue;
          }
                    for (int valueX = 0; valueX < numValues; valueX++) {
!           char* name = nsnull;
!           char* value = nsnull;
!
! #ifdef FIX_NON_ASCII_MULTIPART
!           if(encoder) {
!               name  = UnicodeToNewBytes(names[valueX].GetUnicode(),
names[valueX].Length(), encoder);
!               value  = UnicodeToNewBytes(values[valueX].GetUnicode(),
values[valueX].Length(), encoder);
!           }
! #endif
!
!           if(nsnull == name)
!               name  = names[valueX].ToNewCString();
!           if(nsnull == value)
!               value = values[valueX].ToNewCString();
!
            if ((0 == names[valueX].Length()) || (0 == values[valueX].Length()))
{
              continue;
            }
***************
*** 1184,1191 ****
--- 1220,1231 ----

    PR_Close(tmpFile);

+ #ifdef FIX_NON_ASCII_MULTIPART
+   NS_IF_RELEASE(encoder);
+ #endif
        //StrAllocCopy(url_struct->post_data, tmpfilename);
        //url_struct->post_data_is_file = TRUE;
+
  }

  // THE FOLLOWING WAS TAKEN FROM CMD/WINFE/FEGUI AND MODIFIED TO JUST

Comment 4

[Frank Tang]

also http://babel/tests/browser/html/test/fup.html

Comment 5

[Frank Tang]

down grad to M8 since Karnaze said this part have never been verify yet. I will
check in my change this afternoon and reassign this to pollmann since ASCII part
does not work yet.

Comment 6

[Frank Tang]

I have check in the conversion code. Now it is pending on bug 8209. Reassign
this to pollmann.
pollmann: please leave this bug open (instead mark it dup of 8209) so IQA can
verify non ASCII cases. Please mark it fixed when 8209 got fix. Thanks.

Comment 7

[Eric Pollmann]

Didn't make M8

Comment 8

[David Hallowell]

Attachment: Testcase to upload files (basically a cut down version of the page at URL linked to above)

Comment 9

[Eric Pollmann]

Frank, I fixed 8209.  I took your conversion code out of the #ifdef's.  It is
working with ASCII cases.  Can you verify if this is working for non-ASCII cases
now too?  Thanks.

Comment 10

[Teruko Kobayashi]

I tested this in 11-02-08 Win32, Linux, Mac build.
I used test case in http://babel/formupload.html
When I try to upload the file which is Japanese file name, the file uploaded
does not contain anything.  When I try to upload the English file name which
contains Japanese data, the file was uploaded correctly in Win32 build only.
In Mac and Linux build, the file uploaded contains anything.
I reopen this bug.

Comment 11

[Frank Tang]

sounds like a codeset conversoin issue w/ file name. You can find the charest
used by the os for file name by calling nsIPlatformCharset service. Then you can
convert PRUnichar* to it by calling the nsIUnicodeEncoder

Comment 12

[Eric Pollmann]

After careful consideration, I've decided that I probably won't get this bug in
for M12.  Currently I have nearly 50 bugs scheduled for M13, so there is a
possibility that this bug may need to be moved out farther still.

Comment 13

[Eric Pollmann]

I don't have any multibyte charset files / file names to test with, would it be
possible to attach one to this bug report (or email it to me?)

I'd also like help understanding the issue.  In the land of form submission, I
get the file name from the nsFileControlFrame in the form of an nsString.  What
do I need to do to this nsString to prepare it for submission to the server?

Currently it is processed using code that I think you (ftang) checked in to help
with this process: nsFormFrame::UnicodeToNewBytes().  Both the name (file input
name) and value (file name) are processed through this.  Will this convert as
you describe?  If not what more should I do?  Thanks!

Comment 14

[ekrock's old account (dead)]

Asked a friend in international Netcenter for test files with Japanese filenames
and Japanese contents. Moving to M15. Removing [TESTCASE] code since we need
additional test cases on this and perhaps someone will help.

Comment 15

[Eric Pollmann]

Attachment: Zip of 2 Japanese files (one name in Japanese) from Emi, thanks!

Comment 16

[Eric Pollmann]

Moving to M16

Comment 17

[Eric Pollmann]

Rescheduling

Comment 18

[Dave]

http://www.rent.net/ctg/cgi-bin/RentNet/AptRentalStates/AAAAfNAAXAAAK2tAAA/?brand=

If you go to the above URL and choose a state, the buttons labeled "search
checked cities", "search region" and "get area map" have forms that use
ENCTYPE="application/x-www-form-urlencoded" which crashes mozilla. I think that
this is the same problem that's being worked on in this bug but I can't be sure
because the URL for this bug is broken and my browser keeps trying to save the
testcase attachment as "showattachment.cgi" :(
Can someone confirm whether these bugs are the same or not?

Comment 19

[Eric Pollmann]

Dave, what build are you using and what operating system are you running on?  I 
do not see a crash on the URL you mentioned above on either Windows NT or Linux 
builds from yesterday, so whatever was causing the crash is likely to have been 
fixed.

At any rate, this bug is for ENCTYPE="multipart/form-data" forms not uploading 
files correctly when they have non-ascii characters in the filename or he file 
itself (Teruko Kobayashi 1999-11-02).  Speaking of which, this bug has not been 
tested in 6 months, so I'll give it a quick run through.

Dave, if you do see a crash when submitting a form, please file a new bug - I 
don't know of any crash bugs in form submission right now.

Comment 20

[Dave]

Opened bug 43643 for rent.net crash. I also did more testing (on
2000062220/Linux) and notice that the crash only occurs when no input is
provided (I.e. none of the boxes are checked)... even the "get map" button
appears to work just as long as one or more of the boxes are checked.

Comment 21

[Eric Pollmann]

This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another 
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration.

Comment 22

[James Salsman]

I am concerned that Eric is assigned to the glacially slow W3C 
XForms working group when he could be doing something else to 
actually improve the utility of Mozilla.

Seriously:  W3C XForms has not even figured out what to do with 
password fields:
  http://lists.w3.org/Archives/Public/www-forms/2000Nov/0004.html

That is not the only case of complete idiocy by the W3C XForms 
group; email me for details.

There is no way that in five years XForms can possibly have more 
than a small dent out of HTML 4 forms; I predict no more than 10%, 
maximum.

Please spare Eric from trying to figure out what Sebasitan and 
Dave R. are trying to pronounce.  He could be so much more.

Comment 23

[Kevin McCluskey (gone)]

Bulk reassigning form bugs to Alex

Comment 24

[Boris Zbarsky [:bzbarsky]]

Is this still an issue?  The current nsFormSubmission.cpp code looks like this
should work....

Comment 25

[Jungshik Shin]

Attachment: testing file upload with a non-ASCII name

sorry for being too lazy to write an html form to test this, but the file is
very short...

Comment 26

[Boris Zbarsky [:bzbarsky]]

And you're hoping bugzilla doesn't screw it up, right?  ;)

Comment 27

[Jungshik Shin]

It worked perfectly. Bugzilla didn't screw it up :-)