If you create directory 'test', directory/file structure is created as follows: c:\winnt\profiles\username\application data\mozilla\users50\test\'saltedname' If 'test' is deleted, saltedname directory and all files in it are deleted but 'test' remains. These are one of the side-effects of fixing bug 56002 (adding a salted directory). It is not harmful and extra-directory sticks around. If there is a salted directory, we may have to go one level up and delete it's parent folder also..

reassigning to racham.

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

comments from bug 60799 having recently gotten back on the trunk, i've noticed that new profiles, rather than containing the files directly under the <profile_name>/ directory, now store files in a <profile_name>/<random>.slt/ subdirectory. i spoke with grace, who sez this a new feature to increase profile info security. true --but this seems rather superficial. one could just cd into in that <random>.slt directory to get at the files. my suggestion here is to get rid of this extra level of obfuscation (i could understand that it's there to confuse, but it doesn't seem terribly effective --unless there's another reason ;). instead, why not implement enhancements such as those suggested in bug 16489, bug 19184 and bug 59120? thx! adding sairuh to cc list

Doing a mass reassign to Conrad Carlen. Conrad is going address all current and upcoming profile manager issues. Please do not hesitate to involve me in any of the issues.

When deleting the profile dir, we need to check its parent dir for the .slt suffix and delete from there.

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

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

nominating.

-> 0.9.1

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

The patch fixes this in a safe manner. When deleting a profile dir, its parent is deleted only if: (1) the profile dir ends in ".slt" (2) the profile dir is the only child of its parent (in case somebody moved their salted profile dir to the root of their hard drive - whoops!) CC'ing Bhuvan & Seth for review.

Conrad, patch looks good to me. I would like to request you to try out some test cases and confirm the results are good before check in as we are talking about deleting the data/folders here and we were in deep troubles with this operation in the past. Sorry for the overload. r=bhuvan

I've tested it. Since it gets the parent dir of the dir it's about to delete anyway and checks that the parent has only one child, that should be safe. What it does now is this: (1) given the profile dir (2) gets its parent (3) ensure that the parent has only one child For an extra measure of safety, it could: (4) ensure that the one child == the profile dir That would even insure against nsIFile::GetParent() doing something wacky or (gasp!) having a bug.

CC'ing waterson for sr.

I'm not hep to the latest string jive, so tell me if this will match ``foo.slt.bar''? (In other words, does it really test that the string _ends_ with ``.slt''? (Maybe that's what end.size_forward() does?) + nsLiteralCString leafNameStr(leafName.get()); + nsReadingIterator<char> start, end; + leafNameStr.BeginReading(start); + leafNameStr.EndReading(end); + FindInReadable(NS_LITERAL_CSTRING(SALT_EXTENSION), start, end); + if ((start == end) || end.size_forward()) + return NS_OK; I'd have called the routine ``IsProfileDirSaltedAndUnique'' or or something, just to indicate that you're also checking that condition. Maybe not, take it or leave it.

It shouldn't match 'foo.slt.bar' Since we salt dir names by adding .slt, if it doesn't end in that, the user has changed it and I'd say we shouldn't delete it. As far as the routine name, how about ShouldDeleteProfileParentDir() ?

> It shouldn't match 'foo.slt.bar' I agree that it *shouldn't*, but I'm not convinced that it *doesn't*. Certainly, if given the string ``foo.slt.bar'', you'll end up with... foo.slt.bar ^ ^ | end start So |start != end|. What will |end.size_forward()| return in this case? (I guess I'm wishing we had an ``EndsWith'' string helper routine right now!) > As far as the routine name, how about ShouldDeleteProfileParentDir() ? Sounds good to me!

foo.slt.bar ^ ^ | end start So |start != end|. What will |end.size_forward()| return in this case? (I guess I'm wishing we had an ``EndsWith'' string helper routine right now!) That's the truth! If start == end, the string was not found. In this case size_forward() should return 4 and fail the test. Time to call in the string guy. Scott, is this right?

CC'ing Johnny for string iterator review. Can you read the questions Waterson had about searching in strings?

|end.size_forward()| is not a good test of anything except whether there is more data in the hunk of string pointed to by |end| ... there may be more string in a later hunk. In "ReadableUtils.h" where it is declared, |FindInReadable| has documentation. I quote: Returns |PR_TRUE| if a match was found, and adjusts |aSearchStart| and |aSearchEnd| to point to the match. If no match was found, returns |PR_FALSE| and makes |aSearchStart == aSearchEnd| So just collect the function result for your test, e.g., PRBool matchFound = FindInReadable(NS_LITERAL_CSTRING(SALT_EXTENSION), start, end); But now you also want to know if that match was found at the very end of the string? No problem, test like this: nsLiteralCString leafNameStr(leafName.get()); nsReadingIterator<char> stringEnd leafNameStr.EndReading(stringEnd); nsReadingIterator<char> matchEnd = stringEnd, matchStart; leafNameStr.BeginReading(matchStart); PRBool endWithSalt = FindInReadable(NS_LITERAL_CSTRING(SALT_EXTENSION), matchStart, matchEnd) && (matchEnd == stringEnd); Of course, since you're not even interested in finding a match anywhere else in the string, all you really care about is if the last |n| characters of the string match your pattern. If this is the case, just extract the appropriate range for comparison first, e.g., PRBool endsWithSalt = PR_FALSE; NS_NAMED_LITERAL_CSTRING(saltPattern, ".slt"); if ( leafNameString.Length() >= saltPattern.Length() ) { nsReadingIterator<char> stringEnd; leafNameString.EndReading(stringEnd); nsReadingIterator<char> stringStart = stringEnd; stringStart.advance( -saltPattern.Length() ); endsWithSalt = saltPattern.Equals(Substring(stringStart, stringEnd)); // or |==|, or |Compare|, or whatever makes you feel happiest } Also, I would caution against applying the macro |NS_LITERAL_CSTRING| to another macro as in your original example. There have been problems with this using |NS_LITERAL_STRING|, it just seems less safe. Prefer defining the macro from the start, e.g., #define SALT_EXTENSION_CSTRING NS_LITERAL_CSTRING(".slt") or whatever. Hope this helps

The new patch uses Scott's last suggestion for determining whether the name ends in ".slt" It also adds a check for whether the profile dir in question exists before all this work. It would have safely failed later but better to fail sooner.

Alright, now with Scott's string advice taken and the method name changed, can I get sr?

sr=sspitzer the code you needed to write to do "ends with" seems like a lot of work. a while back I added some code to nsPrefMigration.cpp, and I'm sure there are other "ends with" utilitity functions out there. scc, seems like this could be added the string library?

see bug 76946

Fix checked in. Sorry for the delay - I was away for a few days.

Jonathan, please test this for I18n. If you see some problems for I18n, please log the other bug.

build 2001052306