Fil commented 9 years ago

les URLs démarrant par http et https pourraient être identifiés comme une même adresse ; de même on pourrait virer plus de bêtises du type &utm÷ etc (liste à faire)

cf. par exemple http://seenthis.net/messages/404266#message405610

Fil commented 9 years ago

aussi virer:

mobile.
index.html

rastapopougros commented 8 years ago

Il y a aussi les encodages différents des accents. Cf http://seenthis.net/messages/434813

Fil commented 8 years ago

From: @intempestive

Aussi ici http://seenthis.net/messages/481247

au risque d'insister, ce serait bien de reconnaître les https / http, non ? voire les url d'éventuelles ancres qu'elles auraient (style http://site.net/article.html et http://site.net/article.html#ancre)

enfin j'dis ça j'dis rien :)

seenthis_suggestion

http://seenthis.net/messages/481316#message481319

brunob commented 8 years ago

Pour le coup du http vs https, il suffirait pas d'employer une fourberie sql dans cette requête de _creer_lien_riche() ?

https://github.com/seenthis/seenthis/blob/master/inc/traiter_texte.php#L147

Reste à trouver la fourberie en question, et je pense qu'il doit y avoir une autre occurrence quelque part pour la détection de doublon lors de l'ajout d'un nouveau billet (peut-être uniquement dans seenthis_importer_flux ?).

Fil commented 8 years ago

tu es sûr ? de mémoire les liens stockés dans spip_syndic c'est une vieillerie (qui pose d'ailleurs de gros problèmes vu qu'ensuite on en importe le contenu dans cette table) ; le stockage réellement utilisé pour ces liens est fait dans spip_me_tags

brunob commented 8 years ago

Non je ne suis pas sûr car je découvre tout ça :) Mais je vois que c'est bien dans cette fonction qu'on compte le nombre d'occurrences d'une url pour afficher ou non le famuex triangle devant les liens.

Fil commented 8 years ago

vérification faite il faut changer ce comptage pour tout appuyer sur spip_me_tags, exemple :

| id_me  | tag                                  | class
| 485062 | https://community.letsencrypt.org/   | url

Fil commented 7 years ago

et aussi les blogspot.[pays] https://seenthis.net/messages/529468#message529524

Fil commented 7 years ago

http/https demandé ici aussi https://seenthis.net/messages/563445#message563682

brunob commented 4 years ago

@Fil pour la modification du comptage afin de se baser sur spip_me_tags penses-tu que ce patch suffirait ?

