Support mtasa:// protocol connect args #3211

Lpsd · 2023-10-15T06:54:02Z

Changes

Allows arguments to be supplied to the mtasa:// protocol and received by the server in Lua via onPlayerConnect event parameters.
Adds an extra argument to redirectPlayer which allows you to pass a table to achieve the same result (all keys, values are automatically converted to strings, in order to form the 'argument path').
A fix for some of the SharedUtil tests (Use MTA temp path for client tests #3209) has been modified and merged into this PR.

Additional Info

Total length of arguments is limited to 1024 including / delimiter per key/value pair (arguments are treated as path string for 99% of lifetime)

Subject to change for typical url parameters e.g ?a=1&b=2

This feature is useful for the Discord RPC functionality, servers can create a party system using this or give rewards for joining via Discord (of course this can extend to other platforms).

Example

Have serverside script running:

SERVER_IP, SERVER_PORT, SERVER_PASS = "localhost", 22003, ""
DEFAULT_ARGS = {["foo"] = "abc", ["bar"] = "def"}

local storedArgs

function connect(nick, ip, username, serial, version, versionString, args)
    storedArgs = args
    iprint(args)
end
addEventHandler("onPlayerConnect", root, connect)

function redirect(player)
    redirectPlayer(player, SERVER_IP, SERVER_PORT, SERVER_PASS, (type(storedArgs) == "table") and storedArgs or DEFAULT_ARGS)
end
addCommandHandler("redirect", redirect)

Action	Result
Player connects with `mtasa://123.456.78.9:22003/foo/123/bar/456/baz/789`
Player uses `redirect` command	Same as above

Notes

To test mtasa:// protocol with debug build, edit registry Computer\HKEY_CLASSES_ROOT\mtasa\shell\open\command and set to your mtasa build path e.g "C:\mtasa-blue\Bin\Multi Theft Auto_d.exe"%1

To-do

Handle / in user provided keys/values, either remove character or URL encode? (/ is our delimiter so would affect converting to pairs)
- / will be removed from user provided args
Fix argument parser issue when using std::unordered_map<type, type> for Lua args
- See Fix std::unordered_map<std::string, std::string> parsing #3246
Determine whether keys/values (protocol parameters) should be changed to ?a=1&b=2 format

The method used for URL parameters (key/value pairs delimited by slash) should be changed to use the standard URI parameter format `?foo=bar&baz=boz`

This is going to require a lot of time to investigate how this is currently being used by various services, both public and private (internal). It will affect all areas, to do with downloads, updates, server connections, etc. This is the reason why I initially opted to use the key/value pairs delimited by slash, as it causes the least disruption to the client. This will be a major change, that fittingly should go into a major release.

Supplied to server via onPlayerConnect event parameters

Inder00 · 2023-10-19T11:43:48Z

marge

Server/mods/deathmatch/logic/CPlayer.h

Client/core/CCommandFuncs.cpp

Client/core/CCore.cpp

Client/core/CCore.h

fresholia · 2023-11-04T12:33:17Z

Also it should work with connect console command, as I see i can't connect the game with connect command.

Fernando-A-Rocha · 2023-11-04T14:48:23Z

What about also changing redirectPlayer?

Fernando-A-Rocha · 2023-11-16T21:13:49Z

well done, lgtm

Lpsd · 2023-11-16T21:17:29Z

the redirectPlayer functionality has uncovered an issue with the argument parser which needs resolving before this can be merged. See discussion on dev discord

Lpsd · 2023-11-16T21:38:45Z

Also it should work with connect console command, as I see i can't connect the game with connect command.

Works fine for me, can you confirm how you're using the command and what errors you're seeing? @fresholia

Lpsd · 2023-11-18T06:02:13Z

We can't use std::unordered_map<std::string, std::string> to retrieve table arguments in a Lua definition (because a script providing the wrong type ie {1, 2, 3} will cause a crash, and I couldn't find a solution for this).

