Added too-long and too-big character conversion errors

With this commit, when casting to a single character, as opposed to a
C-style string, we make sure the input wasn't a multi-character string
or a single character with codepoint too large for the character type.

This also changes the character cast op to CharT instead of CharT& (we
need to be able to return a temporary decoded char value, but also
because there's little gained by bothering with an lvalue return here).

Finally it changes the char caster to 'has-a-string-caster' instead of
'is-a-string-caster' because, with the cast_op change above, there's
nothing at all gained from inheritance.  This also lets us remove the
`success` from the string caster (which was only there for the char
caster) into the char caster itself.  (I also renamed it to 'none' and
inverted its value to better reflect its purpose).  The None -> nullptr
loading also now takes place only under a `convert = true` load pass.
Although it's unlikely that a function taking a char also has overloads
that can take a None, it seems marginally more correct to treat it as a
conversion.

This commit simplifies the size assumptions about character sizes with
static_asserts to back them up.

Author: jagerman

include/pybind11/cast.h

@@ -624,33 +624,38 @@ template <> class type_caster<bool> {
     PYBIND11_TYPE_CASTER(bool, _("bool"));
 };
 
-// Helper class for UTF-{8,16,32} strings:
+// Helper class for UTF-{8,16,32} C++ stl strings:
 template <typename CharT, class Traits, class Allocator>
 struct type_caster<std::basic_string<CharT, Traits, Allocator>, enable_if_t<is_std_char_type<CharT>::value>> {
-    static constexpr unsigned int UTF_N =
-        std::is_same<CharT, char>::value     ? 8 :
-        std::is_same<CharT, char16_t>::value ? 16 :
-        std::is_same<CharT, char32_t>::value ? 32 :
-        (sizeof(CharT) == 2 ? 16 : 32); /* std::wstring is UTF-16 on Windows, UTF-32 everywhere else */
-
+    // Simplify life by being able to assume standard char sizes (the standard only guarantees
+    // minimums), but Python requires exact sizes
+    static_assert(!std::is_same<CharT, char>::value || sizeof(CharT) == 1, "Unsupported char size != 1");
+    static_assert(!std::is_same<CharT, char16_t>::value || sizeof(CharT) == 2, "Unsupported char16_t size != 2");
+    static_assert(!std::is_same<CharT, char32_t>::value || sizeof(CharT) == 4, "Unsupported char32_t size != 4");
+    // wchar_t can be either 16 bits (Windows) or 32 (everywhere else)
+    static_assert(!std::is_same<CharT, wchar_t>::value || sizeof(CharT) == 2 || sizeof(CharT) == 4,
+            "Unsupported wchar_t size != 2/4");
+    static constexpr size_t UTF_N = 8 * sizeof(CharT);
     static constexpr const char *encoding = UTF_N == 8 ? "utf8" : UTF_N == 16 ? "utf16" : "utf32";
 
-    // C++ only requires char/char16_t/char32_t to be at least 8/16/32 bits, but Python's encoding
-    // assumes exactly 1/2/4 bytes:
-    static_assert(sizeof(CharT) == UTF_N / 8,
-            "Internal error: string type_caster requires 1/2/4-sized character types");
-
     using StringType = std::basic_string<CharT, Traits, Allocator>;
 
     bool load(handle src, bool) {
+#if PY_VERSION_MAJOR < 3
         object temp;
+#endif
         handle load_src = src;
         if (!src) {
             return false;
         } else if (!PyUnicode_Check(load_src.ptr())) {
+#if PY_VERSION_MAJOR >= 3
+            return false;
+            // The below is a guaranteed failure in Python 3 when PyUnicode_Check returns false
+#else
             temp = reinterpret_steal<object>(PyUnicode_FromObject(load_src.ptr()));
             if (!temp) { PyErr_Clear(); return false; }
             load_src = temp;
+#endif
         }
 
         object utfNbytes = reinterpret_steal<object>(PyUnicode_AsEncodedString(
@@ -661,7 +666,6 @@ struct type_caster<std::basic_string<CharT, Traits, Allocator>, enable_if_t<is_s
         size_t length = (size_t) PYBIND11_BYTES_SIZE(utfNbytes.ptr()) / sizeof(CharT);
         if (UTF_N > 8) { buffer++; length--; } // Skip BOM for UTF-16/32
         value = StringType(buffer, length);
-        success = true;
         return true;
     }
 
@@ -674,24 +678,29 @@ struct type_caster<std::basic_string<CharT, Traits, Allocator>, enable_if_t<is_s
     }
 
     PYBIND11_TYPE_CASTER(StringType, _(PYBIND11_STRING_NAME));
-protected:
-    bool success = false;
 };
 
-template <typename CharT> struct type_caster<CharT, enable_if_t<is_std_char_type<CharT>::value>>
-: type_caster<std::basic_string<CharT>> {
+// Type caster for C-style strings.  We basically use a std::string type caster, but also add the
+// ability to use None as a nullptr char* (which the string caster doesn't allow).
+template <typename CharT> struct type_caster<CharT, enable_if_t<is_std_char_type<CharT>::value>> {
     using StringType = std::basic_string<CharT>;
     using StringCaster = type_caster<StringType>;
-    using StringCaster::success;
-    using StringCaster::value;
+    StringCaster str_caster;
+    bool none = false;
 public:
     bool load(handle src, bool convert) {
-        if (src.is_none()) return true;
-        return StringCaster::load(src, convert);
+        if (!src) return false;
+        if (src.is_none()) {
+            // Defer accepting None to other overloads (if we aren't in convert mode):
+            if (!convert) return false;
+            none = true;
+            return true;
+        }
+        return str_caster.load(src, convert);
     }
 
     static handle cast(const CharT *src, return_value_policy policy, handle parent) {
-        if (src == nullptr) return none().inc_ref();
+        if (src == nullptr) return pybind11::none().inc_ref();
         return StringCaster::cast(StringType(src), policy, parent);
     }
 
@@ -704,10 +713,55 @@ template <typename CharT> struct type_caster<CharT, enable_if_t<is_std_char_type
         return StringCaster::cast(StringType(1, src), policy, parent);
     }
 
-    operator CharT*() { return success ? const_cast<CharT *>(value.c_str()) : nullptr; }
-    operator CharT&() { return value[0]; }
+    operator CharT*() { return none ? nullptr : const_cast<CharT *>(static_cast<StringType &>(str_caster).c_str()); }
+    operator CharT() {
+        if (none)
+            throw value_error("Cannot convert None to a character");
+
+        auto &value = static_cast<StringType &>(str_caster);
+        size_t str_len = value.size();
+        if (str_len == 0)
+            throw value_error("Cannot convert empty string to a character");
+
+        // If we're in UTF-8 mode, we have two possible failures: one for a unicode character that
+        // is too high, and one for multiple unicode characters (caught later), so we need to figure
+        // out how long the first encoded character is in bytes to distinguish between these two
+        // errors.  We also allow want to allow unicode characters U+0080 through U+00FF, as those
+        // can fit into a single char value.
+        if (StringCaster::UTF_N == 8 && str_len > 1 && str_len <= 4) {
+            unsigned char v0 = static_cast<unsigned char>(value[0]);
+            size_t char0_bytes = !(v0 & 0x80) ? 1 : // low bits only: 0-127
+                (v0 & 0xE0) == 0xC0 ? 2 : // 0b110xxxxx - start of 2-byte sequence
+                (v0 & 0xF0) == 0xE0 ? 3 : // 0b1110xxxx - start of 3-byte sequence
+                4; // 0b11110xxx - start of 4-byte sequence
+
+            if (char0_bytes == str_len) {
+                // If we have a 128-255 value, we can decode it into a single char:
+                if (char0_bytes == 2 && (v0 & 0xFC) == 0xC0) { // 0x110000xx 0x10xxxxxx
+                    return static_cast<char>(((v0 & 3) << 6) + (static_cast<unsigned char>(value[1]) & 0x3F));
+                }
+                // Otherwise we have a single character, but it's > U+00FF
+                throw value_error("Character code point not in range(0x100)");
+            }
+        }
+
+        // UTF-16 is much easier: we can only have a surrogate pair for values above U+FFFF, thus a
+        // surrogate pair with total length 2 instantly indicates a range error (but not a "your
+        // string was too long" error).
+        else if (StringCaster::UTF_N == 16 && str_len == 2) {
+            char16_t v0 = static_cast<char16_t>(value[0]);
+            if (v0 >= 0xD800 && v0 < 0xE000)
+                throw value_error("Character code point not in range(0x10000)");
+        }
+
+        if (str_len != 1)
+            throw value_error("Expected a character, but multi-character string found");
+
+        return value[0];
+    }
 
     static PYBIND11_DESCR name() { return type_descr(_(PYBIND11_STRING_NAME)); }
+    template <typename _T> using cast_op_type = typename std::remove_reference<pybind11::detail::cast_op_type<_T>>::type;
 };
 
 template <typename T1, typename T2> class type_caster<std::pair<T1, T2>> {

tests/test_python_types.cpp

@@ -458,6 +458,12 @@ test_initializer python_types([](py::module &m) {
     m.def("u16_ibang", [=]() -> char16_t { return ib16; });
     m.def("u32_mathbfA", [=]() -> char32_t { return mathbfA32; });
     m.def("wchar_heart", []() -> wchar_t { return 0x2665; });
+
+    m.attr("wchar_size") = py::cast(sizeof(wchar_t));
+    m.def("ord_char", [](char c) { return 0 + static_cast<unsigned char>(c); });
+    m.def("ord_char16", [](char16_t c) { return 0 + c; });
+    m.def("ord_char32", [](char32_t c) { return 0 + c; });
+    m.def("ord_wchar", [](wchar_t c) { return 0 + c; });
 });
 
 #if defined(_MSC_VER)

tests/test_python_types.py

@@ -446,3 +446,57 @@ def test_unicode_conversion():
     assert u16_ibang() == u'‽'
     assert u32_mathbfA() == u'𝐀'
     assert wchar_heart() == u'♥'
+
+
+def test_single_char_arguments():
+    """Tests failures for passing invalid inputs to char-accepting functions"""
+    from pybind11_tests import ord_char, ord_char16, ord_char32, ord_wchar, wchar_size
+
+    def toobig_message(r): return "Character code point not in range({0:#x})".format(r)
+    toolong_message = "Expected a character, but multi-character string found"
+
+    assert ord_char(u'a') == 0x61  # simple ASCII
+    assert ord_char(u'é') == 0xE9  # requires 2 bytes in utf-8, but can be stuffed in a char
+    with pytest.raises(ValueError) as excinfo:
+        assert ord_char(u'Ā') == 0x100  # requires 2 bytes, doesn't fit in a char
+    assert str(excinfo.value) == toobig_message(0x100)
+    with pytest.raises(ValueError) as excinfo:
+        assert ord_char(u'ab')
+    assert str(excinfo.value) == toolong_message
+
+    assert ord_char16(u'a') == 0x61
+    assert ord_char16(u'é') == 0xE9
+    assert ord_char16(u'Ā') == 0x100
+    assert ord_char16(u'‽') == 0x203d
+    assert ord_char16(u'♥') == 0x2665
+    with pytest.raises(ValueError) as excinfo:
+        assert ord_char16(u'🎂') == 0x1F382  # requires surrogate pair
+    assert str(excinfo.value) == toobig_message(0x10000)
+    with pytest.raises(ValueError) as excinfo:
+        assert ord_char16(u'aa')
+    assert str(excinfo.value) == toolong_message
+
+    assert ord_char32(u'a') == 0x61
+    assert ord_char32(u'é') == 0xE9
+    assert ord_char32(u'Ā') == 0x100
+    assert ord_char32(u'‽') == 0x203d
+    assert ord_char32(u'♥') == 0x2665
+    assert ord_char32(u'🎂') == 0x1F382
+    with pytest.raises(ValueError) as excinfo:
+        assert ord_char32(u'aa')
+    assert str(excinfo.value) == toolong_message
+
+    assert ord_wchar(u'a') == 0x61
+    assert ord_wchar(u'é') == 0xE9
+    assert ord_wchar(u'Ā') == 0x100
+    assert ord_wchar(u'‽') == 0x203d
+    assert ord_wchar(u'♥') == 0x2665
+    if wchar_size == 2:
+        with pytest.raises(ValueError) as excinfo:
+            assert ord_wchar(u'🎂') == 0x1F382  # requires surrogate pair
+        assert str(excinfo.value) == toobig_message(0x10000)
+    else:
+        assert ord_wchar(u'🎂') == 0x1F382
+    with pytest.raises(ValueError) as excinfo:
+        assert ord_wchar(u'aa')
+    assert str(excinfo.value) == toolong_message