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 of locale-sensitive methods with undefined locale may cause environment-sensitive bugs #6016

Closed
2 of 3 tasks
lionel-rowe opened this issue Sep 19, 2024 · 3 comments · Fixed by #6204
Closed
2 of 3 tasks
Labels
bug Something isn't working needs triage

Comments

@lionel-rowe
Copy link
Contributor

lionel-rowe commented Sep 19, 2024

Describe the bug

I've mentioned this before tangentially in a couple of issues (ex), but it's probably worth its own issue.

Use of locale-sensitive methods, such as toLocaleLowerCase, with an unspecified (undefined) locale may cause environment-sensitive bugs, because the default locale is host defined.

Steps to Reproduce

For example, in http/cookie.ts, which is marked browser-compatible:

switch (key.toLocaleLowerCase()) {

  1. Open Firefox
  2. Go to https://codepen.io/lionel-rowe/pen/rNEXmoG?editors=0010:
    import { getSetCookies } from 'https://esm.sh/jsr/@std/[email protected]/cookie'
    
    const setCookie = 'a=b; EXPIRES=Thu, 19 Sep 2024 07:47:28 GMT'
    const headers = new Headers({ 'set-cookie': setCookie })
    const cookie = getSetCookies(headers)[0]
                                
    document.querySelector('#target').textContent = JSON.stringify(cookie, null, 4)
  3. Observe expected output (assuming your locale isn't Turkish):
    {
        "name": "a",
        "value": "b",
        "expires": "2024-09-19T07:47:28.000Z"
    }
    
  4. Go to about:preferences
  5. Set language to Turkish ("Türkçe"; Turkish for "language" is "dil" if you need to find the setting again)
  6. Go back to or refresh https://codepen.io/lionel-rowe/pen/rNEXmoG?editors=0010 page
  7. Observe output:
    {
        "name": "a",
        "value": "b",
        "unparsed": [
            "EXPIRES=Thu, 19 Sep 2024 07:47:28 GMT"
        ]
    }
    

Expected behavior

Consistent behavior in all locales

Environment


  • http/cookie
  • http/negotiation
  • text
@lionel-rowe lionel-rowe added bug Something isn't working needs triage labels Sep 19, 2024
@lionel-rowe
Copy link
Contributor Author

As far as I can see, the problematic uses are toLocaleLowerCase and toLocaleUpperCase in http/cookie.ts, http/_negotiation/encoding.ts, and in the various casing functions under text. By contrast, fmt/bytes.ts is a best practice for allowing opt-in locale sniffing:

std/fmt/bytes.ts

Lines 193 to 195 in 6a4eb6c

* - If locale is a string, the value is expected to be a locale-key (for example: `de`).
* - If locale is true, the system default locale is used for translation.
* - If no value for locale is specified, the number is returned unmodified.

and testing it with minimal hard-coding:

std/fmt/bytes_test.ts

Lines 9 to 10 in 6a4eb6c

const decimal = parts.find(({ type }) => type === "decimal")!.value;
const group = parts.find(({ type }) => type === "group")!.value;

std/fmt/bytes_test.ts

Lines 75 to 82 in 6a4eb6c

assertEquals(format(-0.4, { locale: true }), `-0${decimal}4 B`);
assertEquals(format(0.4, { locale: true }), `0${decimal}4 B`);
assertEquals(format(1001, { locale: true }), "1 kB");
assertEquals(format(10.1, { locale: true }), `10${decimal}1 B`);
assertEquals(
format(1e30, { locale: true }),
`1${group}000${group}000 YB`,
);

@kt3k
Copy link
Member

kt3k commented Nov 22, 2024

Can this be closed now?

@lionel-rowe
Copy link
Contributor Author

Can this be closed now?

I opened #6204 with the minimal fix for text, which is just replacing all occurrences of toLocaleXCase with toXCase. Maybe in future an option could be added to specify a locale and/or opt in to host locale sensitivity, if there's demand for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants