Pass lobby certificate status as a separate value and allow Xmpp error strings to become static
ClosedPublic
Actions

Authored by elexis on Sep 9 2019, 6:14 PM.

Details

Reviewers: None
Commits: rP22880: Pass XmppClient CertificateErrorToString independent of ConnectionErrorToString…

Summary

With rP22856 / D2264 it became possible to pass arbitrary JS object properties to GUI messages, so the certificate information can be passed comfortably and independently.
This has two advantages for the certificate error string:

All XmppClient functions that take a gloox enum value and create a string from it become independent of the XmppClient instance, allowing this to become static, which allows this function to be used by ScriptInterface::ToJSVal<glooxenum> for instance.
The JS GUI can decide to display the certificate error differently, for example in a different textfield or verbose popup or whatever.

Test Plan

Notice that the function returns emptystring for certificates that are Ok, and returns a string starting with \n otherwise.
Witness that the string concatenation is the same as before, just in JS instead of C++.
Notice that using these functions without an instance allows ToJSVal to use it without refering to g_XmppClient (and without having to add something to IXmppClient).
Notice that utf8 support is currently broken and that this is actually taken from the diff to fix it in a better way than the banal way: D2271.

One can test individual strings to be displayed by changing the according line in XmppClient::onDisconnect to

"certificate_status", CertStatusToString(/*m_certStatus*/static_cast<gloox::CertStatus>(0)));

Observe the certificate status is always Ok i.e. translated to empty string unless ConnTlsFailed.

Hence check the specs on onTLSConnect:

/**
 * This function is called when the TLS handshake completed correctly. The return
 * value is used to determine whether or not the client accepted the server's
 * certificate. If @b false is returned the connection is closed.
 * @param info Information on the server's certificate.
 * @return @b True if the certificate seems trustworthy, @b false otherwise.
 */
bool notifyOnTLSConnect( const CertInfo& info );

Which proves that statement in theory.
m_certStatus is only set in onTLSConnect, so its m_certStatus = 0 by default.
Actually it is m_certStatus = undefined / uninitialized by default, great, that is fixed hereby now.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Sep 9 2019, 6:14 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/113/display/redirect

Harbormaster failed remote builds in B9287: Diff 9684!Sep 9 2019, 6:15 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/622/display/redirect

elexis edited the test plan for this revision. (Show Details)Sep 9 2019, 8:54 PM

