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.

Sign up for GitHub

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

Jump to bottom

Replace lexical #4774

Closed
jqnatividad opened this issue Sep 4, 2023 · 4 comments
Closed

Replace lexical #4774

jqnatividad opened this issue Sep 4, 2023 · 4 comments
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog good first issue Good for newcomers help wanted

Comments

@jqnatividad
Copy link

Describe the bug
There are recent advisories for lexical, mainly for soundness and being unmaintained.

https://rustsec.org/advisories/RUSTSEC-2023-0055.html
GHSA-c2hm-mjxv-89r4

Expected behavior
lexical replaced with alternative techniques/crates.
@jqnatividad jqnatividad added the bug label Sep 4, 2023
@tustvold
Copy link
Contributor

tustvold commented Sep 4, 2023 •
edited

Marking this help wanted and good first issue as it should be fairly straightforward to make the changes and check the benchmarks for performance regressions. Marking this as a feature request not a bug as AFAICT we do not use any of the impacted methods.

@tustvold tustvold added enhancement Any new improvement worthy of a entry in the changelog and removed bug labels Sep 4, 2023
@Weijun-H Weijun-H mentioned this issue Sep 7, 2023
Closed
@tustvold
Copy link
Contributor

tustvold commented Sep 13, 2023 •
edited

Re-reading the advisory it appears to be for lexical, not lexical-core? Additionally I'm not seeing any soundness issues reported against lexical-core?

Moving away from lexical-core would entail some non-trivial performance regressions, given this I'm somewhat inclined to not pursue this at this time...

@tustvold tustvold closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2023
@tustvold
Copy link
Contributor

Given this regresses performance, and the advisory is for lexical not lexical-core, I am closing this for now. Feel free to reopen if I am mistaken

@tustvold tustvold added the arrow Changes to the arrow crate label Oct 18, 2023
@tustvold
Copy link
Contributor

label_issue.py automatically added labels {'arrow'} from #4785

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog good first issue Good for newcomers help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace usage of lexical-core crate with libcore for parsing strings as floats Weijun-H/arrow-rs
2 participants
@tustvold @jqnatividad