Bug 17432 - String(const char*) constructor is harmful
Summary: String(const char*) constructor is harmful
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on: 20313
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-18 18:51 PST by Alp Toker
Modified: 2010-10-21 17:09 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alp Toker 2008-02-18 18:51:10 PST
String::String(const char*) and the related implicit conversions and operators from const char* are the biggest single source of programming errors in the GTK+ port.

The problem: developers often pass (UTF-8 encoded) const gchar* directly into WebCore functions that accept a String. It's difficult to catch these bugs at review time unless the patch comes with sufficient context to see what's going on, which is often not the case. Once landed, the issue won't be apparent until users try to enter non-ASCII text, so the failure mode is obscure.

eg.

>+    Credential credential = Credential(challengePrivate->user,
>+                                       challengePrivate->password,
...

The code looks correct and will work fine since UTF-8 is often valid ASCII until someone tries using a non-ASCII username or password.

I try to catch and correct code to use String::fromUTF8() but this gets missed very often.

We should probably disable the implicit conversions to make people use String::fromUTF8(), at least in any WebCore and WebKit files that use GLib.

Thoughts?
Comment 1 Jan Alonzo 2008-04-13 21:15:18 PDT
Does this we mean we have to check the string if it's a valid utf8 before using String::fromUTF8? How do we disable the implicit convertions?
Comment 2 Martin Robinson 2010-10-21 17:09:30 PDT
Agreed, but we should open a bug for these issues as we find them. Closing for now.