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

update ada-idna to v0.3.1 #911

Merged
merged 5 commits into from
Mar 9, 2025
Merged

update ada-idna to v0.3.1 #911

merged 5 commits into from
Mar 9, 2025

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Mar 8, 2025

No description provided.

@anonrig anonrig requested a review from lemire March 8, 2025 18:19
@anonrig anonrig changed the title Yagiz/update ada idna update ada-idna to v0.3.1 Mar 8, 2025
@anonrig anonrig force-pushed the yagiz/update-ada-idna branch from 77948ab to 2c11e8b Compare March 8, 2025 18:19
@anonrig
Copy link
Member Author

anonrig commented Mar 8, 2025

@lemire It seems simdjson is failing: "JSON error: STRING_ERROR: Problem while parsing a string near"

@anonrig
Copy link
Member Author

anonrig commented Mar 8, 2025

@lemire These 2 tests are offending simdjson: 32f622e

@anonrig
Copy link
Member Author

anonrig commented Mar 8, 2025

Now fuzzer seems to be broken with the error:

idna.cc:(.text.LLVMFuzzerTestOneInput[LLVMFuzzerTestOneInput]+0x285): undefined reference to `ada::idna::ascii_has_upper_case(char*, unsigned long)'

@lemire
Copy link
Member

lemire commented Mar 8, 2025

@anonrig The following

{
    "comment": "V7; A3",
    "input": "a\ud900z",
    "output": null
  }

is not valid JSON. There is no way to represent "a\ud900z" as a valid Unicode string.

You have a few options:

  1. You skip these tests, do not call FAIL. That's what we do in idna currently.
  2. You can call get_wobbly_string() to get a WTF-8 input. Note that I am not sure that the standard tells us what to do with non-valid UTF-8 and all our functions are defined as expecting valid UTF-8. We could validate our inputs (reject invalid Unicode inputs) but that must be deliberate.

@anonrig
Copy link
Member Author

anonrig commented Mar 9, 2025

Ah I see @lemire. Thanks. I recommend omitting the error. What do you think about the fuzzing error? I believe it occurs due to the constexpr function, but I don't have a solution other than removing the constexpr qualifier, which is not ideal.

@anonrig anonrig merged commit af0770e into main Mar 9, 2025
38 checks passed
@anonrig anonrig deleted the yagiz/update-ada-idna branch March 9, 2025 18:55
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.

2 participants