1423855 - Refuse to run on non-Windows platforms if the profile path is not valid UTF-8

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Henri Sivonen (:hsivonen)]

Firefox is broken in various ways on non-Windows platforms is the profile path is not valid UTF-8. Let's check that it is and quit up front if it isn't instead of exhibiting mysterious brokenness.

Comment 1

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1423855 - Quit if the profile path is not valid UTF-8 on non-Windows platforms.

On non-Windows platforms Firefox is broken in various ways (unable to save
history and bookmarks, for example) if the profile path isn't valid UTF-8.
It's better to check that up front instead of being broken in mysterious ways.

Review commit: https://reviewboard.mozilla.org/r/206206/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/206206/

Comment 2

[Henri Sivonen (:hsivonen)]

Note that this assumes the new definition of NS_IsNativeUTF8() from bug 960957.

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8935307 [details]
Bug 1423855 - Quit if the profile path is not valid UTF-8 on non-Windows platforms.

https://reviewboard.mozilla.org/r/206206/#review212050

::: toolkit/xre/nsAppRunner.cpp:4494
(Diff revision 1)
> +  if (NS_IsNativeUTF8()) {
> +    // mDirProvider.DoStartup(); immediately above initialized
> +    // NS_APP_USER_PROFILE_50_DIR availability.
> +    nsCOMPtr<nsIFile> profileDir;
> +    nsAutoCString path;
> +    rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,

you should be able to use mDirProvider.GetUserAppDataDirectory here, and you could do that before mDirProvider.DoStartup and thus before appStartup, meaning you could also just do a simple return NS_ERROR_FAILURE.

::: toolkit/xre/nsAppRunner.cpp:4497
(Diff revision 1)
> +      static const char sUTF8PathMessage[] = "The profile path is not valid UTF-8. Unable to continue.\n";
> +      // Ignoring return values, since there isn't much we could do on failure and there is no
> +      // point in trying to recover from errors.
> +      MOZ_UNUSED(write(STDERR_FILENO, sUTF8PathMessage, MOZ_ARRAY_LENGTH(sUTF8PathMessage) - 1));

We're using PR_fprintf everywhere in this file, you might as well use that.

Comment 4

[Henri Sivonen (:hsivonen)]

Comment on attachment 8935307 [details]
Bug 1423855 - Quit if the profile path is not valid UTF-8 on non-Windows platforms.

https://reviewboard.mozilla.org/r/206206/#review212246

::: toolkit/xre/nsAppRunner.cpp:4494
(Diff revision 1)
> +  if (NS_IsNativeUTF8()) {
> +    // mDirProvider.DoStartup(); immediately above initialized
> +    // NS_APP_USER_PROFILE_50_DIR availability.
> +    nsCOMPtr<nsIFile> profileDir;
> +    nsAutoCString path;
> +    rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,

Looks like `GetUserAppDataDirectory()` isn't the right directory. `GetProfileStartupDir()` looks like the most correct one before `DoStartup` given that `GetProfileDir()` is explicitly wrong before startup.

It's unclear to me under what circumstances `GetProfileStartupDir()` and `GetProfileDir()` can d iffer and if they can only differ in embedding scenarios.

Comment 5

[Henri Sivonen (:hsivonen)]

Comment on attachment 8935307 [details]
Bug 1423855 - Quit if the profile path is not valid UTF-8 on non-Windows platforms.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/206206/diff/1-2/

Comment 6

[Henri Sivonen (:hsivonen)]

Comment on attachment 8935307 [details]
Bug 1423855 - Quit if the profile path is not valid UTF-8 on non-Windows platforms.

https://reviewboard.mozilla.org/r/206206/#review212050

> We're using PR_fprintf everywhere in this file, you might as well use that.

Used PR_fprintf. Now after Firefox exits, I get an error that looks like it comes from python complaining about a broken pipe, but I have no clue how python would be involved.

Comment 7

[Henri Sivonen (:hsivonen)]

Comment on attachment 8935307 [details]
Bug 1423855 - Quit if the profile path is not valid UTF-8 on non-Windows platforms.

https://reviewboard.mozilla.org/r/206206/#review212050

> you should be able to use mDirProvider.GetUserAppDataDirectory here, and you could do that before mDirProvider.DoStartup and thus before appStartup, meaning you could also just do a simple return NS_ERROR_FAILURE.

Moved and used `GetProfileStartupDir()`.

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8935307 [details]
Bug 1423855 - Quit if the profile path is not valid UTF-8 on non-Windows platforms.

https://reviewboard.mozilla.org/r/206206/#review212942

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ee8a8833d65
Quit if the profile path is not valid UTF-8 on non-Windows platforms. r=glandium

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/1ee8a8833d65