fix: Parse SDK internal database file parse.db is accessible for app user on iOS and may be accidentally deleted

[Nidal-Bakir]

New Pull Request Checklist

• I am not disclosing a vulnerability.
• I am creating this PR in reference to an issue.

Issue Description

Closes: #791

Approach

Using LibraryDirectory as a path for the database.

TODOs before merging

This is a breaking change for ios platform because the SDK now will look for the "parse.db" in the LibraryDirectory rather than the ApplicationDocumentsDirectory.

We can support the old directory path by doing something like:

  Future<String> getDatabaseDirectory() async {
    if (Platform.isIOS) {
      if(await _isDatabaseFileExistsInOldPath()){
        return (await path_provider.getApplicationDocumentsDirectory()).path;
      }
      return (await path_provider.getLibraryDirectory()).path;
    } else {
      return (await path_provider.getApplicationDocumentsDirectory()).path;
    }
  }

  Future<bool> _isDatabaseFileExistsInOldPath() async {
    final oldIosDatabaseDirPath =
        (await path_provider.getApplicationDocumentsDirectory()).path;
    final oldDatabaseFile =
        path.join('$oldIosDatabaseDirPath/parse', 'parse.db');

    if (await File(oldDatabaseFile).exists()) {
      return true;
    }
    return false;
  }

• Add tests
• Add changes to documentation (guides, repository pages, in-code descriptions)
• A changelog entry

[parse-github-assistant]

Thanks for opening this pull request!

• 🎉 We are excited about your hands-on contribution!

[codecov]

Codecov Report

Patch coverage: 70.37% and project coverage change: +0.56 🎉

Comparison is base (9ff9794) 15.82% compared to head (1964272) 16.38%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #826      +/-   ##
==========================================
+ Coverage   15.82%   16.38%   +0.56%     
==========================================
  Files          47       47              
  Lines        2876     2899      +23     
==========================================
+ Hits          455      475      +20     
- Misses       2421     2424       +3     

Impacted Files	Coverage Δ
packages/dart/lib/src/objects/parse_object.dart	14.95% <0.00%> (-0.47%)	⬇️
...utter/lib/src/storage/core_store_directory_io.dart	100.00% <100.00%> (+100.00%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Nidal-Bakir]

@mtrezza @mbfakourii Friendly ping about this PR

[mbfakourii]

@mtrezza @mbfakourii Friendly ping about this PR

Unfortunately, I don't have a Mac system to check, but the code has no logic problems and looks good

[mtrezza]

See #791 (comment)

[Nidal-Bakir]

@mtrezza
Before we merge this, I have some concerns about the dbDirectoryfunction

Parse-SDK-Flutter/packages/flutter/lib/parse_server_sdk.dart

Lines 138 to 142 in a19ea87

	Future<String> dbDirectory() async {
	String dbDirectory = '';
	dbDirectory = await CoreStoreDirectory().getDatabaseDirectory();
	return path.join('$dbDirectory/parse', 'parse.db');
	}

$dbDirectory/parse
the / here will cause a bug on Windows OS right ? or I'm missing something....
because the resulting path on windows OS will be like this: dbDirectory/parse\parse.db
the first / should be \ right?

so we need to use the join function for all the parameters like:

Future<String> dbDirectory() async {
  final dbDirectory = await CoreStoreDirectory().getDatabaseDirectory();
  return path.join(
    dbDirectory,
    'parse',
    'parse.db',
  );
}

What do you think?

Should I fix this in this PR or open a new issue with a new PR for it?

[mtrezza]

• Is CoreStoreDirectory().getDatabaseDirectory(); OS agnostic?
• You are correct, path.join is a native method that composes the path in the correct syntax depending on the OS, so you don't have to worry about this. Manually composing paths is usually not a good idea for the reason you pointed out. path.join docs say:

  These manipulate path strings based on your current working directory and the path style (POSIX, Windows, or URLs) of the host platform.

Should I fix this in this PR or open a new issue with a new PR for it?

A new issue would be great. I think we also need to look at this bug separately and I think it merits a separate changelog entry and release. It may even be a breaking change for Windows systems if the fix creates a different path for the file. Not sure what happens with the current bug on Windows.

[Nidal-Bakir]

@mtrezza

Is CoreStoreDirectory().getDatabaseDirectory(); OS agnostic?

In our case yes it is because we use ApplicationDocumentsDirectory for any OS other than iOS which uses LibraryDirectory

A new issue would be great. I think we also need to look at this bug separately and I think it merits a separate changelog entry and release

Got it you are right

It may even be a breaking change for Windows systems if the fix creates a different path for the file

I do not think so! an exception will be thrown I think. I will try it tomorrow and see what will happen.

also, this PR is ready to be merged. I will bump the Flutter SDK version to 3.1.4 and add a changeLog entry

[mtrezza]

I've modified the PR title, could you please review?

[mtrezza]

Minor cleanup

[Nidal-Bakir]

@mtrezza
Thanks for the edits.

LGTM

[Nidal-Bakir]

@mtrezza

@mtrezza Before we merge this, I have some concerns about the dbDirectoryfunction

Parse-SDK-Flutter/packages/flutter/lib/parse_server_sdk.dart

Lines 138 to 142 in a19ea87

	Future<String> dbDirectory() async {
	String dbDirectory = '';
	dbDirectory = await CoreStoreDirectory().getDatabaseDirectory();
	return path.join('$dbDirectory/parse', 'parse.db');
	}

$dbDirectory/parse the / here will cause a bug on Windows OS right ? or I'm missing something.... because the resulting path on windows OS will be like this: dbDirectory/parse\parse.db the first / should be \ right?

so we need to use the join function for all the parameters like:

Future<String> dbDirectory() async {
  final dbDirectory = await CoreStoreDirectory().getDatabaseDirectory();
  return path.join(
    dbDirectory,
    'parse',
    'parse.db',
  );
}

What do you think?

Should I fix this in this PR or open a new issue with a new PR for it?

---

I test the path on Windows 10

It looks like Windows 10 will understand the path and will work normally and the file was created without any issues. But this is not a normal path for Windows OS. I do not know what the consequences of doing that.

As you can see this is not normal at all: c:\nidal\IdeaProjects\dart_application_1/parse\parse.db

If it's working on Windows 10 then I am positive that it will work on Windows 11. But I am not sure what will happen on Windows 8 and Windows 7

[mtrezza]

Thanks for the lint fix.

But this is not a normal path for Windows OS.

I think what's going on here is that while historically in MS-DOS it was not a directory operator, at some point (probably many years back) the path interpreter has been updated to accept forward slashes as well. Probably in the context of URI handling which uses forward slashes. Hence the path you mention is correctly interpreted. I tried this in MS-DOS 6.22 and it's interpreted as a command switch as expected. If you try it in a Windows command prompt and type cd a/b/c or cd a\b/c then it may work. Another theory is that there is something before the OS on Flutter's side that is correcting the path as an intermediary.

I think we are good to merge since you tested it and it worked. I agree with your earlier comment that it will either work or fail because of an invalid path.

[mtrezza]

Looks good!

[mtrezza]

The CI passed merging, but publishing failed, see #835.
Could someone please take a look?