62000 - Bug file attachments broken under Windows

Status: RESOLVED FIXED

Description

[Matthew Mastracci]

The act of attaching a file to a bug under Windows is broken in Bugzilla.  
Adding the following two lines to showattachment.cgi will fix this.  Should 
there be an "is_windows" parameter in the conf file that could wrap all of the 
hacks?

 use diagnostics;
 use strict;

 require "CGI.pl";

+binmode(STDIN);
+binmode(STDOUT);

 ConnectToDatabase();

Comment 1

[Thomas Leufkes]

For creating Attachments under Windows the following two lines

binmode(STDIN);
binmode(STDOUT);

has to be added into the CREATEATTACHMENT.CGI
BEFORE the "require 'CGI.PL;'"- line.

If the two lines are missing, the system seems to hang in
the subroutine ProcessMultipartFormFields (in CGI.pl) and
ends with a "CGI Timeout".

The "is_windows" param is a fine idea !

Comment 2

[Matthew Tuck [:CodeMachine]]

Does this break Unix?  Move to 2.16 for consideration.

Comment 3

[Dave Miller [:justdave] ([e-mail])]

If it does break unix, we can make it conditional if it's windows...

if ($^O =~ /^(windows|winnt|win2k)$/) {
  binmode(STDIN);
  binmode(STDOUT);
}

We'll need to make sure what $^O really returns for various flavors of windows 
though.  I'm just guessing above.

Comment 4

[Thomas Leufkes]

On my BZ installation (W2K Server) a print $^O; displays "MSWin32".
A global param like "is_windows" that is set once,
may help to manage the source code differences.

Comment 5

[Jacob Steenhagen]

I get "MSWin32" on various Win2k and NT 4 running both IIS and Apache

Comment 6

[Dave Miller [:justdave] ([e-mail])]

*** Bug 88040 has been marked as a duplicate of this bug. ***

Comment 7

[Brian Rossmajer]

The perldoc for binmode claims it's always safe, that the logic is handled by 
the underlying libraries:

"In other words: Regardless of platform, use binmode() on binary files, and do 
not use binmode() on text files."

Comment 8

[Zach Lipton [:zach]]

moving

Comment 9

[Zach Lipton [:zach]]

moving

Comment 10

[Matthew Tuck [:CodeMachine]]

*** Bug 96846 has been marked as a duplicate of this bug. ***

Comment 11

[Dave Miller [:justdave] ([e-mail])]

correcting version field lost in product move

Comment 12

[olivier kurzweg]

createattachment.cgi seems to be still broken after having added the binmode 
lines.
After the attachment is sent to bugzilla, the createattachment page is re-
displayed with a blank form and a MIME section is printed in clear text 
(including the GIF attachment) above the form.

Comment 13

[Dave Miller [:justdave] ([e-mail])]

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

Comment 14

[jerry asher]

I am seeing this under W2K for both Mozilla 0.9.7 and IE 5.5.

Would someone clarify, is this a Windows OS problem or a specific browser 
problem?  (I don't understand why it would be an OS problem....).

The other day this occurred under Mozilla on W2K, and I switched to IE and 
uploading an attachment worked.  Today it doesn't appear to work under either 
Mozilla or IE.

Comment 15

[James Benninger]

Adding those 2 lines to createattachment.cgi was not enough to fix the problem 
on my WinNT bugzilla server. I had to add the same to lines in the same place 
to showattachment.cgi. This fixed it for me.

To repeat for clarity:
  I added the following 2 lines:
      binmode(STDIN);
      binmode(STDOUT);
  before the call to:
      require "CGI.pl"; 
  in both createattachment.cgi and showattachment.cgi and now everything works.

Comment 16

[Jouni Heikniemi]

Created attachment 83032 [details] [diff] [review]
Patch

Adds Win32-only binmode calls for attachment.cgi and removes the installation
note telling the user to do this manually.

While there is some speculation over always using binmode (even with text
files), nobody has reported any complications and many people have successfully
used binmode with all types of files on Win32, so I think it's sane until
otherwise proven. 

As for adding a more generic variable for OS detection, I think that needs a
bit more thought - so this patch only fixes the immediate Win32 problem.

Comment 17

[Jouni Heikniemi]

Taking & adding some keywords

Comment 18

[Bradley Baetz (:bbaetz)]

Comment on attachment 83032 [details] [diff] [review]
Patch

r=bbaetz x2

CGI.pm does this always, for $OS=~/^(WINDOWS|DOS|OS2|MSWin)/;

$OS is set to windows if $O =~ /^MSWin/i

Anyway, this is fine for now, and we'll remove it when we start to use CGI.pm

Comment 19

[Jouni Heikniemi]

barnboy: Is the documentation update part of the above patch outdated? Should
only the code changes be checked in?

Comment 20

[Jouni Heikniemi]

I checked the code in separately; the documentation issue is now bug 155743. I
did this because it's not clear whether the sgml files are still the master
copies, so barnboy can sort it out when 2.18 docs are composed.

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.15; previous revision: 1.14
done

Comment 21

[sorin]

*** Bug 170390 has been marked as a duplicate of this bug. ***

Comment 22

[Dave Miller [:justdave] ([e-mail])]

*** Bug 183716 has been marked as a duplicate of this bug. ***