[PATCH] HttpUtils.pm - Besseres Debugging für Header + Auth

Begonnen von Loredo, 20 Februar 2017, 15:58:37

Vorheriges Thema - Nächstes Thema

Loredo

Um HttpUtils etwas besser debuggen zu können, habe ich einige Zusätze eingebaut, um sich die Request- und Response Header mitloggen zu lassen.

Dafür wird Loglevel+1 verwendet, so dass bestehende Setups nicht beeinflusst werden sollten.

Außerdem habe ich eine weitere Option "hideauth" analog zu "hideurl" eingebaut. Somit kann man sich die URL weiterhin anzeigen lassen und nur die Authentifizierungs-Infos werden versteckt. hideurl=1 impliziert hideauth=1.
Authentifizierungs-Infos können auch direkt im Hash übergeben werden.

Ich habe den Patch mit einigen meiner Modulen getestet und konnte keine Probleme feststellen.
Es wäre prima, wenn er so übernommen werden könnte.



Gruß
Julian
Hat meine Arbeit dir geholfen? ⟹ https://paypal.me/pools/c/8gDLrIWrG9

Maintainer:
FHEM-Docker Image, https://github.com/fhem, Astro(Co-Maintainer), ENIGMA2, GEOFANCY, GUEST, HP1000, Installer, LaMetric2, MSG, msgConfig, npmjs, PET, PHTV, Pushover, RESIDENTS, ROOMMATE, search, THINKINGCLEANER

rudolfkoenig

Der Patch ist mir zu kompliziert. Da ich der Maintainer bin, werde ich nicht etwas annehmen, was ich nicht leicht verstehe, ist mir leider schon mit anderen Modulen so passiert, und ich habe beschlossen, daraus zu lernen.

Mit hideauth (und hideurl) habe ich auch ein Problem: debugging ist nicht immer fuer den Modulentwickler, und manchmal will man das exakte URL sehen. Wenn der Benutzer Passwoerter verstecken will, dann soll er sie entfernen. Ja, ich weiss, dass er dazu nicht immer in der Lage ist, dann muss das Problem halt anders geloest werden.

Ich will aber nicht nur bockig sein, und habe das dumpen der HTTP Header eingebaut: bei global verbose 5 wird der HTTP Header ausgegeben, sowhl request wie auf response.

Loredo

#2
Wirklich sehr schade. Ich hatte mir extra sehr viel Mühe gegeben das übersichtlich und gleichzeitig so kompakt wie möglich zu schreiben. Letzteres ist sonst nicht meine Art, sondern eigentlich eher deine; hat nun wohl den gegenteiligen Effekt gehabt.

Ich will aber nicht nur bockig sein und sage trotzdem erstmal Danke!



Allerdings:

Die Logfile Ausgabe ist nun inkonsistent und nicht so schön, wie ich sie vorgesehen hatte (so wie vorher bei Zero-Header-Length Datumstext wie bei einzelnen Zeilen auch). Nun ist nichtmal der Header eingerückt.
Auch hatte ich bei einer Log-Ausgabe den fehlenden Präfix "HttpUtils " gefixt.
Außerdem wird der Response code mit "HTTP response code" geloggt, die Header jedoch nur mit "request header" resp. "response header". Auch hier hatte ich auf die Konsistenz geachtet.

Außerdem ist die Logik eine gänzlich andere. Nun muss ich global verbose=5 setzen, was ich extrem unglücklich finde.
Bei meiner Lösung kann der Modulautor durch setzen von "loglevel" selbst entscheiden, ab welchem global verbose Level geloggt werden soll (und das ggf. auch noch per Attribut für den Benutzer konfigurierbar machen, wie es viele Module tun).

Zitat von: rudolfkoenig am 20 Februar 2017, 18:33:59
Mit hideauth (und hideurl) habe ich auch ein Problem: debugging ist nicht immer fuer den Modulentwickler, und manchmal will man das exakte URL sehen.