Missing file, also init m_CertStatus

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/119/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lobby/XmppClient.h
|  24| 
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceStunClient{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  47|  47|  * The playerlist will be assembled using these values.
|  48|  48|  */
|  49|  49| var g_PlayerStatuses = {
|  50|    |-	"available": { "color": "0 219 0",     "status": translate("Online") },
|    |  50|+	"available": { "color": "0 219 0", "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'away'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  48|  48|  */
|  49|  49| var g_PlayerStatuses = {
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|    |-	"away":      { "color": "229 76 13",   "status": translate("Away") },
|    |  51|+	"away": { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  48|  48|  */
|  49|  49| var g_PlayerStatuses = {
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|    |-	"away":      { "color": "229 76 13",   "status": translate("Away") },
|    |  51|+	"away":      { "color": "229 76 13", "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'playing'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  49|  49| var g_PlayerStatuses = {
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|    |-	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|    |  52|+	"playing": { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  49|  49| var g_PlayerStatuses = {
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|    |-	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|    |  52|+	"playing":   { "color": "200 0 0", "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'offline'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|    |-	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    |  53|+	"offline": { "color": "0 0 0",       "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|  56|  56| 
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '"status"'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  50|  50| 	"available": { "color": "0 219 0",     "status": translate("Online") },
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|    |-	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|    |  53|+	"offline":   { "color": "0 0 0", "status": translate("Offline") },
|  54|  54| 	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|  56|  56| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'unknown'.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|  51|  51| 	"away":      { "color": "229 76 13",   "status": translate("Away") },
|  52|  52| 	"playing":   { "color": "200 0 0",     "status": translate("Busy") },
|  53|  53| 	"offline":   { "color": "0 0 0",       "status": translate("Offline") },
|  54|    |-	"unknown":   { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|    |  54|+	"unknown": { "color": "178 178 178", "status": translateWithContext("lobby presence", "Unknown") }
|  55|  55| };
|  56|  56| 
|  57|  57| var g_RoleNames = {
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 210| 210| 					me ?
| 211| 211| 						translate("You have been muted.") :
| 212| 212| 						translate("%(nick)s has been muted.") :
| 213|    |-				msg.newrole == "moderator" ?
|    | 213|+					msg.newrole == "moderator" ?
| 214| 214| 					me ?
| 215| 215| 						translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 211| 211| 						translate("You have been muted.") :
| 212| 212| 						translate("%(nick)s has been muted.") :
| 213| 213| 				msg.newrole == "moderator" ?
| 214|    |-					me ?
|    | 214|+						me ?
| 215| 215| 						translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 212| 212| 						translate("%(nick)s has been muted.") :
| 213| 213| 				msg.newrole == "moderator" ?
| 214| 214| 					me ?
| 215|    |-						translate("You are now a moderator.") :
|    | 215|+							translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
| 218| 218| 					me ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 213| 213| 				msg.newrole == "moderator" ?
| 214| 214| 					me ?
| 215| 215| 						translate("You are now a moderator.") :
| 216|    |-						translate("%(nick)s is now a moderator.") :
|    | 216|+							translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
| 218| 218| 					me ?
| 219| 219| 						translate("You have been unmuted.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 214| 214| 					me ?
| 215| 215| 						translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217|    |-				msg.oldrole == "visitor" ?
|    | 217|+						msg.oldrole == "visitor" ?
| 218| 218| 					me ?
| 219| 219| 						translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 215| 215| 						translate("You are now a moderator.") :
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
| 218|    |-					me ?
|    | 218|+							me ?
| 219| 219| 						translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 216| 216| 						translate("%(nick)s is now a moderator.") :
| 217| 217| 				msg.oldrole == "visitor" ?
| 218| 218| 					me ?
| 219|    |-						translate("You have been unmuted.") :
|    | 219|+								translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
| 222| 222| 						translate("You are not a moderator anymore.") :
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 217| 217| 				msg.oldrole == "visitor" ?
| 218| 218| 					me ?
| 219| 219| 						translate("You have been unmuted.") :
| 220|    |-						translate("%(nick)s has been unmuted.") :
|    | 220|+								translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
| 222| 222| 						translate("You are not a moderator anymore.") :
| 223| 223| 						translate("%(nick)s is not a moderator anymore.");
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 218| 218| 					me ?
| 219| 219| 						translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221|    |-					me ?
|    | 221|+							me ?
| 222| 222| 						translate("You are not a moderator anymore.") :
| 223| 223| 						translate("%(nick)s is not a moderator anymore.");
| 224| 224| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 219| 219| 						translate("You have been unmuted.") :
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
| 222|    |-						translate("You are not a moderator anymore.") :
|    | 222|+								translate("You are not a moderator anymore.") :
| 223| 223| 						translate("%(nick)s is not a moderator anymore.");
| 224| 224| 
| 225| 225| 			addChatMessage({
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/lobby/lobby.js
| 220| 220| 						translate("%(nick)s has been unmuted.") :
| 221| 221| 					me ?
| 222| 222| 						translate("You are not a moderator anymore.") :
| 223|    |-						translate("%(nick)s is not a moderator anymore.");
|    | 223|+								translate("%(nick)s is not a moderator anymore.");
| 224| 224| 
| 225| 225| 			addChatMessage({
| 226| 226| 				"text": "/special " + sprintf(txt, { "nick": msg.nick }),

binaries/data/mods/public/gui/lobby/lobby.js
|1012| »   »   switch·(sortBy)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/628/display/redirect

Harbormaster completed remote builds in B9296: Diff 9693.Sep 9 2019, 9:02 PM

This revision was not accepted when it landed; it landed in state Needs Review.Sep 9 2019, 11:27 PM

Closed by commit rP22880: Pass XmppClient CertificateErrorToString independent of ConnectionErrorToString… (authored by elexis). · Explain Why

This revision was automatically updated to reflect the committed changes.

elexis added a commit: rP22880: Pass XmppClient CertificateErrorToString independent of ConnectionErrorToString….

Owners added a subscriber: Restricted Owners Package.Sep 9 2019, 11:27 PM

elexis mentioned this in rP22891: Fix missing wstring_from_utf8 for multi-user-chat messages and translated….Sep 12 2019, 7:24 PM