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

Anführungszeichen in Beschreibungen von Systemvariablen nicht akzeptiert #2285

Closed
TinkyWonky opened this issue Apr 11, 2023 · 17 comments · Fixed by #2291
Closed

Anführungszeichen in Beschreibungen von Systemvariablen nicht akzeptiert #2285

TinkyWonky opened this issue Apr 11, 2023 · 17 comments · Fixed by #2291
Labels
🐛 bug-report Something isn't working 🏷️ WebUI This refs the WebUI component

Comments

@TinkyWonky
Copy link

TinkyWonky commented Apr 11, 2023

Describe the issue you are experiencing

Ausgangssituation: Es existiere eine Systemvariable, die als Beschreibung den langen Text werksseitig definierte Variable, die nach Systemstart den Wert "wahr" enthält enthält. Der Text wird korrekt übernommen und in der Übersicht angezeigt.

Problem: Wird der Systemvariable bearbeiten Dialog erneut geöffnet, wird der String auf werksseitig definierte Variable, die nach Systemstart den Wert gekürzt, siehe Screenshot. Beim Bestätigen des Dialogs wird der gekürzte String persistiert.

Describe the behavior you expected

Der in der Beschreibung gespeicherte Text wird vom Dialog übernommen und kann dort weiter bearbeitet werden.
ODER
Die Verwendung von Anführungszeichen wird vom Dialog nicht erlaubt (analog zum Dialog zum Ändern der Beschreibung von Programmen).

Steps to reproduce the issue

siehe Beschreibung

What is the version this bug report is based on?

3.69.6.20230407

Which base platform are you running?

rpi3 (RaspberryPi3)

Which HomeMatic/homematicIP radio module are you using?

RPI-RF-MOD

Anything in the logs that might be useful for us?

nix

Additional information

Unbenannt

@TinkyWonky TinkyWonky added the 🐛 bug-report Something isn't working label Apr 11, 2023
@jp112sdl
Copy link
Contributor

Das Problem tritt hier auf:
https://github.com/eq-3/occu/blob/master/WebUI/www/rega/pages/tabs/admin/msg/newSysVar.htm#L44-L51

      <%
        var svDesc = "";
        if (system.GetVar("createNew") == 0) 
        { 
          svDesc =  sv.DPInfo(); 
        } 
      %>
      <input id="inpDesc" type="text" value="<%Write(svDesc)%>" onblur="stringRemoveNonPrintableChars(this);"/>

Sobald in der svDesc ein " auftaucht, ist der value=... an der Stelle zu Ende.

Das trifft auch auf die erste Spalte Name zu.

Trägt man irgendwas mit ' ein, wird der Inhalt gar nicht erst gespeichert.

Keine Ahnung, wie man das fixen kann... evtl. mit einem Warn-Popup unzulässiger Zeichen 😎

@jens-maus jens-maus added the 🏷️ WebUI This refs the WebUI component label Apr 12, 2023
@jens-maus
Copy link
Owner

Da müsste man vmtl. das " mit einem \" ersetzen bevor man das an value=... übergibt. Und dann eben nach der Eingabe wieder zurückübersetzen.

@jp112sdl
Copy link
Contributor

Ein DPInfo().Replace('"','\"'); hat jedenfalls nicht geholfen.
Hatte es auch mit %22% versucht, aber das steht dann auch im Klartext im Eingabefeld 😕

@jens-maus
Copy link
Owner

Und wenn du das value="...." in ein value='...' änderst? Oder das Write(svDesc) in ein Write(svDesc).Replace(...) änderst?

@jp112sdl
Copy link
Contributor

Nope:
<input id="inpDesc" type="text" value="<%Write(svDesc.Replace('"','\"'))%>" onblur="stringRemoveNonPrintableChars(this);"/>

Es wird weiterhin am 1. " abgeschnitten.

Was geht ist &quot;:
<input id="inpDesc" type="text" value="<%Write(svDesc.Replace('"','&quot;'))%>" onblur="stringRemoveNonPrintableChars(this);"/>

Dann wird es korrekt angezeigt und auch gespeichert.

@jens-maus
Copy link
Owner

Ok, das mit &quot; wäre ja schon einmal ein Anfang. Aber ein ' geht immer noch nicht?

Haben wir vllt. ne möglichkeit das im Javscript Kontext für alle Chars zu erledigen? Sowas wie das hier vllt.:

https://stackoverflow.com/questions/18749591/encode-html-entities-in-javascript

D.h.

var encodedStr = rawStr.replace(/[\u00A0-\u9999<>\&]/g, function(i) {
   return '&#'+i.charCodeAt(0)+';';
});

Im Grunde also alles ab einem gewissen Unicode-Range einfach in &#XXXX wandeln und dann das an value=XXXX übergeben?

@MichaelN0815
Copy link
Contributor

Ich würde ganz brutal das Eingeben von String Delimitern verbieten.
Zum Beispiel kann man zulässige Zeichen mit dem pattern Attribut festlegen:
https://www.w3schools.com/tags/att_input_pattern.asp

