Bug#972898: hplip: no network scanner detection with 3.20.9
Package: hplip
Version: 3.20.9+dfsg0-4
Severity: important
Tags: patch
Dear Maintainer,
hplip version 3.20.9 replaced the custom mDNS implementation for scanner discovery (protocol/discovery/mdns.c) with using avahi (protocol/discovery/avahiDiscovery.c).
While this is a welcome change in principle, unfortunately the new code does not work.
The main problem is that it does not wait until all callbacks are called before stopping via avahi_simple_poll_quit.
Thus there is a 50/50 chance whether the '_scanner._tcp.local' or the '_uscan._tcp.local' mDNS reply gets processed (in the browse_callback) and it is practically impossible that any scanner gets processed (in the resolve_callback), because avahi_simple_poll_quit is called when the first mDNS reply has been processed.
It seems like this code was never really tested with an actual scanner on the network.
Attached is a patch fixing this by implementing a check_terminate function similar to the one used by avahi-browse.
The second problem is that the code expects the 'ty' part of the mDNS reply, which contains the device name, to start with 'HP'. However this is not always the case.
Previous versions of hplip would simply remove the first three letters of the scanner name and continue, which could be worked around by also removing these three letters in the models.dat. That issue has been reported two years ago upstream without response from HP. (see: https://bugs.launchpad.net/hplip/+bug/1797501)
The new code however discards the scanner if its 'ty' name does not start with 'HP', breaking that workaround.
Fortunately, since the new code now uses avahi, a proper fix is relatively simple: If the 'ty' part of the mDNS response does not start with 'HP', check whether the 'mfg' part does.
The second attached patch implements this fix.
While debugging this I also noticed that the log level for one message is wrong, causing spurious errors to be reported.
The third attached patch changes the log level for this message from BUG to DBG to fix this.
I hope HP solves this eventually, but until then please include these patches in the Debian package, so that scanner detection works again.
Thanks in advance,
Ahzo
PS: I tried to report this upstream, but my Launchpad login attempts always fail, because "something just went wrong in Launchpad".
>From 67dbad25f503e1d8c6794efba3f17718c77848ea Mon Sep 17 00:00:00 2001
From: Ahzo <Ahzo@tutanota.com>
Date: Sun, 25 Oct 2020 22:17:04 +0100
Subject: [PATCH 1/3] avahiDiscovery: wait for all callbacks
When calling avahi_simple_poll_quit too early, not all callbacks will be called.
---
protocol/discovery/avahiDiscovery.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/protocol/discovery/avahiDiscovery.c b/protocol/discovery/avahiDiscovery.c
index 8d325ffc0..395aec088 100644
--- a/protocol/discovery/avahiDiscovery.c
+++ b/protocol/discovery/avahiDiscovery.c
@@ -28,6 +28,7 @@ char ipAddressBuff[MAX_IP_ADDR_LEN]={'\0'};
static int aBytesRead = 0;
static AvahiSimplePoll *aSimplePoll = NULL;
static int aMemAllocated = 0;
+static int n_all_for_now = 0, n_resolving = 0;
/*
This function will fill the dictionary arguments for the dbus function call
@@ -237,6 +238,15 @@ static bool getHPScannerModel(AvahiStringList *iStrList, const char *ikey,char *
return aValueFound;
}
+static void check_terminate() {
+
+ assert(n_all_for_now >= 0);
+ assert(n_resolving >= 0);
+
+ if (n_all_for_now <= 0 && n_resolving <= 0)
+ avahi_simple_poll_quit(aSimplePoll);
+}
+
/*
This function will gets called whenever a service has been resolved successfully or timed out
*/
@@ -300,6 +310,9 @@ static void resolve_callback(
}
}
//avahi_service_resolver_free(r);
+ assert(n_resolving > 0);
+ n_resolving--;
+ check_terminate();
}
/* Called whenever a new services becomes available on the LAN or is removed from the LAN */
static void browse_callback(
@@ -337,11 +350,14 @@ static void browse_callback(
if (!(avahi_service_resolver_new(c, interface, protocol, name, type, domain, AVAHI_PROTO_INET, (AvahiLookupFlags)0, resolve_callback, c)))
BUG( "Failed to resolve service '%s': %s\n", name, avahi_strerror(avahi_client_errno(c)));
+ else
+ n_resolving++;
break;
case AVAHI_BROWSER_ALL_FOR_NOW:
- avahi_simple_poll_quit(aSimplePoll);
+ n_all_for_now--;
+ check_terminate();
break;
}
}
@@ -444,6 +460,7 @@ static void avahi_setup(const int iCommandType, const char* iHostName)
goto fail;
}
}
+ n_all_for_now++;
/* Create the service browser */
if (!(sb = avahi_service_browser_new(client, AVAHI_IF_UNSPEC, AVAHI_PROTO_INET, "_scanner._tcp", NULL, (AvahiLookupFlags)0, browse_callback, client)))
@@ -451,6 +468,7 @@ static void avahi_setup(const int iCommandType, const char* iHostName)
BUG( "Failed to create service browser: %s\n", avahi_strerror(avahi_client_errno(client)));
goto fail;
}
+ n_all_for_now++;
}
else if ( iCommandType == AVAHI_HOST_LOOKUP ) /* Find the IP (IPV4) address of given host name */
{
--
2.28.0
>From 498643ef8e8bf78511b0a5988d473627b790dc62 Mon Sep 17 00:00:00 2001
From: Ahzo <Ahzo@tutanota.com>
Date: Sun, 25 Oct 2020 22:18:20 +0100
Subject: [PATCH 2/3] avahiDiscovery: also recognize scanners via mfg
The value of ty does not always start with 'HP'.
In that case check if the value of mfg starts with 'HP'.
Otherwise these scanners are not recognized, even though they should be.
---
protocol/discovery/avahiDiscovery.c | 28 ++++++++++++++++++----------
protocol/discovery/avahiDiscovery.h | 2 +-
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/protocol/discovery/avahiDiscovery.c b/protocol/discovery/avahiDiscovery.c
index 395aec088..a3d757e81 100644
--- a/protocol/discovery/avahiDiscovery.c
+++ b/protocol/discovery/avahiDiscovery.c
@@ -207,20 +207,29 @@ Note:
"mopria-certified-scan=1.2" "vers=2.63" "txtvers=1"
*/
-static bool getHPScannerModel(AvahiStringList *iStrList, const char *ikey,char **oKeyValue,size_t *aMdlStrLen)
+static bool getHPScannerModel(AvahiStringList *iStrList, const char *ikey,char **oKeyValue,size_t *aMdlStrLen, size_t *offset)
{
- AvahiStringList *aStrList = NULL;
+ AvahiStringList *aStrList = NULL, *bStrList = NULL;
bool aValueFound = false;
- char *aKey = NULL;
+ char *aKey = NULL, *bKeyValue = NULL;
aStrList = avahi_string_list_find(iStrList, ikey);
+ bStrList = avahi_string_list_find(iStrList, MFG_NAME);
+ if (bStrList)
+ bKeyValue = avahi_string_list_get_text(bStrList);
/*
aStrList will be Null ,if given key is not found,
avahi_string_list_get_pair will return zero if key found,
also need to process the response form HP scanner only.
*/
- if( ( aStrList != NULL ) && ( avahi_string_list_get_pair(aStrList, &aKey, oKeyValue, aMdlStrLen) == 0 )
- && ( *oKeyValue != NULL ) && ( strncmp(MFG_HP,*oKeyValue,MFG_HP_LEN) == 0 ))
- {
+ if ( ( aStrList != NULL ) && ( avahi_string_list_get_pair(aStrList, &aKey, oKeyValue, aMdlStrLen) == 0 ) && ( *oKeyValue != NULL ) ) {
+ if ( strncmp(MFG_HP, *oKeyValue, MFG_HP_LEN) == 0 ) {
+ aValueFound = true;
+ *offset = HP_SKIP_MFG_NAME_SIZE;
+ }
+ else if ( bKeyValue != NULL && strncmp(MFG_HP, bKeyValue + 4, MFG_HP_LEN) == 0 ) {
+ aValueFound = true;
+ *offset = 0;
+ }
size_t aIndex = 0;
for(aIndex = 0; aIndex<(*aMdlStrLen); aIndex++)
{
@@ -231,7 +240,6 @@ static bool getHPScannerModel(AvahiStringList *iStrList, const char *ikey,char *
(*oKeyValue)[aIndex] = tolower((*oKeyValue)[aIndex]);
}
DBG("oKeyValue is %s\n", *oKeyValue);
- aValueFound = true;
}
if( aKey != NULL )
avahi_free( (void *)aKey );
@@ -273,16 +281,16 @@ static void resolve_callback(
case AVAHI_RESOLVER_FOUND: {
char aIPAddress[AVAHI_ADDRESS_STR_MAX]={'\0'};
char *aMdlStr = NULL;
- size_t aMdlStrLen = 0 ;
+ size_t aMdlStrLen = 0, offset = 0;
DBG( "Service '%s' of type '%s' in domain '%s':\n", name, type, domain);
avahi_address_snprint(aIPAddress, AVAHI_ADDRESS_STR_MAX, address);
DBG("avahi_address_snprint : \n %s \n",aIPAddress);
- if( getHPScannerModel(txt,TYPE_NAME,&aMdlStr,&aMdlStrLen) == true )
+ if( getHPScannerModel(txt, TYPE_NAME, &aMdlStr, &aMdlStrLen, &offset) == true )
{
DBG("aMdlStr name is %s \n",aMdlStr);
DBG("aMdlStrLen is %zu \n",aMdlStrLen);
char aTempUri[MAX_URI_LEN] = {'\0'};
- snprintf( aTempUri, MAX_URI_LEN, "hp:/net/%s?ip=%s&queue=false", aMdlStr+HP_SKIP_MFG_NAME_SIZE, aIPAddress);
+ snprintf( aTempUri, MAX_URI_LEN, "hp:/net/%s?ip=%s&queue=false", aMdlStr + offset, aIPAddress);
if(aUriBuf == NULL)
{
aUriBuf = (char *)calloc(HP_MAX_SCAN_BUFF,sizeof(char));
diff --git a/protocol/discovery/avahiDiscovery.h b/protocol/discovery/avahiDiscovery.h
index 2bc3ce292..e746c680c 100644
--- a/protocol/discovery/avahiDiscovery.h
+++ b/protocol/discovery/avahiDiscovery.h
@@ -30,7 +30,7 @@
#define HP_SKIP_MFG_NAME_SIZE 3
#define MFG_HP "HP"
#define MFG_HP_LEN 2
-#define MFG_NAME "usb_MFG"
+#define MFG_NAME "mfg"
#define MDL_NAME "mdl"
#define TYPE_NAME "ty"
#define HPMUD_LINE_SIZE 256
--
2.28.0
>From f248a2ba76c149350f7090472b061a21f34aad9a Mon Sep 17 00:00:00 2001
From: Ahzo <Ahzo@tutanota.com>
Date: Sun, 25 Oct 2020 22:18:55 +0100
Subject: [PATCH 3/3] avahiDiscovery: fix log level for debug message
This is not a bug, so it should not be logged as one.
---
protocol/discovery/avahiDiscovery.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/protocol/discovery/avahiDiscovery.c b/protocol/discovery/avahiDiscovery.c
index a3d757e81..d078b62f0 100644
--- a/protocol/discovery/avahiDiscovery.c
+++ b/protocol/discovery/avahiDiscovery.c
@@ -349,7 +349,7 @@ static void browse_callback(
return;
case AVAHI_BROWSER_NEW:
- BUG( "(Browser) NEW: service '%s' of type '%s' in domain '%s'\n", name, type, domain);
+ DBG( "(Browser) NEW: service '%s' of type '%s' in domain '%s'\n", name, type, domain);
/* We ignore the returned resolver object. In the callback
function we free it. If the server is terminated before
--
2.28.0
Reply to: