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

Use strcoll_l() for locale-independent comparisons #2087

Merged
merged 1 commit into from
Feb 9, 2022

Conversation

dan-rigdon-bel
Copy link
Contributor

@dan-rigdon-bel dan-rigdon-bel commented Feb 7, 2022

The update code uses the compareFiles function to determine the sort order of file names in a directory, which is then compared with a list of files in the Sparkle update info. If the list of files of a directory from Sparkle's update is different than what is on disk, the Sparkle abandons the update and does a full download. This bug would prevent Sparkle from updating properly when the file lists are calculated on an English language system, but then checked on a destination Japanese system.

The fix is to use strcoll_l(), which is locale-independent, when comparing file names for sorting. This will ignore any locale changes the host application has made that could potentially affect Sparkle's sort order.

(Insert summary of your pull request here)

Fixes # (issue)

Misc Checklist:

  • My change requires a documentation update on Sparkle's website repository
  • My change requires changes to generate_appcast, generate_keys, or sign_update

Only bug fixes to regressions or security fixes are being backported to the 1.x (master) branch now. If you believe your change is significant enough to backport, please also create a separate pull request against the master branch.

Testing

I tested and verified my change by using one or multiple of these methods:

  • Sparkle Test App
  • Unit Tests
  • My own app
  • Other (please specify)

(Describe all the cases that were tested)

macOS version tested: [place version here]

The update code uses the compareFiles function to determine the sort order of file names in a directory, which is then compared with a list of files in the Sparkle update info. If the list of files of a directory from Sparkle's update is different than what is on disk, the Sparkle abandons the update and does a full download. This bug would prevent Sparkle from updating properly when the file lists are calculated on an English language system, but then checked on a destination Japanese system.

The fix is to use strcoll_l(), which is locale-independent, when comparing file names for sorting. This will ignore any locale changes the host application has made that could potentially affect Sparkle's sort order.
@zorgiepoo
Copy link
Member

zorgiepoo commented Feb 8, 2022

Thank you. I assume cases where this fails is when files have some sort of non-english or accent characters or some such in them (otherwise I am curious what a reproducible case is). I'm also guessing using the c locale is somewhat equivalent to using strcmp(). Either way, change looks good.

@dan-rigdon-bel
Copy link
Contributor Author

I assume cases where this fails is when files have some sort of non-english or accent characters or some such in them

Exactly right. We hit this bug with internationalized file names in Japanese.

@zorgiepoo zorgiepoo merged commit 8edc77f into sparkle-project:2.x Feb 9, 2022
@zorgiepoo
Copy link
Member

I should have asked, do you still have an example of two filenames that don't sort the same way on en_US vs ja_JP?

@dan-rigdon-bel
Copy link
Contributor Author

I should have asked, do you still have an example of two filenames that don't sort the same way on en_US vs ja_JP?

I'm sorry to say that I do not. Reversing our changes to get the list would be challenging.

@zorgiepoo
Copy link
Member

I could probably figure it out if the app was publicly available, but it's not a big deal either way.

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

2 participants