@jp112sdl
Copy link
Contributor

Ich würde ganz brutal das Eingeben von String Delimitern verbieten.

Ja, war auch mein Gedanke mit dem "Hinweis Popup".
An anderen Stellen wird ja auch verhindert, dass "<>,&" usw. verwendet werden.
z.B. bei Gerätenamen.
Bildschirmfoto 2023-04-12 um 09 40 52

Aber ein ' geht immer noch nicht?

Sobald ein ' drin ist, kommt beim Speichern ein Server-Fehler 500
Weil der POST-Inhalt dann falsch ist. Hier werden Zeichenfolgen mit ' eingeschlossen.
Hier mal mit te'st:

09:37:14.433 XHRPOSThttp://192.168.1.254/esp/system.htm?sid=@bQD8X8UjM6@
[HTTP/1.1 500 Internal Server Error 78ms]

<![CDATA[action = 'saveSysVar';integer createNew = 0;integer varid = 11105;string sName = 'te'st';string sInfo = '';string sValue = 'null';string sUnit = '';integer iSubType = 11;integer iChnId = 0;]]>

@jens-maus
Copy link
Owner

Ah verstehe. Dann schade, aber wohl nicht zu ändern das man wirklich das eingeb en dieser Sonderzeichen gänzlich unterbindet. Da ist die WebUI und alles drumherrum leider nicht ganz so flexibel und man sieht ja an dem sName im CDATA das es dann an anderen Stellen schief gehen kann und wird. Ergo würde ich nun auch dafür plädieren in dem input ein erlaubte pattern einzuführen das man beim tippen schon gar nicht diese Zeichen eingeben kann. Ob es ein extra Popup dann braucht müsste man sehen wieviel aufwand das bedeutet.

@jp112sdl
Copy link
Contributor

jp112sdl commented Apr 12, 2023

Ob es ein extra Popup dann braucht müsste man sehen wieviel aufwand das bedeutet.

Das Popup kommt automatisch von der JS-Methode isTextAllowed()

Es müsste die saveSysVars (https://github.com/eq-3/occu/blob/master/WebUI/www/rega/pages/tabs/admin/msg/newSysVar.htm#L166-L247) eigentlich nur in eine Bedingungsprüfung gepackt werden

 if ( isTextAllowed($("inpDesc").value) && isTextAllowed($("inpName").value) ) {
...
}

@jens-maus
Copy link
Owner

Das hört sich nach einem guten Plan an! Ich würde allerdings vorschlagen nicht die Ganze saveSysVars jetzt einzurücken und das if() drumherum zu bauen, sondern um die Änderungen gegenüber der originalen saveSysVars gering zu halten das if() eine negativprüfung durchführen zu lasen ala:

if ( isTextAllowed($("inpDesc").value) === false || isTextAllowed($("inpName").value) === false ) {
  return
}

D.h. das ganz oben zu machen damit der rest der funktion unangefasst bleibt und das potential für merge konflikte mit OCCU dann gering gehalten wird.

@jp112sdl
Copy link
Contributor

Ja, so ist es eleganter.

Baust du das mit ein in den 0035 ?

@jens-maus
Copy link
Owner

Baust du das mit ein in den 0035 ?

Öhhh, da hatte ich jetzt irgendwie auf dich gesetzt :)

@jp112sdl
Copy link
Contributor

Kann ich auch machen. Aber in den 0035 mit rein, oder noch einen neuen Patch bauen?

@jens-maus
Copy link
Owner

Da es nicht ganz thematisch in 0035 passt würde ich vorschlagen einen frischen/neuen Patch dafür zu bauen.

@jp112sdl
Copy link
Contributor

einen frischen/neuen Patch dafür zu bauen.

Dürfen Lücken wiederverwendet werden, z.B. 0174 ?

jp112sdl added a commit to jp112sdl/RaspberryMatic that referenced this issue Apr 13, 2023
@jens-maus
Copy link
Owner

einen frischen/neuen Patch dafür zu bauen.

Dürfen Lücken wiederverwendet werden, z.B. 0174 ?

Besser nicht, wegen history. Können schon, aber wir sollten es vermeiden IMHO. Manchmal kann es notwendig sein um einen Patch vor dem anderen ausführen zu lassen (wenn es abhängigkeiten) gibt. Aber wenn keine Not ist würde ich sagen wir sollten immer nur neue generieren.

@jens-maus jens-maus linked a pull request Apr 13, 2023 that will close this issue
7 tasks
jens-maus added a commit that referenced this issue Apr 13, 2023
…ich validates system variable inputs to not allow to use reserved special characters (#2285, #2291, @jp112sdl)

Co-authored-by: Jens Maus <mail@jens-maus.de>
@jens-maus jens-maus moved this from To Do to Done in WebUI improvements/fixes Apr 15, 2023
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
Development

Successfully merging a pull request may close this issue.

4 participants