Skip to content

Fix failure on getting a list with out of range offset #1947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 10, 2023

Conversation

annagrin
Copy link
Contributor

@annagrin annagrin commented Feb 9, 2023

Fix failure on getObject on list with offset greater or equal than the list length.

  • Add a check and return empty range instead.
  • Add corresponding tests for lists and maps.

Closes: #1948

@@ -495,6 +501,9 @@ class Debugger extends Domain {
int? length,
}) async {
String rangeId = objectId;
if (_isEmptyRange(offset: offset, count: count, length: length)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Optional] Conceptually I think an empty range is a type of subrange. Would it make sense to have isSubRange return true if it's an empty range, and _subRange return []?

Copy link
Contributor Author

@annagrin annagrin Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_subrange does not return a list but a remote object that refers to the sublist. I want to avoid unnecessary 2 roundtrips through the chrome debugger to get the empty list properties when we have all the information available statically.
Alternatively, we could make _subRange return null and test for it and return an empty list as a result but I think the current way is clearer...
Basically, It is a bug fix (for when offset >= length) but also is just an optimization for all empty cases we can detect without the runtime information.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG

Copy link
Contributor

@elliette elliette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@annagrin annagrin mentioned this pull request Feb 10, 2023
@annagrin annagrin merged commit d6229e3 into dart-lang:master Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getObject on list fails if offset is out of bounds
2 participants