dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.1k stars 1.56k forks source link

VM's lookupResolvedPackageUris returns mismatched results when a filename contains a comma #52632

Open DanTup opened 1 year ago

DanTup commented 1 year ago

Found in https://github.com/Dart-Code/Dart-Code/issues/4572. If a filename contains a comma, the results of lookupResolvedPackageUris appear to treat the URI as if it is two (split on the comma) and returns two nulls in the results. This means all of the subsequent results are shifted off-by-one (for each comma), and the length of the results do not match the length of the input.

lookupResolvedPackageUris request/response

@bkonyi FYI. I don't know how this could happen if the JSON is deserialized as JSON, but maybe you have some ideas what might be happening here?

DanTup commented 1 year ago

Possibly the issue is in ParseJsonArray here, which might be assuming commas are separators and not taking into account quoted strings?

https://github.com/dart-lang/sdk/blob/463c7cb5a2fdedf1022e3e71db9b79aa5bc8849e/runtime/vm/service.cc#L3605-L3618

mkustermann commented 1 year ago

/cc @bkonyi @derekxu16 @rmacnak-google

bkonyi commented 1 year ago

So there's a couple of issues here. As @DanTup pointed out, ParseJSONArray isn't equipped to handle , or ] characters if they're part of a string in the list. This isn't difficult to fix assuming the VM service is sending JSON arrays of strings with actual string formatted elements (e.g., ["foo", "bar"]).

Unfortunately, the VM service uses toString() to convert each element of JSON maps, and List<String>.toString() doesn't actually wrap the individual elements with quotes. This means that, <String>['foo,dart', 'bar.dart'].toString() gives '[foo,dart, bar.dart]', which would be passed directly to the service's C++ code.

DanTup commented 9 months ago

I think this issue is the cause of https://github.com/flutter/flutter/issues/137163 which is the top Flutter crash on stable/3.16.4.

I'm going to improve the error (we shouldn't try to complete a completer twice) and see if I can work around it, but with a workaround it's likely going to leave something not working correctly, because we won't have the mapping for some files.

knowbee commented 5 months ago

CC: @DanTup

flutter doctor

[✓] Flutter (Channel stable, 3.19.6, on macOS 14.4.1 23E224 darwin-arm64, locale en-RW)
    • Flutter version 3.19.6 on channel stable at /Users/knowbee/fvm/versions/3.19.6
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 54e66469a9 (14 hours ago), 2024-04-17 13:08:03 -0700
    • Engine revision c4cd48e186
    • Dart version 3.3.4
    • DevTools version 2.31.1
Screenshot 2024-04-18 at 12 26 53
DanTup commented 5 months ago

@knowbee do any your filenames happen to contain a comma? If so, then removing the comma will resolve the issue until this issue is resolved.

knowbee commented 5 months ago

@DanTup There are no commas in any of my files. What's intriguing is that the app runs OK using the Flutter CLI but fails to run when using the Vscode debugger tool thus throwing the above error.

DanTup commented 5 months ago

@knowbee

What's intriguing is that the app runs OK using the Flutter CLI but fails to run when using the Vscode debugger tool thus throwing the above error.

That makes sense, because the CLI doesn't use this API. It's used by VS Code to map package: URIs back to file paths on your disk, so when the debugger stops in package:foo/foo.dart we can tell VS Code which file to open on disk.

There are no commas in any of my files.

In that case, would you mind capturing a log so we can review the API calls?

Extract any lines that contain lookupresolved or urilist (case insensitively) and share them here. Please review there's nothing sensitive in them before sharing (they should only contain file and package names/paths). If you need to edit anything out, please try to avoid modifying too much (replacing words is fine, but try not to modify anything that might be triggering this bug, such as non-alphanumerics that maybe are also affecting the json parsing).

Thanks!

knowbee commented 5 months ago

@DanTup Here's the file; I only extracted lines containing lookupresolved or urilist and saved them to a separate txt file.

DanTup commented 5 months ago

@knowbee looks like you have a comma in this file here:

So if you rename that file, it should avoid the bug :-)

Shashwat2708 commented 1 month ago

Thank you so much for this fix. For anyone else who is not that technical and is facing the same issue, all you need to do is:

  1. Click on any of the folders or files in V.S. Code(for me all the folders appear on the right).
  2. Then press F3.
  3. Just search for a ",".
  4. Change the file name and also do not forget to change the name in all the other files you might have called it in.
  5. Have fun building!