For this reason I've changed to use CLuaArgument in the unordered_map, as it can handle any value from the Lua side, then convert this to a string using the already available CLuaArgument::GetAsString method

However, I did have to implement a hash function for CLuaArgument in order to use it with std::unordered_map - could do with some opinions on this; not sure if it's correct (approach and implementation-wise). This is an issue we inevitably need to tackle at some point anyway (accepting tables of any value using new arg parser)...

mtasa-blue/Client/mods/deathmatch/logic/lua/CLuaArgument.h

Lines 68 to 78 in c96a8ff

    
           struct Hash 
        
           { 
        
               size_t operator()(const CLuaArgument& a) const 
        
               { 
        
                   SString str; 
        
                   if (!a.GetAsString(str))            // If we can't convert to a string, then use pointer address instead 
        
                       str = std::to_string((unsigned long long)(void**)&a); 
        
                   return std::hash<int>{}(a.GetType()) ^ (std::hash<std::string>{}(str) << 1); 
        
               } 
        
           };

Shared/sdk/SharedUtil.Misc.hpp

Server/mods/deathmatch/logic/packets/CPlayerJoinDataPacket.cpp

Client/mods/deathmatch/logic/lua/CLuaArgument.cpp

Client/core/CCore.cpp

tederis · 2023-11-19T04:48:07Z

We can't use std::unordered_map<std::string, std::string> to retrieve table arguments in a Lua definition (because a script providing the wrong type ie {1, 2, 3} will cause a crash, and I couldn't find a solution for this).

For this reason I've changed to use CLuaArgument in the unordered_map, as it can handle any value from the Lua side, then convert this to a string using the already available CLuaArgument::GetAsString method

However, I did have to implement a hash function for CLuaArgument in order to use it with std::unordered_map - could do with some opinions on this; not sure if it's correct (approach and implementation-wise). This is an issue we inevitably need to tackle at some point anyway (accepting tables of any value using new arg parser)...

mtasa-blue/Client/mods/deathmatch/logic/lua/CLuaArgument.h

Lines 68 to 78 in c96a8ff

struct Hash

{

size_t operator()(const CLuaArgument& a) const

{

SString str;

if (!a.GetAsString(str)) // If we can't convert to a string, then use pointer address instead

str = std::to_string((unsigned long long)(void**)&a);

return std::hash<int>{}(a.GetType()) ^ (std::hash<std::string>{}(str) << 1);

}

};

I think it would be better to fix a crash instead of constructing hash related complicated things. I hope my PR #3246 will help.

This reverts commit c96a8ff.

tederis · 2023-12-01T11:37:29Z

Client/core/CCore.h

@@ -286,6 +288,9 @@ class CCore : public CCoreInterface, public CSingleton<CCore>
    bool   IsUsingCustomStreamingMemorySize();
    size_t GetStreamingMemory();

+    void               SetProtocolConnectArgs(const std::string&& args) { m_strProtocolConnectArgs = args; }


Unfortunately, const rvalue reference doesn't make sense. Even without a const qualifier it wont work as expected because in the function's body args is actually lvalue reference. So you should remove const and cast args to the rvalue explicitly: SetProtocolConnectArgs(std::string&& args) { m_strProtocolConnectArgs = std::move(args); }

I agree with this.
Also, that custom hash function for CLuaArgument isn't good at all, and should be avoided at all costs.
Actually, and kind of comparasion/hashing of it should be avoided.

tederis · 2023-12-01T11:50:27Z

Client/sdk/core/CCoreInterface.h

@@ -190,6 +190,9 @@ class CCoreInterface
    virtual bool   IsUsingCustomStreamingMemorySize() = 0;
    virtual size_t GetStreamingMemory() = 0;

+    virtual void        SetProtocolConnectArgs(const std::string&& args) = 0;


The same: const rvalue has no sense

tederis · 2023-12-01T11:57:12Z

