382508 - Breakpad does not work on WinXP with non-ASCII username: "Couldn't move crash data"

Status: VERIFIED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Antti Tervasmäki]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070530 Minefield/3.0a5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070530 Minefield/3.0a5pre

After crash, all that is seen is error dialog.

Reproducible: Always

Steps to Reproduce:
1.Crash Minefield (I used printing)
2.
3.
Actual Results:  
Error dialog is shown.

Expected Results:  
Crash report send.

I can see that minidumps and .extra -files are generated on path
C:\Documents and Settings\Antti Tervasmäki\Aplication Data\Mozilla\Firefox\profiles\ruopg9e4.default\minidumps

On path C:\Documents and Settings\Antti Tervasmäki\Aplication Data\Mozilla\Firefox\Crash Reports\pending dirs are empty.

Comment 1

[Antti Tervasmäki]

Attachment: Error dialog on WinXP

Comment 2

[Antti Tervasmäki]

After writing those paths I did test as administrator and Breakpad did work...
So it is path with my name that is the problem...?

Comment 3

[(not currently active) Ted Mielczarek]

I'm guessing it's the non-ascii characters in your username.  We should be smarter about those.

Comment 4

[Antti Tervasmäki]

Yeah, there IS life outside US-english world...:-)
 

Comment 5

[Jesse Ruderman]

The "%s Crash Reporter" dialog title is cute, too.

Comment 6

[Dave Camp (:dcamp)]

Attachment: handle unicode arguments

handles unicode arguments in wWinMain and when opening files.

Comment 7

[Carsten Book [:Tomcat]]

requesting blocking 1.9 since it would prevent users with non-ASCII usernames to send crash data

Comment 8

[Benjamin Smedberg]

This needs to block a6

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 269256 [details] [diff] [review]
handle unicode arguments

>diff -r e94d2ef07977 toolkit/airbag/client/crashreporter.cpp
>+  ifstream* f = UIOpenRead(path);
>+  bool success = false;
>+  if (f->is_open()) {
>+    success = ReadStrings(*f, strings, unescape);
>+  }
>+
>+  f->close();
>+  delete f;
>+  return success;
> }

Nit: move the f->close() inside the if block.

>diff -r e94d2ef07977 toolkit/airbag/client/crashreporter.h
>+std::ifstream* UIOpenRead(const std::string& filename);
>+std::ofstream* UIOpenWrite(const std::string& filename);

Could you make these take a std::(i|o)stream& as an inout parameter instead?  That'd avoid having to delete the return value.  I know there's some trickiness in the Windows impl, so maybe it's not possible.

Comment 10

[Dave Camp (:dcamp)]

checked in

Comment 11

[Antti Tervasmäki]

So this now works?
I finally got a way to crash now very stable Minefield:
 
Breakpad really started, and send something (firewall noticed) but did not leave me a report????
There is nothing in those dirs (like minidumps, Crash reports/submitted, etc)
No error messages.

If I am logged in as administrator, I get a report in "reports/submitted" but with my own name, nothing??

Comment 12

[(not currently active) Ted Mielczarek]

We fixed this specific issue, that doesn't mean that we've covered all issues that would prevent this from working for you.  If you crash, and the Crash Reporter appears, before clicking any buttons on it, can you view the contents of Crash Reports/pending and see if the minidumps get moved there?  If so, then we know that something is working.

Comment 13

[(not currently active) Ted Mielczarek]

campd says he missed a spot.

Comment 14

[Antti Tervasmäki]

Yes, I can see minidumps are moved to pending and I guess they are send, 
only that is missing after that is ID/link in /submitted.

Comment 15

[Dave Camp (:dcamp)]

Attachment: use UIOpenWrite in AddSubmittedReport()

Comment 16

[Dave Camp (:dcamp)]

checked in, thanks for the report

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 270091 [details] [diff] [review]
use UIOpenWrite in AddSubmittedReport()

+  ofstream* file = UIOpenWrite(path);

why not use std::auto_ptr, to avoid having to call delete yourself?

Comment 18

[Antti Tervasmäki]

OK, now Breakpad works for me as expected, thanks for fast fix!