Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

True\Punycode auf Version 2 heben #8693

Closed
BugBuster1701 opened this issue Apr 5, 2017 · 18 comments
Closed

True\Punycode auf Version 2 heben #8693

BugBuster1701 opened this issue Apr 5, 2017 · 18 comments
Assignees
Labels
Milestone

Comments

@BugBuster1701
Copy link
Contributor

BugBuster1701 commented Apr 5, 2017

Aus der Changelog:

    [Enhancement] PHP 7 support
    [Fix] Renamed True namespace to TrueBV as it is a reserved word in PHP 7

Achtung, der Namespace ändert sich, daher müsste hier dieser angepasst werden:
https://github.com/contao/core/blob/master/system/modules/core/library/Contao/Idna.php#L13

in
use TrueBV\Punycode;

in composer.json dann
true/punycode:~2.0

Betrifft auch Contao 4, siehe contao/core-bundle#748

Nachtrag: Der PHP 7 Grund ist wohl nicht eindeutig, aber die Aktualisierung wäre trotzdem wünschenswert. Es gibt auch weitere Bugfixe wie "Cannot decode domain in uppercase"

@ausi
Copy link
Member

ausi commented Apr 5, 2017

Bei mir läuft True\Punycode ohne Probleme in PHP 7. Namespaces sind von reserved words nicht betroffen soweit ich weiß.

@BugBuster1701
Copy link
Contributor Author

Steht hier anders:
http://php.net/manual/de/reserved.other-reserved-words.php

Wie im anderem Ticket schon geschrieben, benötigen andere Libs bereits die 2.x von Punycode, in Contao 3 nicht ganz das Problem wegen der getrennten composer.json, aber in Contao 4 dann ein Problem wegen Konflikte. Auf die aktuelle Version zu gehen wäre daher sinnvoll denke ich.

@ausi
Copy link
Member

ausi commented Apr 5, 2017

Dann ist vermutlich die PHP-Dokumenation falsch. Ein kurzer Test funktioniert in allen PHP-Versionen von 5.6.0 bis 7.1.3: https://3v4l.org/LCEX3

@BugBuster1701
Copy link
Contributor Author

Hatte mich auch schon gewundert, da ich auch deiner Meiung war bisher.
Aber ich bleibe dabei, eine Aktualisierung wäre trotzdem sinnvoll, auch wenn das bedeutet eine Zeile im Code anpassen zu müssen. Es gibt ja auch noch weitere Bugfixe in Punycode.

@BugBuster1701 BugBuster1701 changed the title True\Punycode auf Version 2 heben wegen PHP7 True\Punycode auf Version 2 heben Apr 5, 2017
@leofeyer leofeyer added the defect label Apr 5, 2017
@leofeyer leofeyer added this to the 3.5.26 milestone Apr 5, 2017
@leofeyer leofeyer self-assigned this Apr 18, 2017
@leofeyer
Copy link
Member

leofeyer commented Apr 18, 2017

Geändert in f8633ac.

@leofeyer
Copy link
Member

Ich habe die Änderungen aufgrund von #8704 in 87d76ad zurückgenommen.

@BugBuster1701
Copy link
Contributor Author

Planst du die Rücknahme auch für Contao 4?
Das wäre ein Aus für meine Erweiterungen, da wie gesagt durch eine Drittanbeiter Lib über Composer die Punycode 2.x verlangt wird.

@leofeyer
Copy link
Member

Für Contao 4.3 werden wir auf jeden Fall wieder Version 1 nutzen. Du hast ja gesehen, wie viele Probleme die Änderung gemacht hat!

Für Contao 4.4 werde ich versuchen, die Version 2 einzubauen.

@BugBuster1701
Copy link
Contributor Author

Warum wird die Youtube ID eigentlich als URL geprüft? Ist doch gar keine?

@fritzmg
Copy link
Contributor

fritzmg commented Apr 25, 2017

Warum wird die Youtube ID eigentlich als URL geprüft? Ist doch gar keine?

In Contao 4 kann die YouTube ID aus der URL extrahiert werden (in einem DCA save_callback). In Contao 3 ist das aber noch nicht der Fall. Aber selbst dort ist 'rgxp' => 'url' definiert.

@BugBuster1701
Copy link
Contributor Author

Fehler? Oder gibts ein Grund dafür?

@Znrl
Copy link

Znrl commented Apr 25, 2017