Server/mods/deathmatch/logic/packets/CPlayerJoinDataPacket.h

@@ -44,6 +44,9 @@ class CPlayerJoinDataPacket final : public CPacket

    bool IsOptionalUpdateInfoRequired() { return m_bOptionalUpdateInfoRequired; }

+    const char* GetConnectArgs() { return m_strConnectArgs; };


Why don't you return a string object reference?

tederis · 2023-12-01T11:58:41Z

Server/mods/deathmatch/logic/CGame.cpp

@@ -1735,6 +1735,11 @@ void CGame::Packet_PlayerJoinData(CPlayerJoinDataPacket& Packet)
        pPlayer->SetBitStreamVersion(Packet.GetBitStreamVersion());
        g_pNetServer->SetClientBitStreamVersion(Packet.GetSourceSocket(), Packet.GetBitStreamVersion());

+        const char* connectArgs = Packet.GetConnectArgs();


This related to a CPlayerJoinDataPacket class problem: it's probably better to use a string.

tederis · 2023-12-01T12:19:01Z

Server/mods/deathmatch/logic/CPlayer.h

@@ -335,6 +335,9 @@ class CPlayer final : public CPed, public CClient
    const SString& GetQuitReasonForLog() { return m_strQuitReasonForLog; }
    void           SetQuitReasonForLog(const SString& strReason) { m_strQuitReasonForLog = strReason; }

+    void           SetConnectArgs(const SString&& args) { m_strConnectArgs = args; }


The same problem with const rvalue ref

tederis · 2023-12-01T12:27:18Z

Server/mods/deathmatch/logic/CGame.cpp

+        const char* connectArgs = Packet.GetConnectArgs();
+
+        if (std::strlen(connectArgs) > 0)
+            pPlayer->SetConnectArgs(std::move(Packet.GetConnectArgs()));


I would recommend to remove std::move here because Packet_PlayerJoinData(and even CGame) method doesn't own this string.

tederis · 2023-12-01T12:39:34Z

Client/mods/deathmatch/logic/rpc/CInputRPCs.cpp

