Skip to content

Conversation

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Dec 31, 2018

PR Summary

Ubuntu18.04 seems to default to C.UTF-8 for LANG (representing InvariantCulture) which results in a case-sensitive hashtable since CurrentCultureIgnoreCase doesn't work for that culture. Fix is to use OrdinalIgnoreCase instead.

Fix #7761

PR Checklist

change hashtable to use ordinalignorecase comparer
@lzybkr
Copy link
Contributor

lzybkr commented Dec 31, 2018

I'm pretty sure this is a breaking change.

@SteveL-MSFT SteveL-MSFT added Breaking-Change breaking change that may affect users Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Dec 31, 2018
@iSazonov
Copy link
Collaborator

Ubuntu18.04 seems to default to C.UTF-8 for LANG (representing InvariantCulture)

Could you please reference where it come from and why?

Is there other cultures like C.UTF-8 on Unix and Windows with the problem?

@SteveL-MSFT
Copy link
Member Author

@iSazonov read the details in the related issue

@iSazonov
Copy link
Collaborator

Perhaps it makes sense to consider wider the culture-insensitive and case-insensitive in PowerShell Committee review. Related #8064 (Not all code base fixed as mentioned in #8068).
For #8120 and #8515 we need find all places where we could be the culture-insensitive and case-insensitive and use the simple case folding comparer.
Another side is issues like #8180. Perhaps sometimes it makes sense to have default to ordinal ignore case in string comparisons.

@ffeldhaus
Copy link
Contributor

ffeldhaus commented Dec 31, 2018

@iSazonov In my opinion this pull request correctly fixes the issue described in #7761 and we should keep wider discussion of changing other culture-insensitive in other places out of this pull request.

As this is a serious issue in a core functionality (Hashtable) and as it is present on the default culture of Ubuntu 18.04 (C.utf8) and thus exposed to a large number of users in my opinion this should be merged with high priority and be backported to 6.1.X and 6.0.X.

@iSazonov
Copy link
Collaborator

My request is not to block the PR. My fear is that there may be other places prone to this problem. This analysis may be useful for fixing other issues.

Copy link
Contributor

@ffeldhaus ffeldhaus left a comment

Choose a reason for hiding this comment

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

Issues also affects Mac OS, so test should run on Mac OS as well.

….Tests.ps1

Co-Authored-By: SteveL-MSFT <slee@microsoft.com>
Copy link
Contributor

@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.

Looks good to me, only a minor, optional suggestion to improve readability of the test :)

Copy link
Contributor

@ffeldhaus ffeldhaus left a comment

Choose a reason for hiding this comment

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

Addresses the issue and test is good as well now!

….Tests.ps1

Co-Authored-By: SteveL-MSFT <slee@microsoft.com>
@SteveL-MSFT
Copy link
Member Author

@PowerShell/powershell-committee reviewed this and agrees to take this change as the intention was to always have Hashtable keys case-insensitive

@SteveL-MSFT SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jan 9, 2019
@iSazonov iSazonov added CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log labels Jan 10, 2019
@iSazonov iSazonov merged commit 9793ed2 into PowerShell:master Jan 10, 2019
@SteveL-MSFT SteveL-MSFT deleted the hashtable-case branch January 10, 2019 04:14
@TravisEz13 TravisEz13 removed the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jan 16, 2019
@TravisEz13
Copy link
Member

@iSazonov Only one CL-* tag. If it is a breaking change, don't put it in one of the others.

@adamedx
Copy link

adamedx commented Feb 2, 2019

Wow, thank you for fixing this. I am new to PowerShell Core and using it on Linux, and it seemed that this was intentional. I sent a lot of time fixing the casing of my hash tables. I'm guessing this is also the issue I ran into with sort-object, which gives different results on Linux and Windows:

PS> $PSVersionTable.Platform;('a', 'ab', 'aC' | Sort-Object ) -join ','
Win32NT
a,ab,aC
PS> $PSVersionTable.Platform;('a', 'ab', 'aC' | Sort-Object ) -join ','
Unix
a,aC,ab

@ffeldhaus
Copy link
Contributor

@adamedx I think it would be a good idea to track this in a separate issue and link it to #8064 where similar issues are being tracked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking-Change breaking change that may affect users CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log Committee-Reviewed PS-Committee has reviewed this and made a decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Powershell Hashtable Keys are case-sensitive on Ubuntu 18.04

8 participants