Zumindest bzgl. Youtube ist das kein Fehler von Punycode.
Der Path-Teil und Query-Strings dürfen nicht von Punycode encoded werden, hier wäre urlencode() richtig.
So wie das hier gemacht wird:
https://www.punycoder.com/
Das heißt es muss die Eingabe sowieso geprüft und die (potentiellen) URI Teile seperat bearbeitet werden.

@BugBuster1701
Copy link
Contributor Author

BugBuster1701 commented Apr 25, 2017

Punycode verlangt ja vom Nutzer, das nur der Host Anteil übergeben wird. Das wird bei normalen URLs auch von Contao richtig getan (über Idna::encodeUrl)
Durch einen damaligen Fehler in parse_url, soll in 5.4.7 behoben sein, wurde ein Schema vorangestellt wenn kein Schema vorhanden ist. Was bei Eingabe nur der youtubeId der Fall ist.
Damit entsteht: https://youtubeId, woraus parse_url dann scheinbar die youtubeId als Host rausfiltert.
Daher ja auch der Workaround im Forum das 'rgxp' => 'url' im DCA beim Youtubefeld rauszunehmen.

@Znrl
Copy link

Znrl commented Apr 25, 2017

Dann könnte man das für Contao 4 ja ignorieren, da hier eh PHP >=5.5 vorausgesetzt wird und damit Contao selbst kein HTTP(S) voanstellen müsste.
In Contao 3.5 sieht das anders aus, wegen PHP >=5.4.0

@leofeyer
Copy link
Member

Durch einen damaligen Fehler in parse_url, soll in 5.4.7 behoben sein, wurde ein Schema vorangestellt wenn kein Schema vorhanden ist

Das ist kein "damaliger Fehler" in parse_url. Contao und nicht PHP stellt das Schema voran, weil parse_url sonst unvollständige URLs als path behandelt – was übrigens korrekt ist. Insofern müssen wir die Idna-Klasse anpassen, um das Problem zu beheben.

@BugBuster1701
Copy link
Contributor Author

Ah, OK, da habe ich mich von PHP Handbuch in die Irre leiten lassen und nicht weiter nach geprüft.

@BugBuster1701
Copy link
Contributor Author

Bin gespannt wie du in der Idna Klasse erkennen willst, das es sich nicht um eine unvollständige URL handelt sondern um eine Youtube-ID.

jsonn pushed a commit to jsonn/pkgsrc that referenced this issue Apr 25, 2017
# Contao calendar bundle change log

### 4.3.8 (2017-04-24)

 * Correctly use the en dash in the calendar modules (see contao/core#8690).
 * Correctly support 64 character template names everywhere (see contao/core#6819).

# Contao comments bundle change log

### 4.3.8 (2017-04-24)

 * Correctly support 64 character template names everywhere (see contao/core#6819).

# Contao core bundle change log

### 4.3.9 (2017-04-25)

 * Revert the Punycode library changes (see contao/core#8693).

### 4.3.8 (2017-04-24)

 * Inline small images in protected folders in the file manager (see #636).
 * Correctly encode the URL in the DataContainer::switchToEdit() method (see #762).
 * Fix the parent view drag and drop in Firefox (see #666).
 * Correctly display the search results in the extended tree view (see #739).
 * Update the Punycode library to version 2 (see #748).
 * Fix the "delete file" button for non-admin users (see #764).
 * Prevent endless loops in the book navigation module (see contao/core#8665).
 * Limit the maximum size of dimensionless SVGs in the back end (see contao/core#8684).
 * Correctly support 64 character template names everywhere (see contao/core#6819).
 * Remove the UTF-8 BOM when combining files (see contao/core#8689).
 * Correctly move folders with an "@" in their name (see contao/core#8674).
 * Correctly redirect to the last page visited upon login (see contao/core#8632).

### 4.3.7 (2017-03-23)

 * Check the database connection in the WebsiteRootsConfigProvider class.
 * Fix the %2B conversion in the Controller::addToUrl() method.

# Contao listing bundle change log

### 4.3.8 (2017-04-24)

 * Correctly support 64 character template names everywhere (see contao/core#6819).

# Contao news bundle change log

### 4.3.8 (2017-04-24)

 * Correctly support 64 character template names everywhere (see contao/core#6819).

# Contao newsletter bundle change log

### 4.3.8 (2017-04-24)

 * Correctly support 64 character template names everywhere (see contao/core#6819).
@leofeyer
Copy link
Member

Genau das ist das Problem. Deswegen müssen wir überall wo wir 'rgxp'=>'url' oder die Idna-Klasse verwenden prüfen, was genau wir übergeben.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants