Skip to content

Don't allow mutating the ScopeId on static readonly IPAddress fields #115456

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 1 commit into from
May 12, 2025

Conversation

AustinWise
Copy link
Contributor

@AustinWise AustinWise commented May 11, 2025

The IPAddress class has some static readonly instances of IPAddress declared on it. The Address field of these instances was made readonly in dotnet/corefx#33531. Somehow ScopeId, the only other mutable property, was missed. I did not see discussion of this in that PR or the related issue.

An example program that illustrates the problem:

Console.WriteLine(IPAddress.IPv6Loopback);
IPAddress.IPv6Loopback.ScopeId = 1;
Console.WriteLine(IPAddress.IPv6Loopback);

When run, this program prints the following, showing that the static readonly instance has been mutated:

::1
::1%1

I think the right behavior is to throw when setting the ScopeId (this pr).

Contributes to #27870

The Address field was made readonly in dotnet/corefx#33531. Somehow ScopeId,
the only other mutatable property, was missed. I did not see discussion of
this in that PR or the related issue.

An example program that illistrates the problem:

```csharp
Console.WriteLine(IPAddress.IPv6Loopback);
IPAddress.IPv6Loopback.ScopeId = 1;
Console.WriteLine(IPAddress.IPv6Loopback);
```

This prints:

```txt
::1
::1%1
```

I think the right behavior is to throw when setting the ScopeId (this pr).

Contributes to dotnet#27870
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 11, 2025
Copy link
Contributor

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

Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable change to me.
Thanks

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.

nice.

@wfurt
Copy link
Member

wfurt commented May 12, 2025

test failures are unrelated.

@wfurt wfurt merged commit ffcdaeb into dotnet:main May 12, 2025
86 of 88 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants