Bug 151250

Summary:

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

Product:

WebKit

Reporter:

Gyuyoung Kim <gyuyoung.kim>

Component:

WebKit EFL

Assignee:

Gyuyoung Kim <gyuyoung.kim>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, lucas.de.marchi

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

30342, 151340

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Gyuyoung Kim

Reported 2015-11-13 00:51:20 PST

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

Attachments
Patch (2.52 KB, patch) 2015-11-13 00:52 PST, Gyuyoung Kim	no flags	Details Formatted Diff Diff
Patch (3.39 KB, patch) 2015-11-14 07:32 PST, Gyuyoung Kim	no flags	Details Formatted Diff Diff
Patch (2.61 KB, patch) 2015-11-14 18:16 PST, Gyuyoung Kim	no flags	Details Formatted Diff Diff
Patch (3.37 KB, patch) 2015-11-14 19:24 PST, Gyuyoung Kim	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Gyuyoung Kim

Comment 1 2015-11-13 00:52:51 PST

Created attachment 265473 [details] Patch

Darin Adler

Comment 2 2015-11-13 09:27:20 PST

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 3 2015-11-13 09:29:07 PST

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

Comment 4 2015-11-14 07:32:30 PST

Created attachment 265541 [details] Patch

Gyuyoung Kim

Comment 5 2015-11-14 07:33:51 PST

(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 6 2015-11-14 16:51:16 PST

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

Comment 7 2015-11-14 18:16:54 PST

Created attachment 265548 [details] Patch

Darin Adler

Comment 8 2015-11-14 18:46:34 PST

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

Comment 9 2015-11-14 19:24:10 PST

Created attachment 265549 [details] Patch

Gyuyoung Kim

Comment 10 2015-11-14 19:25:39 PST

(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 11 2015-11-14 23:56:58 PST

Comment on attachment 265549 [details] Patch Clearing flags on attachment: 265549 Committed r192459: <http://trac.webkit.org/changeset/192459>

WebKit Commit Bot

Comment 12 2015-11-14 23:57:03 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.