# [PATCH] Bug 43477 - Unicode error mode ignored - Thread: https://externals.io/message/36185 - Original archive: https://news-web.php.net/php.internals/36185 - Started by Stephen Bach on 2008-03-18T03:51:12+00:00 - Messages: 12 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #36185 — Stephen Bach — 2008-03-18T03:51:12+00:00 URL: https://externals.io/message/36185#36185 Attached is a simple proposed patch that fixes Bug 43477. Basically, the code that set the error mode of the ICU converter was giving it an instruction (the context parameter) to only skip or substitute if the code point was not represented in the new encoding. However, it still was returning an error for illegal sequences. The test suite returns the same results with or without the patch. Test also attached. -Stephen Bach --- ## #36193 — Andrei Zmievski — 2008-03-18T17:43:13+00:00 URL: https://externals.io/message/36185#36193 · In reply to: #36185 Why would we not want to stop on illegal sequences? -Andrei Stephen Bach wrote: > Attached is a simple proposed patch that fixes Bug 43477. Basically, the code > that set the error mode of the ICU converter was giving it an instruction > (the context parameter) to only skip or substitute if the code point was not > represented in the new encoding. However, it still was returning an error for > illegal sequences. > > The test suite returns the same results with or without the patch. Test also > attached. > > -Stephen Bach --- ## #36196 — Geoffrey Sneddon — 2008-03-18T18:25:00+00:00 URL: https://externals.io/message/36185#36196 · In reply to: #36193 > Why would we not want to stop on illegal sequences? Stuff relies on things _not_ stopping. No web browser stops on an illegal sequence: they all use some replacement character (U+FFFD REPLACEMENT CHARACTER in most). Sure, idealists will say that stopping on errors will get the errors fixed, but that manifestly isn't true. There's a huge amount of ill-formed XML out there. Shipping something that required strings to be valid is just asking to not be used. It breaks too much. We already claim to have error modes that don't stop on error. This is broken. -- Geoffrey Sneddon Why would we not want to stop on illegal sequences? > > -Andrei > > Stephen Bach wrote: > > […] --- ## #36199 — Geoffrey Sneddon — 2008-03-18T22:02:36+00:00 URL: https://externals.io/message/36185#36199 · In reply to: #36198 > Shouldn't U_CONV_ERROR_SKIP and U_CONV_ERROR_SUBST work the same way? I guess: U_CONV_ERROR_SKIP is just U_CONV_ERROR_SUBST with the substitution string as nothing, though I expect slight speed gains could be made by keeping them separate (due to no attempt to even add anything after coming across an invalid sequence — though the speed gains will be very slight). -- Geoffrey Sneddon > > […] > > I guess: U_CONV_ERROR_SKIP is just U_CONV_ERROR_SUBST with the > substitution string as nothing, though I expect slight speed gains > could be made by keeping them separate (due to no attempt to even add > anything after coming across an invalid sequence — though the speed > gains will be very slight). > > > -- > Geoffrey Sneddon > Sorry for the ambiguity. Allow me to clarify: I meant that U_CONV_ERROR_SKIP > and U_CONV_ERROR_SUBST should work the same as the other error modes. > Otherwise, what's the point of having them? > > -Stephen > > On Tuesday 18 March 2008 06:02:36 pm Geoffrey wrote: > > […] --- ## #36232 — Antony Dovgal — 2008-03-21T12:09:37+00:00 URL: https://externals.io/message/36185#36232 · In reply to: #36185 > Attached is a simple proposed patch that fixes Bug 43477. Basically, the code > that set the error mode of the ICU converter was giving it an instruction > (the context parameter) to only skip or substitute if the code point was not > represented in the new encoding. However, it still was returning an error for > illegal sequences. > > The test suite returns the same results with or without the patch. Test also > attached. Patch committed, thanks. -- Wbr, Antony Dovgal --- ## #36233 — Geoffrey Sneddon — 2008-03-21T15:28:46+00:00 URL: https://externals.io/message/36185#36233 · In reply to: #36232 > > > […] > > Patch committed, thanks. Can we test U_CONV_ERROR_SUBST too? See attached patch. Also, the bug should be closed. -- Geoffrey Sneddon > […] > > Can we test U_CONV_ERROR_SUBST too? See attached patch. Also, the bug > should be closed. The patch breaks the test. Can you guys decide on what should work and how, I'll commit the patch afterwards, ok? -- Wbr, Antony Dovgal --- ## #36239 — Stephen Bach — 2008-03-21T16:57:26+00:00 URL: https://externals.io/message/36185#36239 · In reply to: #36234 The original patch works as it should. Substitution with user-defined characters only works in the "FROM_UNICODE" case. When converting to Unicode, U+FFFD is always substituted because it is the standard substitution character. Even the ICU library does not allow this to be changed (with good reason). As for the test, *I think* the testing script would have to be Unicode-compatible and the test script would have to be in some UTF encoding to verify that U+FFFD was substituted. I don't know if this is the case yet. On Friday 21 March 2008 12:06:34 pm you wrote: > > > […] > > The patch breaks the test. > > Can you guys decide on what should work and how, I'll commit the patch > afterwards, ok? --- ## #36437 — Andrei Zmievski — 2008-03-24T16:55:37+00:00 URL: https://externals.io/message/36185#36437 · In reply to: #36233 Where is the patch? Geoffrey Sneddon wrote: > > > > > […] > > Can we test U_CONV_ERROR_SUBST too? See attached patch. Also, the bug > should be closed. > > > -- > Geoffrey Sneddon >