Skip to content

[release/6.0] Fix parsing the output of ping on Samsung phones #80692

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 17, 2023
Merged

[release/6.0] Fix parsing the output of ping on Samsung phones #80692

merged 3 commits into from
Jan 17, 2023

Conversation

simonrozsival
Copy link
Member

@simonrozsival simonrozsival commented Jan 16, 2023 •
edited by karelz
Loading

Backport of #80133 and (older 7.0 fix) #65312 to release/6.0

Fixes #78990

Unlike in #80502, we need to backport #65312 that wasn't originally backported to 6.0.

Customer Impact

Some Android installations contain a ping utility that produces slightly different output when TTL is exceeded from what our code expects. This seems to be the case at least on Samsung phones. This PR updates the parser to account for this variation.

Testing

  • Manual testing on a Samsung phone (API 31) and on Android emulators of various API levels.
  • CI - Android emulator (API 29)

Risk

Low.

rzikm and others added 3 commits January 13, 2023 18:42
@ghost ghost added the area-System.Net label Jan 16, 2023
@ghost ghost assigned simonrozsival Jan 16, 2023
@ghost
Copy link

ghost commented Jan 16, 2023

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #65312 and #80133 to release/6.0

Unlike in #80502, we need to backport #65312 that wasn't originally backported to 6.0.

Customer Impact

Some Android installations contain a ping utility that produces slightly different output when TTL is exceeded from what our code expects. This seems to be the case at least on Samsung phones. This PR updates the parser to account for this variation.

Testing

  • Manual testing on a Samsung phone (API 31) and on Android emulators of various API levels.
  • CI - Android emulator (API 29)

Risk

Low.

Author: simonrozsival
Assignees: -
Labels:

area-System.Net
Milestone: -

@dotnet dotnet deleted a comment from azure-pipelines bot Jan 16, 2023
@simonrozsival simonrozsival requested review from rzikm and wfurt January 16, 2023 15:18
@simonrozsival simonrozsival added this to the 6.0.x milestone Jan 16, 2023
@simonrozsival simonrozsival added the Servicing-consider Issue for next servicing release review label Jan 16, 2023
@simonrozsival simonrozsival marked this pull request as ready for review January 16, 2023 15:19
@build-analysis build-analysis bot mentioned this pull request Jan 16, 2023
Closed
wfurt
wfurt approved these changes Jan 16, 2023
View reviewed changes
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 17, 2023
@rbhanda rbhanda modified the milestones: 6.0.x, 6.0.13, 6.0.14 Jan 17, 2023
@carlossanlop
Copy link
Contributor

Approved by Tactics (6.0.14).
Signed off by area owner.
CI failures are unrelated and pre-existing: "No hay más extremos disponibles desde el asignador de extremos." which translates to "No more endpoints available from the endpoint mapper". Seems to affect other PRs, specifically in Windows.10.Amd64.Server2022.ES.Open.
No OOB changes needed for System.Net.Ping (No IsPackable=true).
Ready to merge. :shipit: cc @mmitche

@carlossanlop carlossanlop merged commit 8e8a6e0 into dotnet:release/6.0 Jan 17, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wfurt wfurt wfurt approved these changes

@rzikm rzikm Awaiting requested review from rzikm

Assignees

@simonrozsival simonrozsival

Labels
area-System.Net Servicing-approved Approved for servicing release
Projects
None yet
Milestone
6.0.14
Development

Successfully merging this pull request may close these issues.
5 participants
@simonrozsival @carlossanlop @wfurt @rbhanda @rzikm