Closed
Bug 514209
Opened 15 years ago
Closed 8 years ago
Illegal profilname with a directory delimiter breaks CreateProfile dialog and creates an artifact profile directory
Categories
(Toolkit :: Startup and Profile System, defect)
Toolkit
Startup and Profile System
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: benjamin.rampe, Assigned: jaso35)
Details
Attachments
(2 files, 2 obsolete files)
39.73 KB,
image/png
|
Details | |
2.56 KB,
patch
|
Details | Diff | Splinter Review |
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.
Comment 1•14 years ago
|
||
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]
Reporter | ||
Comment 2•14 years ago
|
||
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.
Updated•14 years ago
|
Whiteboard: [CLOSEME 2010-12-01]
Version: unspecified → 3.6 Branch
Comment 3•11 years ago
|
||
17.0.1 affected...
Component: General → Startup and Profile System
Product: Firefox → Toolkit
Assignee | ||
Comment 4•8 years ago
|
||
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)
Assignee | ||
Comment 5•8 years ago
|
||
Picture of the issue
Assignee | ||
Updated•8 years ago
|
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
Reporter | ||
Comment 6•8 years ago
|
||
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)
Assignee | ||
Comment 7•8 years ago
|
||
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)
Comment 8•8 years ago
|
||
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)
Assignee | ||
Comment 9•8 years ago
|
||
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
Assignee | ||
Updated•8 years ago
|
Attachment #8723074 -
Flags: review?(jmathies)
Comment 10•8 years ago
|
||
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+
Assignee | ||
Comment 11•8 years ago
|
||
Okay, I removed the reference to gOldProfile I missed the first time. Thanks for your review.
Attachment #8723074 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 12•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5b01783baaa
Keywords: checkin-needed
Comment 14•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f5b01783baaa https://hg.mozilla.org/mozilla-central/rev/dd94eb69270a
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•