@@ -240,6 +240,20 @@ void CInputRPCs::ForceReconnect(NetBitStreamInterface& bitStream)

        if (bitStream.Read(usPort))
        {
+            char* szArgs = new char[MAX_PROTOCOL_CONNECT_ARGS_LENGTH + 1];


It seems that we have a memory leak here. Because szArgs isn't deleted below.
Maybe a preallocated std::string is the solution? It's null terminated and guaranteed to contiguous since C++11 and we have non-const ::data member since C++17(please, use it).

But it's even better to use bitStream.ReadString() (with the modification of a packet itself).

Dutchman101 · 2024-01-28T23:50:18Z

Here's the last status update on this PR:

Dutchman101 — 01/07/2024 1:42 AM
If you address the final Dec 1 code reviews on #3211, it can be merged

lopsi — 01/07/2024 3:04 PM
there is more to do than that
I would like to use other arg format, ?foo=bar&baz=boz
I know it's possible, I already started working on it, but ran into few issues
MTA updater also uses this format in some weird way, I would like to change it too

Nico8340 · 2024-04-07T18:41:12Z

Any updates on this? Would be a really nice feature.

Inder00 · 2025-01-10T00:08:08Z

Bump

lynconsix · 2025-01-20T16:15:06Z

Bump?

arisudesu · 2025-02-11T09:53:21Z

Why would you use this weird syntax with slashes? Aren't usual ampersands enough, as seen in URLs?

What if an argument is specified twice, what value will be in the table - first or last? What if it is specified thrice? E.g. a /arg/1/arg/2/arg/3 or a /arg[]/1/arg[]/2/arg[]/3?

Are all the values of arg[] possible to be extracted? Real-world examples which I know are:

PHP where repeated keys suffixed with [] in the query string form an array;
Golang, we have net/url.Values which is a map of keys to subarrays, that is similar to PHP but doesn't require [];
JavaScript we have URLSearchParams.getAll

and so on.

What if an argument does not have value, then how will it be combined with the arguments following it - like this: /novaluearg//valuearg/1?

Why not pass your "query" string as is and let the server/client script decide how to parse it? And to implement general querystring-like parsing, just add a new fromURLQuery utility function.

Don't reinvent the wheel please, we all (mostly) are used to web URLs and they do their job well, use them.

arisudesu · 2025-02-11T10:06:41Z

/ will be removed from user provided args

What is syntax if one wants to encode slash to an argument key or a value? Is it possible at all, and if it is, how? A usual percent-encoding, as in web URLs?

Lpsd · 2025-02-11T20:34:08Z

Why would you use this weird syntax with slashes? Aren't usual ampersands enough, as seen in URLs?

Requires much more thought and effort to ensure compatibility with existing services which utilize the standard protocol/parameters, such as updates. We'll have to change code surrounding that which hasn't changed in a while.

Also please refer to existing comments to save time #3211 (comment)

I haven't had much interest in this lately so, if you feel motivated, feel free to pick this up!

Lpsd · 2025-02-11T20:38:57Z

All of your other questions can be answered by taking a glance at CLuaPlayerDefs::ArgsToString and testing using what's provided. Not that it is relevant, as the intention was already to use standard URI parameters instead.

Lpsd · 2025-02-11T20:52:28Z

I've converted to draft and updated the description so there's no confusion on the state of this PR.

Lpsd added 7 commits October 15, 2023 07:48

Support mtasa:// protocol connect args

40865c7

Supplied to server via onPlayerConnect event parameters

Fix SharedUtil::Explode declaration

0f93dc5

Increase MAX_PROTOCOL_CONNECT_ARGS_LENGTH to 1024

9a7cc2c

Set nick if empty

be092ca

Update CCoreInterface.h

58ce667

Update CClientGame.cpp

8fdab6d

Remove any usage

ba17a53

tederis reviewed Oct 28, 2023

View reviewed changes

Merge branch 'master' into mtasa-protocol-args

9f05cd8

Lpsd mentioned this pull request Nov 16, 2023

Use MTA temp path for client tests #3209

Closed

Lpsd added 4 commits November 16, 2023 06:00

Address code review

69671b1

Merge fix from multitheftauto#3209

962d7cd

Update fix for SharedUtil.Tests.hpp

5b3849a

Add similar functionality to redirectPlayer via argument

ed7f1cb

Lpsd added 2 commits November 18, 2023 05:47

Allow CLuaArgument usage in std::unordered_map

c96a8ff

Update redirectPlayer args handling

ea878ef

Lpsd added 4 commits November 18, 2023 06:08

Fix compilation error

3398882

Addendum to 3398882

1c4092b

Remove debug code from SharedUtil.Tests.hpp

682e87e

Remove obsolete copy

975a6f1

TheNormalnij reviewed Nov 18, 2023

View reviewed changes

Shared/sdk/SharedUtil.Misc.hpp Outdated Show resolved Hide resolved

TheNormalnij reviewed Nov 18, 2023

View reviewed changes

Server/mods/deathmatch/logic/packets/CPlayerJoinDataPacket.cpp Outdated Show resolved Hide resolved

TheNormalnij reviewed Nov 18, 2023

View reviewed changes

Client/mods/deathmatch/logic/lua/CLuaArgument.cpp Outdated Show resolved Hide resolved

TheNormalnij reviewed Nov 18, 2023

View reviewed changes

Client/core/CCore.cpp Outdated Show resolved Hide resolved

tederis mentioned this pull request Nov 19, 2023

Fix std::unordered_map<std::string, std::string> parsing #3246

Merged

Lpsd added 12 commits November 20, 2023 23:44

Merge branch 'master' into mtasa-protocol-args

9735872

Revert ea878ef (Update redirectPlayer args handling)

9798f5a

Revert "Allow CLuaArgument usage in std::unordered_map"

38831b2

This reverts commit c96a8ff.

Modify table to string processing

64716d9

Remove redundant qualification from declaration

db90a35

Check bitstream version in CPlayerJoinDataPacket

dc3f322

Update curl path in client core project

9e24a06

Change Explode to Split

25809fc

Update CPlayerJoinDataPacket.cpp

5483850

Update CLuaPlayerDefs.cpp

b2f3bed

Modify string replacement in CLuaPlayerDefs::ArgsToString

eb9d516

Remove unnecessary include

d7d597d

tederis reviewed Dec 1, 2023

View reviewed changes

StrixG added the enhancement New feature or request label Dec 21, 2023

Merge branch 'master' into mtasa-protocol-args

adfc201

Lpsd marked this pull request as draft February 11, 2025 20:41

Lpsd added the help wanted Extra attention is needed label Feb 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support mtasa:// protocol connect args #3211

Support mtasa:// protocol connect args #3211

Lpsd commented Oct 15, 2023 •

edited

Loading

Inder00 commented Oct 19, 2023

fresholia commented Nov 4, 2023

Fernando-A-Rocha commented Nov 4, 2023

Fernando-A-Rocha commented Nov 16, 2023

Lpsd commented Nov 16, 2023 •

edited

Loading

Lpsd commented Nov 16, 2023

Lpsd commented Nov 18, 2023 •

edited

Loading

tederis commented Nov 19, 2023

tederis Dec 1, 2023

Pirulax Dec 26, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023 •

edited

Loading

tederis Dec 2, 2023

Dutchman101 commented Jan 28, 2024

Nico8340 commented Apr 7, 2024

Inder00 commented Jan 10, 2025

lynconsix commented Jan 20, 2025

arisudesu commented Feb 11, 2025

arisudesu commented Feb 11, 2025

Lpsd commented Feb 11, 2025

Lpsd commented Feb 11, 2025 •

edited

Loading

Lpsd commented Feb 11, 2025

		@@ -44,6 +44,9 @@ class CPlayerJoinDataPacket final : public CPacket

		bool IsOptionalUpdateInfoRequired() { return m_bOptionalUpdateInfoRequired; }

		const char* GetConnectArgs() { return m_strConnectArgs; };

Support mtasa:// protocol connect args #3211

Are you sure you want to change the base?

Support mtasa:// protocol connect args #3211

Conversation

Lpsd commented Oct 15, 2023 • edited Loading

Changes

Additional Info

Example

Notes

To-do

The method used for URL parameters (key/value pairs delimited by slash) should be changed to use the standard URI parameter format ?foo=bar&baz=boz

Inder00 commented Oct 19, 2023

fresholia commented Nov 4, 2023

Fernando-A-Rocha commented Nov 4, 2023

Fernando-A-Rocha commented Nov 16, 2023

Lpsd commented Nov 16, 2023 • edited Loading

Lpsd commented Nov 16, 2023

Lpsd commented Nov 18, 2023 • edited Loading

tederis commented Nov 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tederis Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dutchman101 commented Jan 28, 2024

Nico8340 commented Apr 7, 2024

Inder00 commented Jan 10, 2025

lynconsix commented Jan 20, 2025

arisudesu commented Feb 11, 2025

arisudesu commented Feb 11, 2025

Lpsd commented Feb 11, 2025

Lpsd commented Feb 11, 2025 • edited Loading

Lpsd commented Feb 11, 2025

Lpsd commented Oct 15, 2023 •

edited

Loading

The method used for URL parameters (key/value pairs delimited by slash) should be changed to use the standard URI parameter format `?foo=bar&baz=boz`

Lpsd commented Nov 16, 2023 •

edited

Loading

Lpsd commented Nov 18, 2023 •

edited

Loading

tederis Dec 1, 2023 •

edited

Loading

Lpsd commented Feb 11, 2025 •

edited

Loading