diff --git a/inc/traiter_texte.php b/inc/traiter_texte.php
index c424045..959491e 100644
--- a/inc/traiter_texte.php
+++ b/inc/traiter_texte.php
@@ -180,8 +180,8 @@ function _creer_lien_riche($lien) {
    if ($id_syndic) {
        $query_total = sql_query("SELECT count(*) as c
            FROM spip_me
-           RIGHT JOIN spip_me_syndic ON spip_me_syndic.id_me=spip_me.id_me
-           WHERE spip_me_syndic.id_syndic=$id_syndic
+           RIGHT JOIN spip_me_tags ON spip_me_tags.id_me=spip_me.id_me
+           WHERE spip_me_tags.tag=$long
                AND spip_me.statut='publi'");
        include_spip('inc/urls');
        $url = generer_url_entite($id_syndic,'site');

Testé en local, la requête est valide et fonctionne.

Ça ne règle pas l'objet du ticket, mais ça permet d'avancer un peu sur le sujet.

brunob commented 4 years ago

Pour continuer, je pense que ça se joue dans inserer_tags_liens() pour la partie stockage en base cf :

https://github.com/seenthis/seenthis/blob/master/seenthis_options.php#L1048

Et dans action_messages_lien() pour la partie qui alerte l'utilisateur quand il poste :

https://github.com/seenthis/seenthis_squelettes/blob/master/action/messages_lien.php

À suivre...

brunob commented 4 years ago

Plus j'avance sur la question, et plus je me dis qu'il serait plus fin d'arrêter de stocker des doublons en base pour devoir gérer ces bourdes plus tard dans la code. Ne serait-il pas malin de stocker les urls sans leur protocole, ça permettrait de simplifier tout ça, mais ça aurait peut-être des effets de bord ?

rastapopougros commented 4 years ago

En théorie deux URL identifiques mais avec un protocole différent peuvent parfaitement rediriger vers deux pages totalement différentes, même pas sur le même serveur, enfin on fait ce qu'on veut.

En pratique, pour ce qui concerne le partage de lien d'actus ou ce genre, je ne vois pas de cas où on tomberait vraiment sur des choses différentes. Enfin pour http/https uniquement. Car oui on peut pas juste les stocker sans protocole : on peut très bien sur seenthis faire des liens vers ftp://, ou que sais-je comme autre protocole donc attention.

Fil commented 4 years ago

ama on est obligé de stocker le protocole car tous les sites ne sont pas https (parfois seul http fonctionne)

brunob commented 4 years ago

'k, du coup ça donne ce patch pas très propre pour la double détection http/https dans _creer_lien_riche(), je préfère vous le montrer ici, vous aurez peut-être une idée pour éviter ce double test qui me chagrine :

diff --git a/inc/traiter_texte.php b/inc/traiter_texte.php
index 959491e..2bc5e6b 100644
--- a/inc/traiter_texte.php
+++ b/inc/traiter_texte.php
@@ -149,8 +149,10 @@ function _creer_lien_riche($lien) {
        //list($width, $height) = @getimagesize($lien);
        //if (($width * $height) >= 300) return;
    }
-   
-   $query = sql_query("SELECT id_syndic, lang, titre, url_syndic, md5 FROM spip_syndic WHERE url_site=".sql_quote($lien));
+
+   // virer le http/https en début d'url + le slash final
+   $lien_flou = preg_replace(',/$,', '', preg_replace(',^(https?://)?,i', '', $lien));
+   $query = sql_query("SELECT id_syndic, lang, titre, url_syndic, md5 FROM spip_syndic WHERE url_site LIKE ".sql_quote('%' . $lien_flou)); 
    if ($row = sql_fetch($query)) {
        $id_syndic = $row["id_syndic"];
        $lang = $row["lang"];
@@ -178,10 +180,11 @@ function _creer_lien_riche($lien) {
    // Ne faire apparaître le lien_court
    // que si plusieurs billets referencent le lien
    if ($id_syndic) {
+       $long = preg_replace(',/$,', '', preg_replace(',^(https?://)?,i', '', $long));
        $query_total = sql_query("SELECT count(*) as c
            FROM spip_me
            RIGHT JOIN spip_me_tags ON spip_me_tags.id_me=spip_me.id_me
-           WHERE spip_me_tags.tag=$long
+           WHERE spip_me_tags.tag LIKE ". sql_quote('%' . $long) ."
                AND spip_me.statut='publi'");
        include_spip('inc/urls');
        $url = generer_url_entite($id_syndic,'site');

Pour info, ça ne fonctionne que pour la détection du doublon (affichage de la tite flèche donc).

Reste le problème du lien vers la page sites/xxx : celui-ci pointe vers le premier id_syndic remonté par la requête, on arrive donc sur une page qui n'affichera que les messages qui référence le lien en http par exemple (ceux qui référencent le lien en https sont liés à un autre id_syndic).

Pour améliorer ça, on pourrait modifier la page d'un site/xxx pour qu'elle affiche les messages qui référencent le lien en http et en https, mais ça commence à faire beaucoup de modifs, gros chantier en perspective...

brunob commented 4 years ago

Au final le chantier n'est pas si gros que je craignais, il suffit de patcher calculer_enfants_syndic() afin de lui faire renvoyer l'id_syndic de l'url en http et en https puisque c'est elle qui est utilisée pour construire la liste des messages liés à une url, cf ce patch :

diff --git a/seenthissq_fonctions.php b/seenthissq_fonctions.php
index 1c3ac30..c0f7142 100644
--- a/seenthissq_fonctions.php
+++ b/seenthissq_fonctions.php
@@ -30,9 +30,12 @@ function share_tw_url($id_me) {
 function calculer_enfants_syndic($id_syndic, $url_racine = '', $afficher_url = '', $ret = array()) {

    $ret[] = $id_syndic;
-// if (!$afficher_url) $afficher_url = $url_racine;

-       
+   // si on a la même url en http & https, ajouter le doublon au tableau de retour
+   $lien_flou = preg_replace(',/$,', '', preg_replace(',^(https?://)?,i', '', $url_racine));
+   if ($doublon = sql_getfetsel('id_syndic', 'spip_syndic', "id_syndic != $id_syndic AND url_site LIKE ".sql_quote('%' . $lien_flou))) {
+       $ret[] = $doublon;
+   }

    $query = sql_select("*", "spip_syndic", "id_parent=$id_syndic", /* group by */ '', /* order by*/ '', /* limit */ '0,100');
    $total = sql_count($query);

C'est pas super clair/propre comme solution, mais je ne vois pas comment faire mieux pour gérer un cas foireux à la base. J'ajoute ça à la PR #239 ou vous préférez une PR spécifique pour cette partie ?

brunob commented 4 years ago

Voilà, j'ai fait simple en complétant la PR existante.

brunob commented 4 years ago

Il ne restait plus que le cas de seenthis_importer_rss_article() dans le plugin d'import de flux RSS, je viens d'envoyer une PR pour ça.

brunob commented 4 years ago

@seenthis/commiters les deux PRs listées ici attendent toujours vos +1 ou -1 si c'est pourrite :p

Fil commented 4 years ago

le like '%lien_flou' me paraît un poil buggué (listes.rezo.net serait identique à rezo.net, mais rezo.net ne serait pas identique à listes.rezo.net) ?

brunob commented 4 years ago

@Fil ha oui merdum, moi qui souhaitait utliser like pour ne pas plomber les perfs, je crains qu'il ne faille jouer de la regexp :\

PS : à moins d'ajouter un not like à la requête genre and tag NOT LIKE '.sql_quote('%.' . $url)

brunob commented 4 years ago

Autre piste à base de replace sur l'url soumise et celle recherchée, pas très clean, mais ça fonctionne bien :

diff --git a/action/messages_lien.php b/action/messages_lien.php
index b7f0a74..89ee7d8 100644
--- a/action/messages_lien.php
+++ b/action/messages_lien.php
@@ -10,7 +10,7 @@ function action_messages_lien() {
    $url = rawurldecode(_request("url"));
    spip_log($url);
    $url_messages = array();
-   $id_possibles = sql_allfetsel('id_me', 'spip_me_tags', 'class = '.sql_quote('url').' and tag = '.sql_quote(preg_replace(',/$,', '', $url)));
+   $id_possibles = sql_allfetsel('id_me', 'spip_me_tags', 'class = '.sql_quote('url').' and REPLACE(REPLACE(tag,"http://",""),"https://","") = REPLACE(REPLACE('.sql_quote(preg_replace(',/$,', '', $url)).',"http://",""),"https://","")');
    $id_publies = sql_allfetsel(
        'id_me',
        'spip_me', array("statut = 'publi'", sql_in('id_me', array_map('array_pop', $id_possibles))));

Fil commented 4 years ago

Sinon il y a l'option (url = (http://…………) ) OR (url = (https://…………) ), peut-être plus rapide (et plus lisible) (?)

brunob commented 4 years ago

Ha mais oui bien sûr, ça donnerait ça en partant du patch que je proposais dans #239 :

diff --git a/action/messages_lien.php b/action/messages_lien.php
index 0603c6b..e393a62 100644
--- a/action/messages_lien.php
+++ b/action/messages_lien.php
@@ -12,7 +12,7 @@ function action_messages_lien() {
    $url_messages = array();
    // virer le http/https en début d'url + le slash final
    $url = preg_replace(',/$,', '', preg_replace(',^(https?://)?,i', '', $url));
-   $id_possibles = sql_allfetsel('id_me', 'spip_me_tags', 'class = '.sql_quote('url').' and tag LIKE '.sql_quote('%' . $url));
+   $id_possibles = sql_allfetsel('id_me', 'spip_me_tags', 'class = '.sql_quote('url').' and (tag = '.sql_quote('http://' . $url).' or tag = '.sql_quote('https://' . $url).')');
    $id_publies = sql_allfetsel(
        'id_me',
        'spip_me', array("statut = 'publi'", sql_in('id_me', array_map('array_pop', $id_possibles))));

brunob commented 4 years ago

Et voilà j'ai mis à jour les trois PRs avec ce pattern, à vos relectures :)

https://github.com/seenthis/seenthis_squelettes/pull/239/ https://github.com/seenthis/seenthis/pull/31/ https://github.com/seenthis/seenthis_importer_flux/pull/6/

Fil commented 4 years ago

4 ans et demi de travail acharné ! seenthis n'est pas mort !

brunob commented 4 years ago

Voilà c'est mergé et poussé en prod, merci pour les retours, est-ce qu'on laisse ouvert pour les autres "bêtises du type &utm÷ etc" ou on verra ça dans un éventuel autre ticket ?

Fil commented 4 years ago

normalement les utm sont "sucrés" (terminologie officielle) au bookmarklet, et peut-être même à l'enregistrement; s'il reste un souci je suggère de le mettre dans un autre ticket

brunob commented 4 years ago

Oui, on sucre déjà les utm cf https://github.com/seenthis/seenthis/blob/7e83816d6d369a31bc3d05a257ee4fabe27f549f/inc/traiter_texte.php#L199

brunob commented 4 years ago

Hmm problème signalé ici https://seenthis.net/messages/820186

D'après les logs on trouve bien un id_syndic depuis https://github.com/seenthis/seenthis/blob/master/inc/traiter_texte.php#L155 et ça renvoie :

(
    [id_syndic] => 1752062
    [lang] => 
    [titre] => 
    [url_syndic] => 
    [md5] => faa74724bfdbf296f4c4800d865a4e4f
)

Et comme url_syndic n'est pas renseigné, on ne peuple pas $long ce qui fait foirer la détection de doublon. Je ne m'explique pas comment ça se fait qu'on a des données de ce type en base, mais il y a pas mal comme ça...

MariaDB [stdev]> select count(*) from spip_syndic where url_syndic ='';
+----------+
| count(*) |
+----------+
|   829377 |
+----------+
1 row in set (20.28 sec)

brunob commented 4 years ago

On dirait bien que ça quelques effets de bord sur la détection des doublons :\

https://seenthis.net/messages/829080#message829384

brunob commented 4 years ago

Encore un exemple avec https://seenthis.net/sites/7282461336007514739 posté ici https://seenthis.net/messages/858725 et là https://seenthis.net/messages/859003

Si je tente de poster un message avec l'url en question, elle est bien détectée, le problème ne porterait donc que sur la "tite flèche" qui reste blanche devant le lien.

brunob commented 4 years ago

Pour revenir là dessus, ça fonctionne bien pour la détection lors de la rédaction d'un message (qui cherche dans la table spip_me_tags), par contre ça déconne pour _creer_lien_riche() et peut-être aussi calculer_enfants_syndic() qui cherchent dans spip_syndic.

De mon côté, si je teste en local, le doublon est bien détecté dans tous les cas, mais chez moi le champ url_syndic est bien renseigné pour les "sites" en question, ce qui n'est pas toujours le cas sur seenthis.

Après d'autres recherche, je pense avoir trouvé l'origine du problème dans https://github.com/seenthis/seenthis/commit/95f3d8a2d01f00a33edc17442e947613b6e1f54d#diff-47d0bd8b76a27259aac327027e571c56 où j’introduisais l'usage de $long pour compter les occurrences du lien, alors qu'il suffirait de réutiliser $lien_flou pour que ça fonctionne (testé et approuvé) cf :

diff --git a/inc/traiter_texte.php b/inc/traiter_texte.php
index c0763a9..249d82c 100644
--- a/inc/traiter_texte.php
+++ b/inc/traiter_texte.php
@@ -180,11 +180,10 @@ function _creer_lien_riche($lien) {
    // Ne faire apparaître le lien_court
    // que si plusieurs billets referencent le lien
    if ($id_syndic) {
-       $long = preg_replace(',/$,', '', preg_replace(',^(https?://)?,i', '', $long));
        $query_total = sql_query("SELECT count(*) as c
            FROM spip_me
            RIGHT JOIN spip_me_tags ON spip_me_tags.id_me=spip_me.id_me
-           WHERE (spip_me_tags.tag = ".sql_quote('http://' . $long)." OR spip_me_tags.tag = ".sql_quote('https://' . $long).")
+           WHERE (spip_me_tags.tag = ".sql_quote('http://' . $lien_flou)." OR spip_me_tags.tag = ".sql_quote('https://' . $lien_flou).")
                AND spip_me.statut='publi'");
        include_spip('inc/urls');
        $url = generer_url_entite($id_syndic,'site');

seenthis / seenthis_squelettes

URLs identiques #130

seenthis_suggestion