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

Add some safeties around charset detection and transliteration #16123

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

uberbrady
Copy link
Collaborator

In some environments, you need to specify character sets for iconv() in UPPERCASE. So this adds that.

In addition, sometimes the character set transformation can fail (such as when it's not in uppercase), and while that only usually raises an E_NOTICE, Laravel will tend to transform those notices into Exceptions, so we need to catch those.

And finally, if you don't happen to have iconv installed at all, we should gracefully detect that and default to the 'regular' behavior.

Copy link

what-the-diff bot commented Jan 23, 2025

PR Summary

  • Improved Encoding Conversion Process
    The team has updated the way our system converts encoding to better handle errors that might occur during the process.

  • Error Logging Added to Transliteration
    We have added a logging feature to the transliteration process. Now, if the shift from a detected encoding to UTF-8 fails for any reason, we will still have a record of the incident.

  • Adjusted Formatting of Detected Encoding Strings
    We've made changes to how our system deals with the detected encoding strings. Now, it converts these strings to uppercase before handing them to the conversion function, which should help with uniformity.

  • Replaced Transliteration Code Block
    The old piece of code in charge of changing file contents was removed and replaced with a more robust error-handling structure. This should enhance the resilience and efficiency of the process.

  • Memory Optimization Post-Processing
    Finally, we've inserted a function to nullify the variable holding file contents once they have been processed. This change will help optimize the memory usage of our system.

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

Successfully merging this pull request may close these issues.

1 participant