Tatsächlich habe ich den Patch genau aus diesem Grund geschrieben. Momentan kann man als Modulautor nur entscheiden die URL gar nicht anzuzeigen oder voll.
Übergibt man die Benutzerdaten nicht per URL, sondern per Hash, schützt man diese zwar vor der Ausgabe. Allerdings hat man dann keine Möglichkeit mehr zu erkennen, ob bei der Anfrage eine Benutzerkennung mitgeschickt wurde (außer jetzt global verbose=5 zu setzen...). Als Beispielmodul kann ich 70_PHTV anführen.

Dieser Patch separiert die Ausgabe der Benutzerdaten von der URL.
Der Modulautor steuert (wie bisher) über hideurl, ob die Daten komplett anonymisiert werden sollen. hideauth ist die abgeschwächte Form und versteckt tatsächlich nichts weiter als die reinen Benutzerdaten.

Zitat von: rudolfkoenig am 20 Februar 2017, 18:33:59
Wenn der Benutzer Passwoerter verstecken will, dann soll er sie entfernen. Ja, ich weiss, dass er dazu nicht immer in der Lage ist, dann muss das Problem halt anders geloest werden.

Es war schon immer die Entscheidung des Modulautors, die Einstellung hideurl=1 fest einzubauen oder über ein eigenes Geräteattribut für den Benutzer änderbar zu machen. Damit, dass Benutzer etwas verstecken wollen, hat das nichts zu tun. Es sind die Modulautoren, die diese Entscheidung treffen, denn der Standardwert ist 0.

Zitat von: rudolfkoenig am 20 Februar 2017, 18:33:59
Der Patch ist mir zu kompliziert. Da ich der Maintainer bin, werde ich nicht etwas annehmen, was ich nicht leicht verstehe, ist mir leider schon mit anderen Modulen so passiert, und ich habe beschlossen, daraus zu lernen.

Das verstehe ich.
Kann ich etwas tun, damit der Patch für dich leichter verständlich wird? Ich kann Logfiles bei hideurl=0, hideurl=1 und hideauth=1 anfertigen, wenn es dir hilft.
Soll ich mehrere Patches daraus machen?

Mir hilft dabei immer eine Gegenüberstellung in einer Zweifenster-Darstellung (z.B. FileMerge aus den XCode Tools unter macOS; ruft Versions.app bei mir automatisch auf).



Gruß
Julian






PS: Beiliegender Patch anonymisiert bei Digest Auth nur noch url und username.
Hat meine Arbeit dir geholfen? ⟹ https://paypal.me/pools/c/8gDLrIWrG9

Maintainer:
FHEM-Docker Image, https://github.com/fhem, Astro(Co-Maintainer), ENIGMA2, GEOFANCY, GUEST, HP1000, Installer, LaMetric2, MSG, msgConfig, npmjs, PET, PHTV, Pushover, RESIDENTS, ROOMMATE, search, THINKINGCLEANER

rudolfkoenig

ZitatNun ist nichtmal der Header eingerückt.
Stimmt, aber sehr gut im Log erkennbar.

ZitatAußerdem wird der Response code mit "HTTP response code" geloggt, die Header jedoch nur mit "request header" resp. "response header".
Stimmt, finde aber response code (ohne HTTP) nicht verstaendlich, der "response header" aber schon.

ZitatAußerdem ist die Logik eine gänzlich andere. Nun muss ich global verbose=5 setzen, was ich extrem unglücklich finde.
Sehe ein, dass auf global zu pruefen verwirrend ist, habe deswegen auf $hash->{loglevel}+1 geaendert.
Im Normalbetrieb (verbose 3) will man keine Ausgaben sehen. Bleibt also 4 oder 5. Da man verbose nur bis 5 waehlen kann, muss $hash->{loglevel} 4 sein, sonst wird httpheader nicht mehr ausgegeben. Weiss auch nicht mehr, wieso ich diese Freiheit dem Modulautor gelassen habe, man kann damit nur Unsinn machen.