_resolveNearestExistingPath safely resolves symbolic links by traversing up the tree until an ancestor exists, but it previously dropped the components of the path that were skipped. This causes non-existent dummy paths used in tests to incorrectly resolve to filesystem roots (like C:\ on Windows or / on POSIX), causing permission check failures.
This fix computes the relative path suffix between the original path and the closest existing ancestor using path.relative, and appends it back to the resolved path with path.join. POSIX dummy path coverage has also been added to verify identical behavior across platforms.
Fixes: https://github.com/dart-lang/sdk/issues/63147
Change-Id: Iac908998f052ad9054e8213ba059156a5982a301
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/495000
Reviewed-by: Jessy Yameogo <yjessy@google.com>
Auto-Submit: Ben Konyi <bkonyi@google.com>
Commit-Queue: Jessy Yameogo <yjessy@google.com>
Previously, FileSystem.readFileAsString, writeFileAsString, and
listDirectoryContents only validated the requested path textually against the
configured IDE workspace roots. A symlink inside the workspace could
therefore resolve to a location outside the workspace and still be accessed.
This patch resolves workspace roots and requested filesystem targets before
performing the authorization check, and resolves the nearest existing ancestor
for write targets so new files inside a workspace continue to work.
It also adds a source-backed regression test in pkg/dtd_impl/test/dtd_test.dart
that verifies read, write, and directory listing requests through a symlink
escaping the workspace all fail with permission denied.
R=bquinlan@google.com
Tested:
- HOME=/tmp XDG_CONFIG_HOME=/tmp DART_SUPPRESS_ANALYTICS=1 /tmp/dart-sdk-3.12.0-221.0.dev/dart-sdk/bin/dart test pkg/dtd_impl/test/dtd_test.dart
Change-Id: I7abf00f6220bff42b352e2942f396167af53adb8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/493420
Reviewed-by: Jessy Yameogo <yjessy@google.com>
Auto-Submit: 진호 <orangemush777@gmail.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
The `Match.operator[]` does the same thing and is
generally recommended (and shorter).
(I want to deprecate `group` and `groups`)
Tested: Refactoring.
CoreLibraryReviewExempt: Calling equivalent function.
Change-Id: I4c758968ae622fe16b7322be1b29b05b91e7fcd9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489021
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
dtd._recordDtdConnectionInfo() call.
Moved dtd._recordDtdConnectionInfo() to be called immediately after service startup (_startService), ensuring it executes before any console log announces that the daemon is listening.
Change-Id: I41c9b2dac6df54489bcaf9894fb7c3b222775aab
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/488860
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Jaime Wren <jwren@google.com>
This CL is part of an effort to bump the SDK requirement to `3.12.0-0`
for all the packages in `pkg` that are not published to `pub`, so that
we can get better testing of the "private named parameters" feature.
(Packages that *are* published to `pub` can't be safely bumped yet,
because SDK 3.12 hasn't been released, and I don't want to block those
packages' ability to publish useful updates to customers.)
This change covers the following packages, which are owned by
OWNERS_VM:
- pkg/dart2bytecode
- pkg/dart2native
- pkg/dtd_impl (jointly owned with OWNERS_DEVTOOLS)
- pkg/mmap
- pkg/observatory
- pkg/vm
Changes to `pubspec.yaml` files were made manually.
Changes to `.dart` files were made automatically, using `dart
fix`. Two kinds of changes were made:
- Migrate to using private named parameters where it is possible to do
so without changing semantics. Note that this migration is
conservative; see https://github.com/dart-lang/sdk/issues/58607 for
details.
- Migrate to using null aware elements.
Tested: Standard trybots
Change-Id: I1666e5771a34ef3c30ceeadced3d505d6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/487942
Auto-Submit: Paul Berry <paulberry@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
The WebSocket client that IntelliJ is using does not handle pings (despite them being part of the spec), so this change resulted in connections being dropped after 15s.
This changes pings to be opt-in, which unfortunately means other clients that don't opt-in won't get the behaviour (and therefore might see issues with Norton and other antivirus programs that proxy and drop idle connections), but this is better than all IJ users being broken.
See https://github.com/flutter/dart-intellij-third-party/issues/205
Change-Id: I7d5f5225cb9d04c15552970a821f5441c45ace18
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/472580
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This enables the web socket ping by default at an interval of 15s. Having ping enables can prevent proxies/antivirus from dropping the web connections if they have no traffic for some period.
Having Norton 360 installed results in idle DTD connections dropping after 60s even if every feature is disabled, because the connections still go through it and it still drops idle connections.
I implemented this in the server because it applies to all uses of DTD regardless of client and it was easier to do here because pingInterval isn't exposed everywhere (but pkg:shelf does have a pass-through).
See https://github.com/Dart-Code/Dart-Code/issues/5794
Fixes https://github.com/dart-lang/sdk/issues/62148
Change-Id: If9df674b061b397cef1aa321602b563f44b9525d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465680
Reviewed-by: Jessy Yameogo <yjessy@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This CL:
- changes the name of the `ConnectedApp.getVmServiceUris` service method to `ConnectedApp.getVmServices` since the response includes more than just the URI for each VM service object.
- Adds a new response type `VmServicesResponse`
- Enhances test coverage for existing response types
Work towards https://github.com/dart-lang/sdk/issues/60540.
Change-Id: Id4a1c05dde28c9b23975ad1d2ca68cb9e2bddb22
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/430545
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Kenzie Davisson <kenzieschmoll@google.com>
This CL contains breaking changes for package:dtd and prepares both package:dart_service_protocol_shared and package:dtd for publish.
This CL also fixes https://github.com/dart-lang/sdk/issues/60757 so that DTD-registered services are sent over the `Service` stream upon initial subscription like what is done for client-registered services.
Change-Id: I619af816e64af01864c7ed9b98743c6691bf7e0b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429161
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Kenzie Davisson <kenzieschmoll@google.com>
This is a reland of commit b9b77058a9
Original change's description:
> Migrate to use pub workspace
>
> Use `pub get` to generate `.dart_tool/package_config.json` on gclient sync.
>
> All pkg/ (and a few third_party) packages that are developed inside the sdk repo are included in the workspace from the root `pubspec.yaml`.
>
> All dependencies that are pulled in via DEPS are added as path dependencies via `dependency_overrides` in the root `pubspec.yaml`.
>
> Bug: https://github.com/dart-lang/sdk/issues/56220
> Change-Id: I38c12b608c68da54c57821116cf9aa6696936746
> Tested: relies on CQ of existing tests. Should have no effect on functionality
> CoreLibraryReviewExempt: only core library change is adding a `// ignore:` comment. Should have no influence on functionality
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/397164
> Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
> Reviewed-by: Alexander Thomas <athom@google.com>
Bug: https://github.com/dart-lang/sdk/issues/56220
Change-Id: I29afabade2d2447dea05121cb87ff50bb21a4b76
Cq-Include-Trybots: luci.dart.try:flutter-linux-try,flutter-web-try
Tested: relies on CQ of existing tests. Should have no effect on functionality
CoreLibraryReviewExempt: only core library change is adding a `//
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415561
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
This reverts commit b9b77058a9.
Revert "Add missing sample pubspec to workspace"
This reverts commit 892ea15ac7.
These seem to break the engine.
Change-Id: Ieee26deb7928c3869a1b6265326c3ce568ffe731
Tested: this is a revert.
CoreLibraryReviewExempt: this is a revert
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415582
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Use `pub get` to generate `.dart_tool/package_config.json` on gclient sync.
All pkg/ (and a few third_party) packages that are developed inside the sdk repo are included in the workspace from the root `pubspec.yaml`.
All dependencies that are pulled in via DEPS are added as path dependencies via `dependency_overrides` in the root `pubspec.yaml`.
Bug: https://github.com/dart-lang/sdk/issues/56220
Change-Id: I38c12b608c68da54c57821116cf9aa6696936746
Tested: relies on CQ of existing tests. Should have no effect on functionality
CoreLibraryReviewExempt: only core library change is adding a `// ignore:` comment. Should have no influence on functionality
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/397164
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
This adds additional "Editor" service method definitions as used by the new DTD sidebar. I moved the Editor service definitions into a new file (because they're now quite large and dtd_common_services seems better as an intro and list of the common specified services).
I also split the existing info about `navigateToCode` into a more terse definition (in the new file) and a general example. I also updated the error example as I believe it was incorrectly wrapped in `result`.
Change-Id: I6136eb550cce0dab86d535db02f9f06ee42892e0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/378141
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This is a proposal for a standardized service method for IDEs to register for navigating to code. This is intended to replace the `navigate` `ToolEvent` since there isn't a way for an IDE to respond to that notification and verify that the DevTools-IDE interaction was successful.
Change-Id: I92c0ad36b3f0557ffb80b868b466808ef3c3563d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375080
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Helin Shiah <helinx@google.com>
This question came up in another change, about whether a client would have hanging requests if the other DTD client providing the service disconnected.
This adds a test to ensure that any pending requests are terminated with an error.
It also tidies up some other tests to give better errors if they fail.
Change-Id: I77e04ee59f757498af090878da542d6ed1d152b0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/374946
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Removes the port from `startService()` (which as far as I can see is unused) and parses it from `args` instead.
I had to add support for "options" in the argument classes and switched from a bool flag `isFlag` to an enum to be a bit more descriptive (and be easier to support multi-options in future if required).
Fixes https://github.com/dart-lang/sdk/issues/56000
Change-Id: Ib08513139f58e8845a7c2cbbeeea06787dbc2df1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/371840
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Dan Chevalier <danchevalier@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
- devtools server can now take the dtd uri as a parameter on startup
- dtdUri is passed to the server handler so that devtools can request it.
- FileService is implemented inside the Dart Tooling Daemon (dtd_impl)
- Added FileService.setProjectRoots
- Added unit tests and rpc exceptions
- on startup dtd now prints { uri, secret } so that the caller has the secret that allows them to call FileService.setProjectRoots
Fixes: https://github.com/dart-lang/sdk/issues/54790
Change-Id: I7a66aca2f8f06f6a611a46c727a9d5c2485fbe3e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346922
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Dan Chevalier <danchevalier@google.com>