Uh oh!
There was an error while loading. Please reload this page.
Promote unknown encoding warning to ValueError in Mbstring extension - #5317
Promote unknown encoding warning to ValueError in Mbstring extension#5317Girgias wants to merge 1 commit into
Conversation
Uh oh!
There was an error while loading. Please reload this page.
nikic
left a comment
There was a problem hiding this comment.
Seems sensible to convert this to an Error. I might have gone on a small refactoring spree after seeing some stuff in here...
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
0aafb9b to
7e7e4feCompareThere was a problem hiding this comment.
@nikic not sure I understood your comment about mb_internal_encoding with an INI setting so I added this tests which well gives something kinda surprising :|
There was a problem hiding this comment.
So, the exception just ends up being ignored?
There was a problem hiding this comment.
Maybe? looking at the mb_internal_encoding_ini_basic2.phpt test I'm not even sure that using mb_internal_encoding() actually changes the value of the INI setting.
There was a problem hiding this comment.
Yeah, mbstring.internal_encoding and mb_internal_encoding() use separate state :) There's default_charset, internal_encoding, mbstring.internal_encoding and mb_internal_encoding() that form a hierarchy, but are all separate. Go figure ;)
Looking at the code it looks like we just silently ignore incorrect encodings and instead assign UTF-8. Ugh.
There was a problem hiding this comment.
Funzies, maybe we should document this somewhere? Should I add a comment somewhere in mbstring or add a new file in the docs/ folder?
Uh oh!
There was an error while loading. Please reload this page.
Promotes only the warnings where the encoding comes only from a string. Functions which accept an array of encodings will be fixed at a later stage. Closes GH-
7e7e4fe to
c801e7cCompare
This promotes and standardizes the various invalid/unknown encoding warnings to ValueErrors.
I needed to change a couple of the internal API to allow the pass through of the argument position.
I didn't touch the
Unsupported encodingwarnings which appear inmb_ord()andmb_chr()functions yet as I'm not sure what to do with them because I think it would make more sense to integrate them in the main check and dispatch theUnsupported encodingconsistently.