Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WEBUI: prototype.js: SyntaxError: Invalid regular expression: nothing to repeat #2521

Closed
gcarls opened this issue Nov 19, 2023 · 23 comments · Fixed by #2530
Closed

WEBUI: prototype.js: SyntaxError: Invalid regular expression: nothing to repeat #2521

gcarls opened this issue Nov 19, 2023 · 23 comments · Fixed by #2530
Labels
🐛 bug-report Something isn't working 🏷️ WebUI This refs the WebUI component

Comments

@gcarls
Copy link

gcarls commented Nov 19, 2023

Describe the issue you are experiencing

Bei Navigation nach Einstellungen - Geräte "hängt" WebUI im Zustand Loading. DIe Liste der Geräte wird nicht angezeigt.
In der Developer Tools Console des Web Browsers gibt es die folgende Fehlermeldung:
SyntaxError: Invalid regular expression: nothing to repeat
bei Ausführung von Prototype.js
image

Describe the behavior you expected

Bei Navigation Einstellungen -> Geräte sollte die Geräteliszte angezeigt werden

Steps to reproduce the issue

  1. Click Einstellungen
  2. Click Geräte
  3. Loading Animation wird angezeigt. Die Anzeige der Geräteliste bleibt aus
    ...

What is the version this bug report is based on?

3.71.12.2023.10.12

Which base platform are you running?

ova (Open Virtual Infrastructure)

Which HomeMatic/homematicIP radio module are you using?

n/a

Anything in the logs that might be useful for us?

siehe Screenshot:
prototype.js
SyntaxError: Invalid regular expression: nothing to repeat

Additional information

Als Radio Module wird HB-RF-ETH verwendet

@gcarls gcarls added the 🐛 bug-report Something isn't working label Nov 19, 2023
@gcarls
Copy link
Author

gcarls commented Nov 24, 2023

Das scheint eine Browser Inkompatibilität zu sein.
Der Fehler tritt bei mir nur im Safari Browser auf. Firefox ist OK.

@jens-maus
Copy link
Owner

Bis jetzt hat das außer dir noch niemand berichtet? Das für sich alleine genommen ist schon sehr sonderbar und deutet daraufhin das es ein reines Problem deines Systems bzw. deiner Nutzungsumgebung ist. Daher bitte andere Leidensgenossen finden wäre der erste Ansatz...

@gcarls
Copy link
Author

gcarls commented Nov 25, 2023

Fehler gefunden:
Dieses Problem tritt auf, wenn der Devicename ein "+" enthält.
Im vorliegenden Fall war das erste Zeichen im Devicename ein '+'.

@gcarls gcarls closed this as completed Nov 25, 2023
@jens-maus jens-maus reopened this Nov 25, 2023
@jens-maus
Copy link
Owner

Ok, aber das bedeutet wir müssten hier ggf. in den Filtern nachbessern. Deshalb bleibt das Ticket erst einmal offen. Vielleicht kann @jp112sdl hiee ja mal schauen was die Regexp in den Filtern da durcheinander bringen wenn da ein + im Namen ist. Er hatte meiner Erinnerung nach ja die Filter um wildcards usw erweitert..

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 25, 2023

Kann das daher kommen?

--- occu/WebUI/www/webui/webui.js.orig
+++ occu/WebUI/www/webui/webui.js
@@ -7882,11 +7882,8 @@
//var patternList = m_value.split("|");
text = text.toLowerCase();
- for (var i = 0, len = patternList.length; i < len; i++)
- {
- if (0 <= text.indexOf(patternList[i])) { return true; }
- }
-
+ var r = new RegExp(m_value.toLowerCase());
+ if (r.test(text) === true) {return true; }

Edit:

Hab das mal in JS online nachgebaut.. Da scheint alles zu funktionieren...

let text = '+Hello$world+';
text = text.toLowerCase();

const r = new RegExp('Hello.*'.toLowerCase());

if(r.test(text) === true){
  console.log("True");
}

@jens-maus
Copy link
Owner

Kann gut sein das es daher kommt. Kam selbst noch nicht das in der webui nachzustellen und zu debuggen... wurde ja auch gesagt das es ggf. auch browserabhängig ist. Muss man also mal debuggen dort..

@Baxxy13
Copy link
Contributor

Baxxy13 commented Nov 25, 2023

Also mit "Chromium based Browsern" kann ich das auch nicht reproduzieren.
Egal ob ein Namensfilter gesetzt ist oder nicht.
RM_plus_vor_Geraetenamen

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 25, 2023

Der Fehler tritt bei mir nur im Safari Browser auf. Firefox ist OK.

Kann das nicht nachvollziehen.. Bei mir (ebenfalls Safari) und einem Gerät mit einem + im Namen.. kein Fehler in der Konsole und die Geräteliste lädt ebenfalls korrekt.

Auch die Suche (beides: als Regex oder nicht) funktioniert, sowohl Suche nach dem jeweiligen Gerät, als auch die Suche nach einem beliebigen Anderen.

@gcarls
Copy link
Author

gcarls commented Nov 25, 2023

