Synergy-core: Oversized Bonjour protocol name could conflict

[nyetwurk]

Bad service type in x.x.x.x._synergyServerZeroconf._tcp.local. Application protocol name must be underscore plus 1-15 characters. See http://www.dns-sd.org/ServiceTypes.html

Bad service type in ._synergyClientZeroconf._tcp.local. Application protocol name must be underscore plus 1-15 characters. See http://www.dns-sd.org/ServiceTypes.html

[nyetwurk]

Service name cannot have more than 15 chars after the initial _

diff --git a/src/gui/src/ZeroconfService.cpp b/src/gui/src/ZeroconfService.cpp
index 165912d..5412944 100644
--- a/src/gui/src/ZeroconfService.cpp
+++ b/src/gui/src/ZeroconfService.cpp
@@ -33,8 +33,8 @@ static const QStringList preferedIPAddress(
                                "10." <<
                                "172.");

-const char* ZeroconfService:: m_ServerServiceName = "_synergyServerZeroconf._tcp";
-const char* ZeroconfService:: m_ClientServiceName = "_synergyClientZeroconf._tcp";
+const char* ZeroconfService:: m_ServerServiceName = "_synergyServer._tcp";
+const char* ZeroconfService:: m_ClientServiceName = "_synergyClient._tcp";

 ZeroconfService::ZeroconfService(MainWindow* mainWindow) :
        m_pMainWindow(mainWindow),

[nyetwurk]

Sigh

[nbolton]

@nyetwurk Do you use Bonjour?

[nyetwurk]

Yes, and unfortunately the PR breaks old/new synergy client/server discovery due to the renaming. Was hoping this PR would be included sooner rather than later so less people would be affected.

If I knew more about Bonjour I could work on forward/backwards compat, but I don't have any dev experience

[nbolton]

@nyetwurk Have you tried Synergy 2 Beta? It has an alternative solution to Bonjour. https://symless.com/synergy/features/s2

[nyetwurk]

I was LITERALLY only using bonjour to test my PR. I assumed it was refused because of the backwards/forwards compatibility issue between old/new client/servers

[nbolton]

I ask because I'm considering disabling it by default. It seems to cause more issues than it solves.

[nyetwurk]

I vote nuke it from orbit.

[nbolton]

💥

[nyetwurk]

Wait, does this mean the code supporting bj is removed? If so, where is the commit?

[nbolton]

Not yet, still considering the impact.

[nyetwurk]

Can you re-open this bug pending your decision (and commit)? Thanks. I have my own search for "open bugs" I keep an eye on to remind me to work on them.

[nbolton]

synergy         Synergy Peer Discovery
                Karl Timmermann <timmerk at gmail.com>
                Protocol description: Proprietary
                Defined TXT keys: None

Interesting, looks like Karl registered this for the original Qt GUI.

[nbolton]

@nyetwurk Agh, just realised that I closed this twice. 🤦‍♂️

[nbolton]

@nyetwurk What is the effect of having an oversized name? We need to consider that changing the name will break compatibility across versions (users often mix versions of Synergy) and if we remove it altogether, then people less familiar with IPs will struggle.

[nyetwurk]

From what I read it may break the proto for other devices, but since synergy is desktop only, it probably doesn't matter.

What was annoying me was the log warning from the bonjour library

Obviously, if you decide to nuke bj altogether, it becomes a non-issue.

I don't even know if people are actually depending on bj for this stuff; plus if the endusers aren't doing authentication/encryption it might be a bad idea anyway.

[nbolton]

What was annoying me was the log warning from the bonjour library

Is this at runtime or is it a compile warning?

[nyetwurk]

IIRC runtime, but it has been a while

[nbolton]

If we go ahead with this, it should solve that issue.
https://github.com/symless/synergy-core/issues/6319

[nyetwurk]

May as well fix the proto names then as well, since new builds will stop working with old bj versions unless autoconfig is enabled on the new version :) I'm always in favor of adhering to standards.

[nyetwurk]

Ooops sorry, fat fingered it lol

[nbolton]

Fair point!