Skip to content
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

Prevent .NET members with names differing only by case from crashing the compatibility profiler #1195

Merged

Conversation

rjmholt
Copy link
Contributor

@rjmholt rjmholt commented Mar 25, 2019

PR Summary

.NET allows type/member names that are case-insensitively equal on the same object (of course), but PowerShell can't differentiate these.

This changes the collector so that it builds a case-sensitive dictionary of types/members for JSON serialisation, rather than crashing when we hit this edge case (for the record, I hit Microsoft.Azure.Management.HDInsight.Job.Models.Profile having both JobID and JobId as properties...).

The query API still handles this without any problems: it just exposes the last property (it overwrites the previous one in the slot if there are duplicates). This is essentially consistent with PowerShell, which exposes the properties by reflection and also clobbers them in an undefined order.

PR Checklist

Copy link
Collaborator

@bergmeister bergmeister left a comment

Choose a reason for hiding this comment

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

Code change looks good to me but please write at least one simple test that would catch a regression

Copy link
Member

@JamesWTruher JamesWTruher left a comment

Choose a reason for hiding this comment

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

lgtm

@JamesWTruher JamesWTruher merged commit df566b5 into PowerShell:development Apr 23, 2019
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.

None yet

3 participants