Closed Bug 514209 Opened 10 years ago Closed 4 years ago

Illegal profilname with a directory delimiter breaks CreateProfile dialog and creates an artifact profile directory

Categories

(Toolkit :: Startup and Profile System, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: benjamin.rampe, Assigned: jaso35)

Details

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.13) Gecko/2009080316 Ubuntu/8.04 (hardy) Firefox/3.0.13
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.13) Gecko/2009080316 Ubuntu/8.04 (hardy) Firefox/3.0.13

The state of the profile path isn't reset after deleting a formerly entered slash and the profile path isn't reset to its initial state when pressing on "back".

Reproducible: Always

Steps to Reproduce:
1. Open the dialog for creating a new profile
2. Remove "Default User" (or whatever it says in your language)
3. Enter a name with a "/" in it (e.g. Foo/Bar)
 a Watch the profile path below!
 b Firefox doesn't allow you to save that, but now the trouble begins!
4. Use backspace to delete any characters including all "/" you inserted
 a Watch the profile path again: The slash is still there.
5. Now try to click backwards to undo the _whole_ steps
 a Watch the profile path again: the profile path is still modified with the artifact.
Actual Results:  
A Subdirectory "profile" under the profile directory /initialSalt.<artifact>/
-- in the profile selection dialog after starting with "-no-remote" the name is the same as the actually entered, valid name.

Expected Results:  
Profile name with the username at the end (/initialSalt.Default User/)

The profile with the crippled folder name seems to work anyway.
Reporter, please retest with Firefox 3.6.12 or later in a fresh profile (http://support.mozilla.com/kb/Managing+profiles). Also update your plugins (flash, adobe reader, java, quicktime, silverlight, etc.) Go to the developer's website and download the latest version from there. If you no longer see this issue, please close this bug as RESOLVED, WORKSFORME. If you do see the bug, please post a comment.
Whiteboard: [CLOSEME 2010-12-01]
Firefox 3.6.12 in the package from Ubuntu Lucid Lynx (3.6.12+build1+nobinonly-0ubuntu0.10.04.1) is still affected.

No Plugins. Fresh profile.
Whiteboard: [CLOSEME 2010-12-01]
Version: unspecified → 3.6 Branch
17.0.1 affected...
Component: General → Startup and Profile System
Product: Firefox → Toolkit
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0
BuildID: 20160208030244

I managed to discover a similar bug in the Windows version which is highly related to this:

STR:
1. Start the Profile Manager with firefox -P and and try to create a new profile.
2. Enter a random name which contains a \
3. The profile manager warns you about that illegal character.
4. Delete the name and try another one.

What happens:

The new path shown in the bottom gets stuck as soon as the backslash appears.
Deleting the invalid input and trying to enter a valid name doesn't seem to change anything. When I try to continue with the new valid name an error message is shown:

Profile couldn't be created. Probably the chosen folder isn't writable. [Exception... "Component returned failure code: 0x80520008 (NS_ERROR_FILE_ALREADY_EXISTS) [nsIToolkitProfileService.createProfile]"  nsresult: "0x80520008 (NS_ERROR_FILE_ALREADY_EXISTS)"  location: "JS frame :: chrome://mozapps/content/profile/createProfileWizard.js :: onFinish :: line 201"  data: no]

Also there are profile folders created based on the invalid names, but i haven't tested them for functionality.

Expected behaviour:

The new name should be accepted normally and a valid profile folder should be created.


Benjamin, could you try to reproduce this bug again with the latest Firefox?
Also update version, platform and description accordingly please.

Thank you for your effort,

Jan
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(benjamin.rampe)
Picture of the issue
OS: Linux → All
Hardware: x86 → All
Summary: Illegal profilname with "/" in it creates artifact in profile path even after stepping back → Illegal profilname with a directory delimiter breaks CreateProfile dialog and creates an artifact profile directory
Version: 3.6 Branch → Trunk
iceweasel:
  Installed: 42.0-1~bpo80+1
  Candidate: 42.0-1~bpo80+1
  Version table:
 *** 42.0-1~bpo80+1 0
        500 http://mozilla.debian.net/ jessie-backports/iceweasel-release amd64 Packages

It's still the same behavior: the illegal character is recognized but the future path of the profile isn't cleaned up accordingly.
Flags: needinfo?(benjamin.rampe)
Attached patch Bug_514209.patch (obsolete) — Splinter Review
I think the main problem is here:

 toolkit/profile/content/createProfileWizard.js

> function updateProfileName(aNewName) {
>  checkCurrentInput(aNewName);
>  var re = new RegExp("^[a-z0-9]{8}\\." +
>                      gOldProfileName.replace(/[|^$()\[\]{}\\+?.*]/g, "\\$&")
>                      + '$');
>  if (re.test(gProfileRoot.leafName)) {
>    gProfileRoot.leafName = saltName(aNewName);
>    updateProfileDisplay();
>  }
>  gOldProfileName = aNewName;
> }

If a directory delimiter is written to leafName we cannot return to the original profile root. The input after the delimiter becomes the new leaf and all input before the delimiter becomes inaccessible in profileRoot.path.

I have added a patch which tries to work around writing to leafName and uses append() instead. profileRoot only keeps the path where the profile directory will be created in and the salted user input is appended to that in onFinish().
For display purposes, we should clone profileRoot temporarily and append the salted user input to that. I also have thought about using profileRoot.parent.path for this but we would need to insert an additional system specific directory limiter then, because it's omitted in the end of .path.

Dave, you seem to have committed to this code in the past. Is this a good approach and what additional work is needed to get a patch into Firefox?

I have tested this patch in Linux and it seems to work fine for me.
Additional testing would be welcome.

Thanks,
Jan
Assignee: nobody → jaso35
Flags: needinfo?(dtownsend)
We already check the profile name for invalid characters like directory separators it just doesn't propagate that all the way back to updateProfileName. We should just do that and then not update the profile path in case of error.
Flags: needinfo?(dtownsend)
Attached patch Bug_514209b.patch (obsolete) — Splinter Review
Okay, I just tried to preserve the current behaviour that the erroneous profile path is still displayed completely. If this is not needed the patch is simpler.
Attachment #8721764 - Attachment is obsolete: true
Attachment #8723074 - Flags: review?(jmathies)
Comment on attachment 8723074 [details] [diff] [review]
Bug_514209b.patch

Sorry this took so long. This fix works well but gOldProfileName is still in use in this file. Looks like you can just delete the final reference in initWizard().
Attachment #8723074 - Flags: review?(jmathies) → review+
Okay, I removed the reference to gOldProfile I missed the first time. Thanks for your review.
Attachment #8723074 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f5b01783baaa
https://hg.mozilla.org/mozilla-central/rev/dd94eb69270a
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.