This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch harden_character_set_conversion in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit b956afbb7fb366453eb7888e66fb73a9135bd918 Author: Stephen Webb <[email protected]> AuthorDate: Mon Aug 17 12:44:58 2026 +1000 Encode an invalid codepoint using a charset-specific character --- src/fuzzers/cpp/TranscoderFuzzer.cpp | 2 +- src/main/cpp/charsetencoder.cpp | 228 ++++++++++++++++------ src/main/cpp/jsonlayout.cpp | 19 +- src/main/cpp/telnetappender.cpp | 13 +- src/main/cpp/transcoder.cpp | 97 +++------ src/main/cpp/transform.cpp | 43 +--- src/main/include/log4cxx/helpers/charsetencoder.h | 7 + src/main/include/log4cxx/helpers/transcoder.h | 49 ++++- src/test/cpp/helpers/charsetencodertestcase.cpp | 7 +- src/test/cpp/helpers/transcodertestcase.cpp | 47 ++--- 10 files changed, 281 insertions(+), 231 deletions(-) diff --git a/src/fuzzers/cpp/TranscoderFuzzer.cpp b/src/fuzzers/cpp/TranscoderFuzzer.cpp index 12850a37..6ccf0188 100644 --- a/src/fuzzers/cpp/TranscoderFuzzer.cpp +++ b/src/fuzzers/cpp/TranscoderFuzzer.cpp @@ -136,7 +136,7 @@ namespace if (CharsetDecoder::isError(stat)) { - out.append(1, (logchar) Transcoder::LOSSCHAR); + Transcoder::encode(Transcoder::LOSSCHAR, out); buf.increment_position(1); } else if (buf.position() == before) diff --git a/src/main/cpp/charsetencoder.cpp b/src/main/cpp/charsetencoder.cpp index ce93dc9d..3a444f06 100644 --- a/src/main/cpp/charsetencoder.cpp +++ b/src/main/cpp/charsetencoder.cpp @@ -35,6 +35,12 @@ #include <stdlib.h> #endif +#if 15 < LOG4CXX_ABI_VERSION +#define LOG4CXX_16_VIRTUAL_SPECIFIER override +#else +#define LOG4CXX_16_VIRTUAL_SPECIFIER +#endif + using namespace LOG4CXX_NS; using namespace LOG4CXX_NS::helpers; @@ -82,7 +88,7 @@ class APRCharsetEncoder : public CharsetEncoder virtual log4cxx_status_t encode(const LogString& in, LogString::const_iterator& iter, - ByteBuffer& out) + ByteBuffer& out) override { apr_status_t stat; size_t outbytes_left = out.remaining(); @@ -116,6 +122,19 @@ class APRCharsetEncoder : public CharsetEncoder return stat; } + /** + * Add onto \c out an encoded equivalent of \c codePoint. + */ + log4cxx_status_t encode(unsigned int codePoint, ByteBuffer& out) LOG4CXX_16_VIRTUAL_SPECIFIER + { + apr_status_t result = APR_SUCCESS; + if (codePoint <= 0x10FFFF) + Transcoder::encodeUTF8(codePoint, out); + else + result = APR_BADARG; + return result; + } + private: APRCharsetEncoder(const APRCharsetEncoder&); APRCharsetEncoder& operator=(const APRCharsetEncoder&); @@ -141,7 +160,7 @@ class WcstombsCharsetEncoder : public CharsetEncoder */ log4cxx_status_t encode(const LogString& in, LogString::const_iterator& iter, - ByteBuffer& out) + ByteBuffer& out) override { log4cxx_status_t stat = APR_SUCCESS; @@ -224,7 +243,7 @@ class USASCIICharsetEncoder : public CharsetEncoder virtual log4cxx_status_t encode(const LogString& in, LogString::const_iterator& iter, - ByteBuffer& out) + ByteBuffer& out) override { log4cxx_status_t stat = APR_SUCCESS; @@ -251,6 +270,21 @@ class USASCIICharsetEncoder : public CharsetEncoder return stat; } + /** + * Add onto \c out an encoded equivalent of \c codePoint. + */ + log4cxx_status_t encode(unsigned int codePoint, ByteBuffer& out) LOG4CXX_16_VIRTUAL_SPECIFIER + { + apr_status_t result = APR_SUCCESS; + if (codePoint <= 0x7F) + out.put(static_cast<char>(codePoint)); + else if (Transcoder::LOSSCHAR == codePoint) + out.put('?'); + else + result = APR_BADARG; + return result; + } + private: USASCIICharsetEncoder(const USASCIICharsetEncoder&); USASCIICharsetEncoder& operator=(const USASCIICharsetEncoder&); @@ -268,33 +302,41 @@ class ISOLatinCharsetEncoder : public CharsetEncoder virtual log4cxx_status_t encode(const LogString& in, LogString::const_iterator& iter, - ByteBuffer& out) + ByteBuffer& out) override { log4cxx_status_t stat = APR_SUCCESS; - if (iter != in.end()) + while (out.remaining() > 0 && iter != in.end()) { - while (out.remaining() > 0 && iter != in.end()) + LogString::const_iterator prev(iter); + unsigned int sv = Transcoder::decode(in, iter); + if (sv <= 0xFF) + out.put(static_cast<char>(sv)); + else { - LogString::const_iterator prev(iter); - unsigned int sv = Transcoder::decode(in, iter); - - if (sv <= 0xFF) - { - out.put((char) sv); - } - else - { - iter = prev; - stat = APR_BADARG; - break; - } + iter = prev; + stat = APR_BADARG; + break; } } - return stat; } + /** + * Add onto \c out an encoded equivalent of \c codePoint. + */ + log4cxx_status_t encode(unsigned int codePoint, ByteBuffer& out) LOG4CXX_16_VIRTUAL_SPECIFIER + { + apr_status_t result = APR_SUCCESS; + if (codePoint <= 0xFF) + out.put(static_cast<char>(codePoint)); + else if (Transcoder::LOSSCHAR == codePoint) + out.put('?'); + else + result = APR_BADARG; + return result; + } + private: ISOLatinCharsetEncoder(const ISOLatinCharsetEncoder&); ISOLatinCharsetEncoder& operator=(const ISOLatinCharsetEncoder&); @@ -313,7 +355,7 @@ class TrivialCharsetEncoder : public CharsetEncoder virtual log4cxx_status_t encode(const LogString& in, LogString::const_iterator& iter, - ByteBuffer& out) + ByteBuffer& out) override { if (iter != in.end()) { @@ -334,14 +376,26 @@ class TrivialCharsetEncoder : public CharsetEncoder return APR_SUCCESS; } + /** + * Add onto \c out an encoded equivalent of \c codePoint. + */ + log4cxx_status_t encode(unsigned int codePoint, ByteBuffer& out) LOG4CXX_16_VIRTUAL_SPECIFIER + { + apr_status_t result = APR_SUCCESS; + if (codePoint <= 0xFF) + out.put(static_cast<char>(codePoint)); + else if (Transcoder::LOSSCHAR == codePoint) + out.put('?'); + else + result = APR_BADARG; + return result; + } + private: TrivialCharsetEncoder(const TrivialCharsetEncoder&); TrivialCharsetEncoder& operator=(const TrivialCharsetEncoder&); }; -#if LOG4CXX_LOGCHAR_IS_UTF8 -typedef TrivialCharsetEncoder UTF8CharsetEncoder; -#else /** * Converts a LogString to UTF-8. */ @@ -354,28 +408,34 @@ class UTF8CharsetEncoder : public CharsetEncoder virtual log4cxx_status_t encode(const LogString& in, LogString::const_iterator& iter, - ByteBuffer& out) + ByteBuffer& out) override { while (iter != in.end() && out.remaining() >= 8) { - unsigned int sv = Transcoder::decode(in, iter); - - if (sv == 0xFFFF) - { - return APR_BADARG; - } - + auto sv = Transcoder::getCodePoint(in, iter); Transcoder::encodeUTF8(sv, out); } return APR_SUCCESS; } + /** + * Add onto \c out an encoded equivalent of \c codePoint. + */ + log4cxx_status_t encode(unsigned int codePoint, ByteBuffer& out) LOG4CXX_16_VIRTUAL_SPECIFIER + { + apr_status_t result = APR_SUCCESS; + if (codePoint <= 0x10FFFF) + Transcoder::encodeUTF8(codePoint, out); + else + result = APR_BADARG; + return result; + } + private: UTF8CharsetEncoder(const UTF8CharsetEncoder&); UTF8CharsetEncoder& operator=(const UTF8CharsetEncoder&); }; -#endif /** * Encodes a LogString to UTF16-BE. @@ -389,23 +449,30 @@ class UTF16BECharsetEncoder : public CharsetEncoder virtual log4cxx_status_t encode(const LogString& in, LogString::const_iterator& iter, - ByteBuffer& out) + ByteBuffer& out) override { while (iter != in.end() && out.remaining() >= 4) { - unsigned int sv = Transcoder::decode(in, iter); - - if (sv == 0xFFFF) - { - return APR_BADARG; - } - + auto sv = Transcoder::getCodePoint(in, iter); Transcoder::encodeUTF16BE(sv, out); } return APR_SUCCESS; } + /** + * Add onto \c out an encoded equivalent of \c codePoint. + */ + log4cxx_status_t encode(unsigned int codePoint, ByteBuffer& out) LOG4CXX_16_VIRTUAL_SPECIFIER + { + apr_status_t result = APR_SUCCESS; + if (codePoint <= 0x10FFFF) + Transcoder::encodeUTF16BE(codePoint, out); + else + result = APR_BADARG; + return result; + } + private: UTF16BECharsetEncoder(const UTF16BECharsetEncoder&); UTF16BECharsetEncoder& operator=(const UTF16BECharsetEncoder&); @@ -424,22 +491,29 @@ class UTF16LECharsetEncoder : public CharsetEncoder virtual log4cxx_status_t encode(const LogString& in, LogString::const_iterator& iter, - ByteBuffer& out) + ByteBuffer& out) override { while (iter != in.end() && out.remaining() >= 4) { - unsigned int sv = Transcoder::decode(in, iter); - - if (sv == 0xFFFF) - { - return APR_BADARG; - } - + auto sv = Transcoder::getCodePoint(in, iter); Transcoder::encodeUTF16LE(sv, out); } return APR_SUCCESS; } + + /** + * Add onto \c out an encoded equivalent of \c codePoint. + */ + log4cxx_status_t encode(unsigned int codePoint, ByteBuffer& out) LOG4CXX_16_VIRTUAL_SPECIFIER + { + apr_status_t result = APR_SUCCESS; + if (codePoint <= 0x10FFFF) + Transcoder::encodeUTF16LE(codePoint, out); + else + result = APR_BADARG; + return result; + } private: UTF16LECharsetEncoder(const UTF16LECharsetEncoder&); UTF16LECharsetEncoder& operator=(const UTF16LECharsetEncoder&); @@ -479,10 +553,7 @@ class LocaleCharsetEncoder : public CharsetEncoder // Encode characters that may require multiple bytes while (nextCodePoint != in.end() && byteCount < availableByteCount && MB_CUR_MAX <= (availableByteCount - byteCount)) { - LogString::const_iterator lastCodePoint = nextCodePoint; - auto ch = Transcoder::decode(in, nextCodePoint); - if (nextCodePoint == lastCodePoint) // invalid input sequence? - nextCodePoint = in.end(); + auto ch = Transcoder::getCodePoint(in, nextCodePoint); auto n = std::wcrtomb(current, ch, &this->state); if (static_cast<std::size_t>(-1) == n) // not a valid wide character? { @@ -496,6 +567,25 @@ class LocaleCharsetEncoder : public CharsetEncoder return result; } + /** + * Add onto \c out an encoded equivalent of \c codePoint. + */ + log4cxx_status_t encode(unsigned int codePoint, ByteBuffer& out) LOG4CXX_16_VIRTUAL_SPECIFIER + { + apr_status_t result = APR_SUCCESS; + if (MB_CUR_MAX <= out.remaining()) + { + auto n = std::wcrtomb(out.current(), codePoint, &this->state); + if (static_cast<std::size_t>(-1) == n) // not a valid wide character? + result = APR_BADARG; + else + out.increment_position(n); + } + else + result = APR_BADARG; + return result; + } + private: std::mbstate_t state; }; @@ -535,7 +625,11 @@ CharsetEncoderPtr CharsetEncoder::getDefaultEncoder() CharsetEncoder* CharsetEncoder::createDefaultEncoder() { #if LOG4CXX_CHARSET_UTF8 +#if LOG4CXX_LOGCHAR_IS_UTF8 + return new TrivialCharsetEncoder(); +#else return new UTF8CharsetEncoder(); +#endif #elif LOG4CXX_CHARSET_ISO88591 return new ISOLatinCharsetEncoder(); #elif LOG4CXX_CHARSET_USASCII @@ -608,7 +702,6 @@ void CharsetEncoder::flush(ByteBuffer& /* out */ ) { } - void CharsetEncoder::encode(CharsetEncoderPtr& enc, const LogString& src, LogString::const_iterator& iter, @@ -633,7 +726,32 @@ void CharsetEncoder::encode(CharsetEncoderPtr& enc, #else #error logchar is unrecognized #endif - dst.put(Transcoder::LOSSCHAR); +#if 15 < LOG4CXX_ABI_VERSION + enc->encode(Transcoder::LOSSCHAR, dst); +#else // LOG4CXX_ABI_VERSION <= 15 + if (auto p = dynamic_cast<TrivialCharsetEncoder*>(enc.get())) + p->encode(Transcoder::LOSSCHAR, dst); + else if (auto p = dynamic_cast<UTF8CharsetEncoder*>(enc.get())) + p->encode(Transcoder::LOSSCHAR, dst); + else if (auto p = dynamic_cast<LocaleCharsetEncoder*>(enc.get())) + p->encode(Transcoder::LOSSCHAR, dst); + else if (auto p = dynamic_cast<USASCIICharsetEncoder*>(enc.get())) + p->encode(Transcoder::LOSSCHAR, dst); + else if (auto p = dynamic_cast<ISOLatinCharsetEncoder*>(enc.get())) + p->encode(Transcoder::LOSSCHAR, dst); + else if (auto p = dynamic_cast<UTF16BECharsetEncoder*>(enc.get())) + p->encode(Transcoder::LOSSCHAR, dst); + else if (auto p = dynamic_cast<UTF16LECharsetEncoder*>(enc.get())) + p->encode(Transcoder::LOSSCHAR, dst); +#if LOG4CXX_LOGCHAR_IS_WCHAR && LOG4CXX_HAS_WCSTOMBS + else if (auto p = dynamic_cast<WcstombsCharsetEncoder*>(enc.get())) + p->encode(Transcoder::LOSSCHAR, dst); +#endif // LOG4CXX_LOGCHAR_IS_WCHAR && LOG4CXX_HAS_WCSTOMBS +#if APR_HAS_XLATE + else if (auto p = dynamic_cast<APRCharsetEncoder*>(enc.get())) + p->encode(Transcoder::LOSSCHAR, dst); +#endif // APR_HAS_XLATE +#endif // LOG4CXX_ABI_VERSION <= 15 } } diff --git a/src/main/cpp/jsonlayout.cpp b/src/main/cpp/jsonlayout.cpp index 6f5e1b86..fe663878 100644 --- a/src/main/cpp/jsonlayout.cpp +++ b/src/main/cpp/jsonlayout.cpp @@ -223,27 +223,14 @@ void JSONLayout::appendItem(const LogString& input, LogString& buf) for (auto nextCodePoint = start; input.end() != nextCodePoint; ) { auto lastCodePoint = nextCodePoint; - auto ch = Transcoder::decode(input, nextCodePoint); - if (nextCodePoint == lastCodePoint) // failed to decode input? - { - // Skip the undecodable run and keep escaping the remaining input - // instead of discarding it; the run collapses to one replacement. - for (++nextCodePoint; nextCodePoint != input.end(); ++nextCodePoint) - { - auto probe = nextCodePoint; - Transcoder::decode(input, probe); - if (probe != nextCodePoint) // next unit starts a decodable sequence - break; - } - ch = 0xFFFD; // The Unicode replacement character - } - else if ((0xD800 <= ch && ch <= 0xDFFF) || 0x10FFFF < ch) + auto ch = Transcoder::getCodePoint(input, nextCodePoint); + if ((0xD800 <= ch && ch <= 0xDFFF) || 0x10FFFF < ch) { ch = 0xFFFD; // The Unicode replacement character } else if (0x22 == ch || 0x5c == ch) // double quote or backslash? ; - else if (0x20 <= ch) // not a control character? + else if (0x20 <= ch && 0xFFFD != ch) // not a control character or the replacement character? continue; if (start != lastCodePoint) diff --git a/src/main/cpp/telnetappender.cpp b/src/main/cpp/telnetappender.cpp index 96845a26..45b39815 100644 --- a/src/main/cpp/telnetappender.cpp +++ b/src/main/cpp/telnetappender.cpp @@ -292,21 +292,10 @@ void TelnetAppender::append( LOG4CXX_APPEND_FORMAL_PARAMETERS ) while (msgIter != msg.end()) { - log4cxx_status_t stat = _priv->encoder->encode(msg, msgIter, buf); + CharsetEncoder::encode(_priv->encoder, msg, msgIter, buf); buf.flip(); write(buf); buf.clear(); - - if (CharsetEncoder::isError(stat)) - { - LogString unrepresented(1, 0x3F /* '?' */); - LogString::const_iterator unrepresentedIter(unrepresented.begin()); - stat = _priv->encoder->encode(unrepresented, unrepresentedIter, buf); - buf.flip(); - write(buf); - buf.clear(); - msgIter++; - } } } } diff --git a/src/main/cpp/transcoder.cpp b/src/main/cpp/transcoder.cpp index 12a0c568..35aee43f 100644 --- a/src/main/cpp/transcoder.cpp +++ b/src/main/cpp/transcoder.cpp @@ -46,27 +46,8 @@ void Transcoder::decodeUTF8(const std::string& src, LogString& dst) while (iter != src.end()) { - std::string::const_iterator start = iter; - unsigned int sv = decode(src, iter); - - if (sv != 0xFFFF) - { - encode(sv, dst); - } - else - { - dst.append(1, LOSSCHAR); - - // decode() returns 0xFFFF both for a decode error (iter left at - // start) and for a successfully decoded U+FFFF (iter already - // advanced past EF BF BF). Only advance here in the former case, - // otherwise the byte following U+FFFF is skipped and, at end of - // input, iter is pushed past src.end(). - if (iter == start) - { - iter++; - } - } + auto sv = getCodePoint(src, iter); + encode(sv, dst); } } @@ -79,17 +60,8 @@ void Transcoder::encodeUTF8(const LogString& src, std::string& dst) while (iter != src.end()) { - unsigned int sv = decode(src, iter); - - if (sv != 0xFFFF) - { - encode(sv, dst); - } - else - { - dst.append(1, LOSSCHAR); - iter++; - } + unsigned int sv = getCodePoint(src, iter); + encode(sv, dst); } #endif @@ -226,7 +198,6 @@ unsigned int Transcoder::decode(const std::string& src, return result; } - void Transcoder::encode(unsigned int sv, std::string& dst) { char tmp[8]; @@ -234,6 +205,11 @@ void Transcoder::encode(unsigned int sv, std::string& dst) dst.append(tmp, bytes); } +/// Does \c str contain the Unicode replacement character +bool Transcoder::hasReplacementCharacter(const std::string& str) +{ + return str.npos != str.find("\xEF\xBF\xBD"); +} void Transcoder::decode(const std::string& src, LogString& dst) { @@ -265,7 +241,7 @@ void Transcoder::decode(const std::string& src, LogString& dst) if (CharsetDecoder::isError(stat)) { - dst.append(1, LOSSCHAR); + encode(LOSSCHAR, dst); buf.increment_position(1); } } @@ -323,7 +299,7 @@ void Transcoder::encode(const LogString& src, std::string& dst) if (CharsetEncoder::isError(stat)) { - dst.append(1, LOSSCHAR); + encode(LOSSCHAR, dst); iter++; } } @@ -413,21 +389,8 @@ void Transcoder::decode(const std::wstring& src, LogString& dst) while (i != src.end()) { - std::wstring::const_iterator start = i; - unsigned int cp = decode(src, i); - - if (cp != 0xFFFF) - { - encode(cp, dst); - } - else - { - dst.append(1, LOSSCHAR); - if (i == start) - { - i++; - } - } + auto cp = getCodePoint(src, i); + encode(cp, dst); } #endif @@ -441,21 +404,8 @@ void Transcoder::encode(const LogString& src, std::wstring& dst) for (LogString::const_iterator i = src.begin(); i != src.end();) { - LogString::const_iterator start = i; - unsigned int cp = Transcoder::decode(src, i); - - if (cp != 0xFFFF) - { - encode(cp, dst); - } - else - { - dst.append(1, LOSSCHAR); - if (i == start) - { - i++; - } - } + unsigned int cp = getCodePoint(src, i); + encode(cp, dst); } #endif @@ -487,7 +437,6 @@ unsigned int Transcoder::decode(const std::wstring& in, #endif } - void Transcoder::encode(unsigned int sv, std::wstring& dst) { #if defined(__STDC_ISO_10646__) @@ -506,6 +455,11 @@ void Transcoder::encode(unsigned int sv, std::wstring& dst) #endif } +/// Does \c str contain the Unicode replacement character +bool Transcoder::hasReplacementCharacter(const std::wstring& str) +{ + return str.npos != str.find(LOSSCHAR); +} #endif @@ -529,7 +483,7 @@ void Transcoder::decode(const std::basic_string<UniChar>& src, LogString& dst) } else { - dst.append(1, LOSSCHAR); + encode(LOSSCHAR, dst); if (i == start) { i++; @@ -580,6 +534,11 @@ void Transcoder::encode(unsigned int sv, std::basic_string<UniChar>& dst) encodeUTF16(sv, dst); } +/// Does \c str contain the Unicode replacement character +bool Transcoder::hasReplacementCharacter(const std::basic_string<UniChar>& str) +{ + return str.npos != str.find(LOSSCHAR); +} #endif #if LOG4CXX_CFSTRING_API @@ -610,7 +569,7 @@ void Transcoder::decode(const CFStringRef& src, LogString& dst) } else { - dst.append(1, LOSSCHAR); + encode(LOSSCHAR, dst); if (i == start) { i++; @@ -672,7 +631,7 @@ std::string Transcoder::encodeCharsetName(const LogString& val) } else { - out.append(1, LOSSCHAR); + out.append(1, '?'); } } diff --git a/src/main/cpp/transform.cpp b/src/main/cpp/transform.cpp index e5cec926..305b9fb0 100644 --- a/src/main/cpp/transform.cpp +++ b/src/main/cpp/transform.cpp @@ -43,23 +43,11 @@ void appendValidCharacters(LogString& buf, const LogString& input, CharProcessor for (auto nextCodePoint = start; input.end() != nextCodePoint; ) { auto lastCodePoint = nextCodePoint; - auto ch = Transcoder::decode(input, nextCodePoint); - if (nextCodePoint == lastCodePoint) // failed to decode input? - { - // Skip the undecodable run and keep escaping the remaining input - // instead of discarding it; the run collapses to one replacement. - for (++nextCodePoint; nextCodePoint != input.end(); ++nextCodePoint) - { - auto probe = nextCodePoint; - Transcoder::decode(input, probe); - if (probe != nextCodePoint) // next unit starts a decodable sequence - break; - } - } - else if (0xD800 <= ch && ch <= 0xDFFF) + auto ch = Transcoder::getCodePoint(input, nextCodePoint); + if (0xD800 <= ch && ch <= 0xDFFF) { // RFC 3629 §3 explicitly forbids surrogate-half values in UTF-8 - ch = 0xFFFF; + ch = 0xFFFD; } else if (((0x20 <= ch && ch <= 0xD7FF) && specials[0] != ch && @@ -67,7 +55,7 @@ void appendValidCharacters(LogString& buf, const LogString& input, CharProcessor specials[2] != ch && specials[3] != ch) || (0x9 == ch || 0xA == ch || 0xD == ch) || - (0xE000 <= ch && ch <= 0xFFFD) || + (0xE000 <= ch && ch < 0xFFFD) || (0x10000 <= ch && ch <= 0x10FFFF)) { LogString escaped; @@ -104,8 +92,8 @@ void appendValidCharacters(LogString& buf, const LogString& input, CharProcessor buf.append(LOG4CXX_STR(">")); break; - case 0xFFFF: // invalid sequence - Transform::appendCharacterReference(buf, 0xFFFD); // The Unicode replacement character + case 0xFFFD: // The Unicode replacement character + Transform::appendCharacterReference(buf, 0xFFFD); break; default: @@ -147,21 +135,8 @@ void Transform::appendEscapingCDATA( { bool cdataEnd = false; auto lastCodePoint = nextCodePoint; - auto ch = Transcoder::decode(input, nextCodePoint); - if (nextCodePoint == lastCodePoint) // failed to decode input? - { - // Skip the undecodable run and keep escaping the remaining input - // instead of discarding it; the run collapses to one replacement. - for (++nextCodePoint; nextCodePoint != input.end(); ++nextCodePoint) - { - auto probe = nextCodePoint; - Transcoder::decode(input, probe); - if (probe != nextCodePoint) // next unit starts a decodable sequence - break; - } - ch = 0xFFFD; // The Unicode replacement character - } - else if (CDATA_END[0] == ch && input.end() != nextCodePoint) + auto ch = Transcoder::getCodePoint(input, nextCodePoint); + if (CDATA_END[0] == ch && input.end() != nextCodePoint) { lastCodePoint = nextCodePoint; if (CDATA_END[1] != Transcoder::decode(input, nextCodePoint) || @@ -176,7 +151,7 @@ void Transform::appendEscapingCDATA( } else if ((0x20 <= ch && ch <= 0xD7FF) || (0x9 == ch || 0xA == ch || 0xD == ch) || - (0xE000 <= ch && ch <= 0xFFFD) || + (0xE000 <= ch && ch < 0xFFFD) || (0x10000 <= ch && ch <= 0x10FFFF)) { continue; diff --git a/src/main/include/log4cxx/helpers/charsetencoder.h b/src/main/include/log4cxx/helpers/charsetencoder.h index e4c0b348..278d1c07 100644 --- a/src/main/include/log4cxx/helpers/charsetencoder.h +++ b/src/main/include/log4cxx/helpers/charsetencoder.h @@ -97,6 +97,13 @@ class LOG4CXX_EXPORT CharsetEncoder : public Object LogString::const_iterator& iter, ByteBuffer& out) = 0; +#if 15 < LOG4CXX_ABI_VERSION + /** + * Add onto \c out an encoded equivalent of \c codePoint. + * @return APR_SUCCESS unless \c codePoint can not be represented in the encoding. + */ + virtual log4cxx_status_t encode(unsigned int codePoint, ByteBuffer& out) = 0; +#endif /** * Resets any internal state. */ diff --git a/src/main/include/log4cxx/helpers/transcoder.h b/src/main/include/log4cxx/helpers/transcoder.h index 68cf8388..60bb68f3 100644 --- a/src/main/include/log4cxx/helpers/transcoder.h +++ b/src/main/include/log4cxx/helpers/transcoder.h @@ -33,7 +33,38 @@ class Pool; class LOG4CXX_EXPORT Transcoder { public: - + /** + * Increment \c nextCodePoint past one \c str code point. + * @pre \c nextCodePoint is a valid, dereferenceable iterator. + * @pre \c nextCodePoint and the end of \c str are in the same sequence. + * @post \c <code>[old_nextCodePoint = nextCodePoint] (old_nextCodePoint < nextCodePoint)</code> // \c nextCodePoint is always advanced + * @param str contains the code point to which \c nextCodePoint refers. + * @param nextCodePoint the start of the current code point. + * @return the code point value or 0xFFFD and \c iter is advanced past + * the invalid sequence if not a valid sequence. + */ + template <typename T> + static unsigned int getCodePoint(const T& str, T::const_iterator& nextCodePoint) + { + auto lastCodePoint = nextCodePoint; + auto ch = decode(str, nextCodePoint); + if (nextCodePoint == lastCodePoint) // failed to decode input? + { + // Skip the undecodable run and keep escaping the remaining input + // instead of discarding it; the run collapses to one replacement. + for (++nextCodePoint; nextCodePoint != str.end(); ++nextCodePoint) + { + auto probe = nextCodePoint; + decode(str, probe); + if (probe != nextCodePoint) // next unit starts a decodable sequence + break; + } + ch = 0xFFFD; // The Unicode replacement character + } + else if (0xFFFF == ch) + ch = 0xFFFD; + return ch; + } /** * Append the UTF-8 characters in \c src onto \c dst. @@ -71,7 +102,7 @@ class LOG4CXX_EXPORT Transcoder * @pre \c iter and the end of \c str are in the same sequence. * @param str contains the code point to which \c iter refers. * @param iter the start of the current code point. - * @return the code point value or 0xFFFF if not a valid sequence. + * @return if a valid sequence, the code point value; otherwise, 0xFFFF and leave \c iter unchanged. */ static unsigned int decode(const std::string& str, std::string::const_iterator& iter); @@ -81,6 +112,9 @@ class LOG4CXX_EXPORT Transcoder */ static void encode(unsigned int ch, std::string& dst); + /// Does \c str contain the Unicode replacement character + static bool hasReplacementCharacter(const std::string& str); + /** * Append the LogString equivalent of \c src onto \c dst. */ @@ -126,7 +160,7 @@ class LOG4CXX_EXPORT Transcoder * @pre \c iter and the end of \c str are in the same sequence. * @param str contains the code point to which \c iter refers. * @param iter the start of the current code point. - * @return the code point value or 0xFFFF if not a valid sequence. + * @return if a valid sequence, the code point value; otherwise, 0xFFFF and leave \c iter unchanged. */ static unsigned int decode(const std::wstring& str, std::wstring::const_iterator& iter); @@ -136,6 +170,8 @@ class LOG4CXX_EXPORT Transcoder */ static void encode(unsigned int ch, std::wstring& dst); + /// Does \c str contain the Unicode replacement character + static bool hasReplacementCharacter(const std::wstring& str); #endif @@ -155,7 +191,7 @@ class LOG4CXX_EXPORT Transcoder * @pre \c iter and the end of \c str are in the same sequence. * @param str contains the code point to which \c iter refers. * @param iter the start of the current code point. - * @return the code point value or 0xFFFF if not a valid sequence. + * @return if a valid sequence, the code point value; otherwise, 0xFFFF and leave \c iter unchanged. */ static unsigned int decode(const std::basic_string<UniChar>& str, std::basic_string<UniChar>::const_iterator& iter); @@ -165,6 +201,8 @@ class LOG4CXX_EXPORT Transcoder */ static void encode(unsigned int ch, std::basic_string<UniChar>& dst); + /// Does \c str contain the Unicode replacement character + static bool hasReplacementCharacter(const std::basic_string<UniChar>& str); #endif #if LOG4CXX_CFSTRING_API @@ -176,8 +214,7 @@ class LOG4CXX_EXPORT Transcoder static CFStringRef encode(const LogString& src); #endif - enum { LOSSCHAR = 0x3F }; - + enum { LOSSCHAR = 0xFFFD }; // Unicode replacement character /** * The logchar equivalent to \c ch. */ diff --git a/src/test/cpp/helpers/charsetencodertestcase.cpp b/src/test/cpp/helpers/charsetencodertestcase.cpp index 8d1430e4..db67fb44 100644 --- a/src/test/cpp/helpers/charsetencodertestcase.cpp +++ b/src/test/cpp/helpers/charsetencodertestcase.cpp @@ -243,8 +243,8 @@ public: /** * Regression test: write malformed UTF-8 through OutputStreamWriter - * using a non-trivial encoder and assert the replacement character - * is emitted (Transcoder::LOSSCHAR). This is deterministic and does + * using a non-trivial encoder and assert the encoder specific + * replacement character is emitted. This is deterministic and does * not rely on process crash. */ void utf8Recovery() @@ -276,8 +276,7 @@ public: LOGUNIT_ASSERT_EQUAL(1, read); LOGUNIT_ASSERT_EQUAL((size_t)1, buf.position()); - LOGUNIT_ASSERT_EQUAL((unsigned char) Transcoder::LOSSCHAR, - (unsigned char) raw[0]); + LOGUNIT_ASSERT_EQUAL('?', raw[0]); } class ThreadPackage diff --git a/src/test/cpp/helpers/transcodertestcase.cpp b/src/test/cpp/helpers/transcodertestcase.cpp index 18e7070c..1aaa55e2 100644 --- a/src/test/cpp/helpers/transcodertestcase.cpp +++ b/src/test/cpp/helpers/transcodertestcase.cpp @@ -203,14 +203,10 @@ public: void encode3_1() { // Test invalid multibyte string - LogString greeting; - greeting.push_back( logchar(0xff) ); std::wstring encoded; - Transcoder::encode(greeting, encoded); - - std::wstring expected; - expected.push_back( log4cxx::helpers::Transcoder::LOSSCHAR ); - LOGUNIT_ASSERT_EQUAL(encoded, expected ); + Transcoder::encode(0xDfff, encoded); + std::wstring::const_iterator i = encoded.begin(); + LOGUNIT_ASSERT_EQUAL(((unsigned int) 0xFFFF), Transcoder::decode(encoded, i)); } #endif @@ -306,7 +302,7 @@ public: std::string src(1, char(0x80)); LogString out; Transcoder::decodeUTF8(src, out); - LOGUNIT_ASSERT_EQUAL(LogString(1, Transcoder::LOSSCHAR), out); + LOGUNIT_ASSERT(Transcoder::hasReplacementCharacter(out)); } void testDecodeUTF8_3() @@ -314,7 +310,7 @@ public: std::string src("\xC2"); LogString out; Transcoder::decodeUTF8(src, out); - LOGUNIT_ASSERT_EQUAL(LogString(1, Transcoder::LOSSCHAR), out); + LOGUNIT_ASSERT(Transcoder::hasReplacementCharacter(out)); } void testDecodeUTF8_4() @@ -333,8 +329,7 @@ public: * (U+D800..U+DFFF). The three-byte sequences ED A0 80 .. ED BF BF must * not decode to the corresponding surrogate code points: doing so lets * lone surrogates enter LogString and be re-emitted by JSON/XML layouts, - * propagating malformed Unicode past the parsing boundary. Each byte of - * the invalid sequence is replaced with Transcoder::LOSSCHAR. + * propagating malformed Unicode past the parsing boundary. */ void testDecodeUTF8_RejectSurrogate() { @@ -342,12 +337,7 @@ public: std::string src("\xED\xA0\x80"); LogString out; Transcoder::decodeUTF8(src, out); - - LogString expected; - expected.append(1, Transcoder::LOSSCHAR); - expected.append(1, Transcoder::LOSSCHAR); - expected.append(1, Transcoder::LOSSCHAR); - LOGUNIT_ASSERT_EQUAL(expected, out); + LOGUNIT_ASSERT(Transcoder::hasReplacementCharacter(out)); } /** @@ -364,11 +354,7 @@ public: std::string src("\xE0\xA0\x80"); LogString out; Transcoder::decodeUTF8(src, out); - - LogString expected; - Transcoder::encode(0x0800, expected); - LOGUNIT_ASSERT_EQUAL(expected, out); - LOGUNIT_ASSERT(out.find(Transcoder::LOSSCHAR) == LogString::npos); + LOGUNIT_ASSERT(!Transcoder::hasReplacementCharacter(out)); } /** @@ -392,8 +378,7 @@ public: std::string src(c.bytes, c.len); LogString out; Transcoder::decodeUTF8(src, out); - bool hasLoss = out.find(Transcoder::LOSSCHAR) != LogString::npos; - LOGUNIT_ASSERT_EQUAL(c.reject, hasLoss); + LOGUNIT_ASSERT_EQUAL(c.reject, Transcoder::hasReplacementCharacter(out)); } } @@ -412,10 +397,7 @@ public: LogString out; Transcoder::decodeUTF8(src, out); - LogString expected; - for (int i = 0; i < 4; ++i) - expected.append(1, Transcoder::LOSSCHAR); - LOGUNIT_ASSERT_EQUAL(expected, out); + LOGUNIT_ASSERT(Transcoder::hasReplacementCharacter(out)); } /** @@ -438,8 +420,7 @@ public: std::string src(c.bytes, c.len); LogString out; Transcoder::decodeUTF8(src, out); - bool hasLoss = out.find(Transcoder::LOSSCHAR) != LogString::npos; - LOGUNIT_ASSERT_EQUAL(c.reject, hasLoss); + LOGUNIT_ASSERT_EQUAL(c.reject, Transcoder::hasReplacementCharacter(out)); } } @@ -448,8 +429,7 @@ public: * branch masks the lead byte with 0x07, discarding those high bits, so * F8 BF BF BF used to slip past the U+10FFFF bound and decode to U+3FFFF * (and FB/FC likewise to other in-range planes) — the same aliasing - * filter-bypass that the F5..F7 rejection guards against. Each byte of an - * invalid lead sequence must be replaced with Transcoder::LOSSCHAR. + * filter-bypass that the F5..F7 rejection guards against. */ void testDecodeUTF8_RejectInvalidLeadByte() { @@ -466,8 +446,7 @@ public: std::string src(c.bytes, c.len); LogString out; Transcoder::decodeUTF8(src, out); - bool hasLoss = out.find(Transcoder::LOSSCHAR) != LogString::npos; - LOGUNIT_ASSERT_EQUAL(c.reject, hasLoss); + LOGUNIT_ASSERT_EQUAL(c.reject, Transcoder::hasReplacementCharacter(out)); } }
