404 error due to extra forward slash in get path

jarvismeier commented 8 months ago

Describe The Bug: Since updating to Unifi 8.1.x the plugin errors out when attempting to connect/query the api path. Looking in the logs there is an extra forward slash which is generating the 404 error ie: https://10.0.1.xxx:8443//v2/api/fingerprint_devices/0

To Reproduce: Update self hosted Unifi to 8.1

Expected behavior: plugin should not retrieve 404 errors when contacting the API

Logs:


error: '<!doctype html><html lang="en"><head><title>HTTP Status 404 – Not Found</title><style type="text/css">body {font-family:Tahoma,Arial,sans-serif;} h1, h2, h3, b {color:white;background-color:#525D76;} h1 {font-size:22px;} h2 {font-size:16px;} h3 {font-size:14px;} p {font-size:12px;} a {color:black;} .line {height:1px;background-color:#525D76;border:none;}</style></head><body><h1>HTTP Status 404 – Not Found</h1></body></html>',
  options: {
    rejectUnauthorized: false,
    jar: RequestJar { _jar: [CookieJar] },
    headers: { 'User-Agent': 'node.js unifi-events UniFi Events' },
    json: true,
    uri: 'https://10.0.1.xxx:8443//v2/api/site/default/clients/active',
    method: 'GET',
    callback: [Function: RP$callback],
    transform: undefined,
    simple: true,
    resolveWithFullResponse: false,
    transform2xxOnly: false
  },

Plugin Config:

N/a

Screenshots:

Environment:

Plugin Version: 1.4.0
Homebridge Version: 1.7.0
Node.js Version: 20.11.1
NPM Version: 10.2.4
Operating System: Debian GNU/Linux Bookworm (12)

DanielSchaffer commented 7 months ago

I'm seeing the same after updating my controller to 8.1

DanielSchaffer commented 6 months ago

I have my unifi controller behind an nginx proxy, so I was able to work around this by adding this to the server section:

        merge_slashes off;
        rewrite (.*?)//+(.*) $1/$2 permanent;

My understanding is that merge_slashes only allows nginx to treat multiple slashes as one, but it doesn't actually rewrite anything, so they'd still get passed through the proxy as-is. The rewrite doesn't work with it still on, I'm guessing because the regexp doesn't pick up the multiple slashes because of that merge_slashes behavior.

peterneutron commented 1 month ago

The issue was caused by improper URL concatenation in the UnifiEvents module. Double slashes (//) appeared in the API paths when controller.href (with a trailing slash) was concatenated with path (starting with a leading slash). This caused a 404 error in certain requests. Since UnifiEvents appears to be an archived project and is no longer responding to issues, I’ll post the fix here.

diff --git a/index.js b/index.js
index 3915ea7..7188c28 100644
--- a/index.js
+++ b/index.js
@@ -1,4 +1,4 @@
-const url = require('url');
+const { URL } = require('url');
 const EventEmitter = require('eventemitter2').EventEmitter2;
 const WebSocket = require('ws');
 const rp = require('request-promise');
@@ -18,9 +18,10 @@ module.exports = class UnifiEvents extends EventEmitter {
         this.opts.site = this.opts.site || 'default';
         this.opts.unifios = this.opts.unifios || false;

-
         this.userAgent = 'node.js unifi-events UniFi Events';
-        this.controller = url.parse('https://' + this.opts.host + ':' + this.opts.port);
+
+        // Sanitize the controller URL once to ensure no trailing slashes
+        this.controller = new URL(`https://${this.opts.host}:${this.opts.port}`);

         this.jar = rp.jar();

@@ -54,7 +55,6 @@ module.exports = class UnifiEvents extends EventEmitter {
     _login(reconnect) {
         let endpointUrl = `${this.controller.href}api/login`;
         if (this.opts.unifios) {
-            // unifios using one authorisation endpoint for protect and network.
             endpointUrl = `${this.controller.href}api/auth/login`;
         }

@@ -76,7 +76,8 @@ module.exports = class UnifiEvents extends EventEmitter {
         let eventsUrl = `wss://${this.controller.host}/wss/s/${this.opts.site}/events`;
         if (this.opts.unifios) {
             eventsUrl = `wss://${this.controller.host}/proxy/network/wss/s/${this.opts.site}/events`;
-        }
+        }
+
         this.ws = new WebSocket(eventsUrl, {
             perMessageDeflate: false,
             rejectUnauthorized: !this.opts.insecure,
@@ -137,7 +138,6 @@ module.exports = class UnifiEvents extends EventEmitter {

     _event(data) {
         if (data && data.key) {
-            // TODO clarifiy what to do with events without key...
             const match = data.key.match(/EVT_([A-Z]{2})_(.*)/);
             if (match) {
                 const [, group, event] = match;
@@ -153,20 +153,27 @@ module.exports = class UnifiEvents extends EventEmitter {
             });
     }

+    // Refactored _url method to use the URL constructor
     _url(path) {
+        const baseUrl = new URL(this.controller.href); // Use the URL constructor
+
         if (this.opts.unifios) {
-            // unifios using an proxy, set extra path
-            if (path.indexOf('/') === 0) {
-                return `${this.controller.href}proxy/network/${path}`;
+            // unifios using a proxy, set extra path
+            if (path.startsWith('/')) {
+                path = `proxy/network${path}`;
+            } else {
+                path = `proxy/network/api/s/${this.opts.site}/${path}`;
             }
-            return `${this.controller.href}proxy/network/api/s/${this.opts.site}/${path}`;
-        }
-        else {
-            if (path.indexOf('/') === 0) {
-                return `${this.controller.href}${path}`;
+        } else {
+            if (path.startsWith('/')) {
+                // Leave path as is
+            } else {
+                path = `api/s/${this.opts.site}/${path}`;
             }
-            return `${this.controller.href}api/s/${this.opts.site}/${path}`;
         }
+
+        // Construct the final URL safely
+        return new URL(path, baseUrl).href;
     }

     get(path) {

DouweM / homebridge-unifi-occupancy

404 error due to extra forward slash in get path #27