Nachdem ich die "+" aus den Devicenamen entfernt habe war der Fehler verschwunden. Ich habe aber eben weiter experimentiert und festgestellt, dass ein + im Filter reicht um den Fehler hervorzurufen.
Ich habe nur "+UV1" im Filter der Spalte "Name" eingegeben, nach Bestätigung passiert zunächst nichts. Verlässt man die Liste und geht anschließend wieder über Einstellungen und Geräte in die Liste, hängt die Liste.
Dummerweise merkt sich der Browsercache dieses Suchkriterium. Wenn man die Liste verlässt und anschließend wieder über Einstellungen/Geräte in die Liste wechselt bleibt es wieder mit der obigen Fehlermeldung hängen.
Erst das Löschen des Browsercache löszt das Problem.
Bei mir ist das Problem im Firefox nur deswegen nicht aufgetreten, weil im cache des Firefox kein Suchkriterium mit "+" gespeichert war.
Durch obige Vorgehensweise ließ sich das Problem bei mir in Firefox und Safari nachstellen.

image

@Baxxy13
Copy link
Contributor

Baxxy13 commented Nov 25, 2023

Ah ja, mit dem "+" im Filter bekomme ich das jetzt auch reproduziert.

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 25, 2023

Ahh.. damit kann ich es reproduzieren.

Ich glaube es liegt hier nicht an RM oder sonstiger Logik.. Der Regex is hier einfach fehlerhaft. + im Regex bedeutet, dass der vorangehende Token (in dem Fall "") > 1x vorkommen muss.

Das passt auch zum Fehler: "...nothing to repeat".

Behen lässt sich das ganze indem man das + escaped; also \+UV würde gehen

@jens-maus
Copy link
Owner

Dann müsste man prüfen ob es ggf. irgendeine Funktion gibt um erst prüfen zu lassen ob der eingegebene Suchstring auch ein valider mit/ohne Regexp ist um dann Event das new Regexp aufzurufen oder eben nicht... So wäre zumindest mein Ansatz.

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 25, 2023

Straight forward wäre das dann sowas hier:

let text = '+Hello$world+';
text = text.toLowerCase();
let regexString = '+Hello.*'.toLowerCase();

function isValidRegex(possibleInvalidRegex) {
  try
  {
    const r = new RegExp(possibleInvalidRegex);
  }
  catch (err){
    return false;
  }
  
  return true;
}

if(isValidRegex(regexString)){
  const r = new RegExp(regexString);

  if(r.test(text) === true){
    console.log("True");
  }
} else {
  // check with orig method?
}

@jens-maus
Copy link
Owner

Sowas in der Art, ja. Wundert mich aber das es da nicht schon was fertiges/vorhandenes bei Tante Google gibt...

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 25, 2023

Naja.. das war von Onkel StackOverflow :D

@jens-maus
Copy link
Owner

Na dann her mit dem PR sodass dieses Problem dann hoffentlich nicht mehr auftritt in Zukunft...

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 25, 2023

Alles klar :)

Da das ja erst meine zweite contrib ist.. ich hoffe ich habe alles korrekt ausgeführt :)

(vorallem die größe des patches mach mir Kopfzerbrechen 🤔 )

@jens-maus
Copy link
Owner

Ja, das passt leider nicht! Die Webui.js muss erst ein einzelne Zeilen mit \n umgewandelt werden bevor man Änderungen in dem Bereich vornehmen kann. Also bitte nochmal nacharbeiten und/oder einfach mal das diff deiner Änderung da mit einpflegen/zeigen.

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 26, 2023

Ok. Habs mir fast gedacht :)

Gibts dafür irgendein script?

@jens-maus
Copy link
Owner

jens-maus commented Nov 26, 2023

Ein Script nicht direkt leider. Aber das folgende Kommando sollte aus der webui.js mit gewrappten Zeilen eine machen die dann auf mehrere Zeilen umgewandelt ist damit man die dann entsprechend bearbeiten kann:

sed -i '1,10s/\\n/\\n\n/g' webui.js

Und dann legt man die dort im Patchverzeichnis ab und nutzt immer die, denn der Build Skript baut die dann automatisch wieder zusammen mit der Gegenteiligen Operation.

Da ist leider die webui.js ein special case wegen diesen gewrappten \n Zeilen wie eQ3 die in OCCU ausliefert...

Achja und die Gegenoperation ist dann diese hier:

sed -i ':a;N;$$!ba;s/\\n\n/\\n/g' webui.js

Dash verwandelt die mehrzeilige Datei dann wieder in eine wo am Schluss die ersten zehn Zeilen aus langen Einzeilern besteht.

Das ist auch hier im Makefile zu finden:

define OCCU_UNWRAP_WEBUI_JS

Hoffe damit ist das Prozedere klarer wie man an der WebUI.js in den ersten 10 Zeilen Änderungen vornehmen kann..

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 26, 2023

Ok, dh. zum committen des Patches

  1. \n unwrap mit erstem Kommando
  2. Änderungen durchführen, Patch erzeugen, committen
  3. \n wrap mit zweitem Kommando

Ich kann das leider nicht testen, da ich keine Testumgebung habe; das Docker-Image kann ich schon bauen, aber ohne Funkmodul wird das Starten nichts

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 26, 2023

Ok, jetzt hab ichs denke ich :)

jens-maus added a commit that referenced this issue Nov 26, 2023
…T-VBFK)

Co-authored-by: IT-VBFK <it@voecklabrucker-freikriche.at>
Co-authored-by: Jens Maus <mail@jens-maus.de>
@jens-maus jens-maus added this to the next release milestone Nov 26, 2023
@jens-maus jens-maus added the 🏷️ WebUI This refs the WebUI component label Nov 26, 2023
@jens-maus
Copy link
Owner

Ok, hab den PR nach keinen Änderungen und einem Fix meinerseits nun mal gemerged. Bitte mit dem nächsten nightly snapshot schauen ob nun alles passt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug-report Something isn't working 🏷️ WebUI This refs the WebUI component
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants