Bug 151250 – [EFL][GTK] Remove use of String::format() in versionForUAString()

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Gyuyoung Kim]

As String::format() will be deprecated due to security problem, reimplement versionForUAString() using a macro.

[Gyuyoung Kim]

Created attachment 265473 [details]
Patch

[Darin Adler]

Comment on attachment 265473 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=265473&action=review

> Source/WebCore/platform/efl/UserAgentEfl.cpp:64
>  static const String& versionForUAString()
>  {
> -    static NeverDestroyed<String> version(String::format("%i.%i", WEBKIT_MAJOR_VERSION, WEBKIT_MINOR_VERSION));
> +#define MAKE_VERSION(major, minor) #major "." #minor
> +    const String& version = MAKE_VERSION(WEBKIT_MAJOR_VERSION, WEBKIT_MINOR_VERSION);
>      return version;
> +#undef MAKE_VERSION
>  }

This is wrong. It will return a reference to an already-destroyed string object.

[Darin Adler]

Comment on attachment 265473 [details]
Patch

Here’s how to do it:

    static const char* versionForUAString()
    {
#define MAKE_VERSION(major, minor) #major "." #minor
    return MAKE_VERSION(USER_AGENT_GTK_MAJOR_VERSION, USER_AGENT_GTK_MINOR_VERSION);
#undef MAKE_VERSION
    }

In the EFL case we also need to change the type from const String& to const char*.

[Gyuyoung Kim]

Created attachment 265541 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #3)
> Comment on attachment 265473 [details]
> Patch
> 
> Here’s how to do it:
> 
>     static const char* versionForUAString()
>     {
> #define MAKE_VERSION(major, minor) #major "." #minor
>     return MAKE_VERSION(USER_AGENT_GTK_MAJOR_VERSION,
> USER_AGENT_GTK_MINOR_VERSION);
> #undef MAKE_VERSION
>     }
> 
> In the EFL case we also need to change the type from const String& to const
> char*.

I see. I update the patch as well as platformVersionForUAString() is modified.

[Darin Adler]

Comment on attachment 265541 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=265541&action=review

> Source/WebCore/platform/gtk/UserAgentGtk.cpp:106
> +    return MAKE_VERSION(name.sysname, name.machine);

This won’t work. I believe this will yield the string:

"name.sysnamename.machine"

This change should not be made. Testing would have shown this is incorrect.

> Source/WebCore/platform/gtk/UserAgentGtk.cpp:109
> +    return MAKE_VERSION(cpuDescriptionForUAString(), "Mac OS X");

This won’t work. I believe this will yield the string:

"cpuDescriptionForUAString()Mac OS X"

This change should not be made. Testing would have shown this is incorrect.

[Gyuyoung Kim]

Created attachment 265548 [details]
Patch

[Darin Adler]

Comment on attachment 265548 [details]
Patch

standardUserAgent in UserAgentEfl.cpp can still be improved so it doesn’t convert the const char* returned by versionForUAString to a String for the local variable version every time it’s called

[Gyuyoung Kim]

Created attachment 265549 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #8)
> Comment on attachment 265548 [details]
> Patch
> 
> standardUserAgent in UserAgentEfl.cpp can still be improved so it doesn’t
> convert the const char* returned by versionForUAString to a String for the
> local variable version every time it’s called

const char* is converted to a String using ASCIILiteral in new patch.

[WebKit Commit Bot]

Comment on attachment 265549 [details]
Patch

Clearing flags on attachment: 265549

Committed r192459: <http://trac.webkit.org/changeset/192459>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.