Author: esj@rezo.net
Date: 2009-08-11 09:06:30 +0200 (mar, 11 aoû 2009)
New Revision: 14380
Log:
Retrait du dernier cas où le fond était passé dans le contexte. Ce paramètre d'URL est donc maintenant comme les autres.
Modified:
spip/ecrire/inc/utils.php
spip/ecrire/public.php
spip/ecrire/public/assembler.php
Bonjour,
Je comprends pas pourquoi on change *encore* l'api de recuperer_fond :
Avant le premier argument $fond etait prioritaire, mais $contexte['fond'] etait pris si $fond était non fourni.
Ca a été changé en cours de dev de la 2.0, $contexte['fond'] est devenue prioritaire, ecrasant $fond dans tous les cas
(me cassant quelques formulaires au passage qui avaient le malheur d'avoir un <input name='fond'../>)
Maintenant on revient en arrière, en remettant $fond prioritaire, mais en ignorant complètement $contexte['fond'].
Du coup, on casse tous les appels avec $fond='', comme ici, qui ont pourtant toujours marché :
Est-ce bien raisonnable que de casser cette fonction à chaque version ?
Il ne faudrait pas oublier qu'elle ne sert pas que dans les squelettes compilés, mais a plein de plugins aussi.
Je pense qu'il faut rétablir la prise en compte de $contexte['fond'] quand $fond est vide, avec le test sécuritaire pour s'assurer que sa valeur ne vient pas directement de l'url.
Cédric
Le 11 août 09 à 09:06, esj@rezo.net a écrit :
Author: esj@rezo.net
Date: 2009-08-11 09:06:30 +0200 (mar, 11 aoû 2009)
New Revision: 14380Log:
Retrait du dernier cas où le fond était passé dans le contexte. Ce paramètre d'URL est donc maintenant comme les autres.Modified:
spip/ecrire/inc/utils.php
spip/ecrire/public.php
spip/ecrire/public/assembler.phpDetails: http://trac.rezo.net/trac/spip/changeset/14380
_______________________________________________
spip-commit@rezo.net - http://listes.rezo.net/mailman/listinfo/spip-commit
dev: http://trac.rezo.net/trac/spip/
Le 11 août 09 à 09:06, esj@rezo.net a écrit :
Author: esj@rezo.net
Date: 2009-08-11 09:06:30 +0200 (mar, 11 aoû 2009)
New Revision: 14380Log:
Retrait du dernier cas où le fond était passé dans le contexte. Ce paramètre d'URL est donc maintenant comme les autres.
Suite à ce retrait, je réalise que la ligne suivante extraite de la fonction inclure_modele:
$GLOBALS['marqueur_notes'] = substr(md5(serialize($contexte)),0,8);
peut calculer la même chose pour des modèles diférent. Je pourrais le rétablir facileement
mais je suis perplexe: pourquoi faire ça pour les modèles et pas pour toutes les inclusions ?
D'autant que l'inverse m'aurait moins surpris: les inclusions manipulent des données différentes, on peut concevoir que les groupes de notes soient traitées indépendamment (on inclut 3 articles, chacun a ses notes 1, 2, 3.... etc, ok), mais un modèle étant appelé dans une donnée textuelle en base, du coup tous les modèles inclus dans cette donné vont avoir des notes de même numéro.
Tout ça ne me semble pas cohérent, il me semble qu'il vaudrait mieux mettre ça au niveau de inclure_page.
Committo,Ergo:Sum
Le 11 août 09 à 11:34, cedric.morin@yterium.com a écrit :
Bonjour,
Je comprends pas pourquoi on change *encore* l'api de recuperer_fond :
Avant le premier argument $fond etait prioritaire, mais $contexte['fond'] etait pris si $fond était non fourni.
Ca a été changé en cours de dev de la 2.0, $contexte['fond'] est devenue prioritaire, ecrasant $fond dans tous les cas
(me cassant quelques formulaires au passage qui avaient le malheur d'avoir un <input name='fond'../>)Maintenant on revient en arrière, en remettant $fond prioritaire, mais en ignorant complètement $contexte['fond'].
Du coup, on casse tous les appels avec $fond='', comme ici, qui ont pourtant toujours marché :
Connexion · GitLabEst-ce bien raisonnable que de casser cette fonction à chaque version ?
Il ne faudrait pas oublier qu'elle ne sert pas que dans les squelettes compilés, mais a plein de plugins aussi.Je pense qu'il faut rétablir la prise en compte de $contexte['fond'] quand $fond est vide, avec le test sécuritaire pour s'assurer que sa valeur ne vient pas directement de l'url.
Comme ici,
http://trac.rezo.net/trac/spip/browser/branches/spip-1.9.2/ecrire/public/assembler.php#L394
mais avec en plus la sécurité enlevée dans [14380]
Cédric
Le 11 août 09 à 11:34, cedric.morin@yterium.com a écrit :
Bonjour,
Je comprends pas pourquoi on change *encore* l'api de recuperer_fond :
Il faut que les fonctions les plus importantes de SPIP soient
faciles à comprendre et à décrire, et en particulier ne dépendent
pas de leur contexte d'exécution dans leur comportement.
La modification qui a consisté à tester _request('fond')
pour des raisons de sécurité était une mauvaise solution,
parce qu'elle amène une ambiguité dans la sémantique de la fonction,
son comportement dépendant du contexte à chaque utilisation.
Se plaindre que "on" change encore ne me semble pas recevable,
"on" étant toi en l'occurrence, et a contrario le fait qu'il a fallu
revenir autant de fois sur cette fonction prouve bien que cette ambiguité
est fondamentalement problématique. En la faisant sauter, on élimine la source
de ces problèmes récurrent, d'autant que je ne vois pas l'avantage de cette écriture.
Je pense qu'elle n'avait qu'une seule origine: la petite difficulté consistant à compiler à part
le paramètre "fond". Maintenant que ce code est écrit, cette licence n'a plus aucune raison d'être.
Committo,Ergo:Sum
Le 11 août 09 à 11:58, Committo,Ergo:sum a écrit :
Se plaindre que "on" change encore ne me semble pas recevable,
"on" étant toi en l'occurrence,
Non mais c'est une blague ?
La fonction a été cassée 2 fois lors du process de developpement de la 2.0 :
- une première fois en étant sucrée au profit de evaluer_fond qui avait une autre API
puis après discussion, ralerie, débat et retour au support de l'api initiale (tout en permettant les extensions proposées),
- une seconde fois en imposant $contexte['fond'] comme argument prioritaire (j'ai rien dit et je me suis contenté de me prendre les bugs dans la figure dans mon coin, sans vraiment voir la raison de ce changement, au passsage).
Je ne suis pour rien dans ces changements intempestifs d'API qu'on se prend dans la figure à chaque fois, sans en voir ni comprendre la nécéssité *absolue* et *vitale* qui justifie de changer l'API et de passer notre temps à reprendre notre code pour suivre une spécification qui évolue au jour le jour.
En l'occurence, l'implémentation actuelle reposant sur la prise en compte prioritaire de
$contexte['fond']
comportait une lacune sécuritaire dans le cas d'un appel du type
recuperer_fond($fond,$_GET);
comme indiquée dans les commentaires.
Je n'ai donc pas modifié la fonction pour mon plaisir, mais simplement pour boucher un trou de sécurité potentiel.
Que tu prenne ce correctif comme pretexte pour casser encore l'API, revenir en arrière sur le choix de cette priorité et, plus fort encore, supprimer complètement la prise en compte de $contexte['fond'] qui est pourtant là depuis la version 1.9.2, cassant au passage tous les appels concernés me parait un peu dur.
et a contrario le fait qu'il a fallu
revenir autant de fois sur cette fonction prouve bien que cette ambiguité
est fondamentalement problématique.
Je ne te le fais pas dire. La version de départ marchait bien et était un bon compromis....
En la faisant sauter, on élimine la source
de ces problèmes récurrent, d'autant que je ne vois pas l'avantage de cette écriture.
Je pense qu'elle n'avait qu'une seule origine: la petite difficulté consistant à compiler à part
le paramètre "fond". Maintenant que ce code est écrit, cette licence n'a plus aucune raison d'être.
Je répète, cette fonction existait avant son utilisation par le compilateur, avec cette API, parce qu'elle correspond à un besoin.
La changer parce que le compilateur n'a plus besoin de cette partie est pour le moins cavalier.
Cédric
Suite à ce retrait, je réalise que la ligne suivante extraite de la fonction
inclure_modele:$GLOBALS\['marqueur\_notes'\] = substr\(md5\(serialize\($contexte\)\),0,8\);peut calculer la même chose pour des modèles diférent. Je pourrais le
rétablir facileement
mais je suis perplexe: pourquoi faire ça pour les modèles et pas pour toutes
les inclusions ?
utilise svn praise ecrire/public/assembler.php
et tu verras que c'est documenté
http://trac.rezo.net/trac/spip/changeset/7105
Le problème est de savoir gérer proprement une note dans le descriptif
d'un document.
-- Fil
Le 11 août 09 à 12:16, cedric.morin@yterium.com a écrit :
Je ne suis pour rien dans ces changements intempestifs d'API qu'on se prend dans la figure à chaque fois, sans en voir ni comprendre la nécéssité *absolue* et *vitale* qui justifie de changer l'API et de passer notre temps à reprendre notre code pour suivre une spécification qui évolue au jour le jour.
Ce que tu ne vois pas c'est qu'il y a un autre écueil aux changements d'API, c'est l'accumulation de fonctions et d'arguments qui font presque la même chose mais pas tout à fait. Pour assurer la compatilité, c'est du vite fait du mal fait, et du coup pour le nouveau venu ça fait que SPIP est une usine à gaz indermédable où rien n'est jamais simple et aucune vision d'ensemble n'est possible. Pour moi, tout le pb a commencé parce qu'on été introduite 2 fonctions presqu'identiques, evaluer_fond et recuperer_fond. J'ai ensuite essayé d'unifier ça dans tous les cas d'utilisations possibles, malheureusement le mal était fait et c'est parti dans tous les sens.
Ce scénario s'est produit souvent, et d'habitude je la ferme, si j'interviens pour recuperer_fond c'est qu'en plus c'est la fonction à présent centrale de SPIP et j'ai besoin qu'elle ait une sémantique claire pour que le débusqueur puisse faire une analyse du code sans ambiguité. J'ajoute que ce "changement d'API" se limite à une restriction (donner toujours un premier argument non vide, "fond" devant être absent du contexte) ce qui veut dire qu'on n'a pas à maintenir plusieurs versions des plugins, il existe une écriture commune, qui est de surcroît la plus intuitive.
Committo,Ergo:Sum
Le 11 août 09 à 12:56, Fil a écrit :
mais je suis perplexe: pourquoi faire ça pour les modèles et pas pour toutes
les inclusions ?utilise svn praise ecrire/public/assembler.php
et tu verras que c'est documenté
http://trac.rezo.net/trac/spip/changeset/7105Le problème est de savoir gérer proprement une note dans le descriptif
d'un document.
Ca ne répond pas à ma question: j'avais bien compris que ça permettait de ne pas mélanger les notes d'une donnée en base et celles des modèles qu'elle contient, mais je m'étonne qu'on ne fasse pas ça avec toutes les inclusions.
Committo,Ergo:Sum
Le 11/08/2009 13:13, Committo,Ergo:sum a écrit :
il existe une écriture commune, qui est
de surcroît la plus intuitive.
En attendant ?exec=controle_forum ne me liste plus les messages en 2.1 et plugin forum depuis [14380] comme le fait remarquer Cédric.
En même temps, je trouve plus clair que la valeur de $contexte n'intervienne en rien dans le choix du fond.
Utiliser recuperer_fond('', $contexte) était tout de même pas très… affordant ^^.
--
MM.
contient, mais je m'étonne qu'on ne fasse pas ça avec toutes les inclusions.
intervenir ici s'était avéré suffisant
-- Fil
Le 11 août 09 à 13:33, Fil a écrit :
contient, mais je m'étonne qu'on ne fasse pas ça avec toutes les inclusions.
intervenir ici s'était avéré suffisant
Soit, mais de nouveau il faut généraliser le problème. Est-ce qu'on dit qu'en cas d'inclusion les notes se trouve en bas du morceau inclus, ou bien est-ce qu'on dit ça dépend du type d'inclusion, et c'est d'autant plus galère que les modèles sont aussi appelable par la balise #MODELE, bonjour le pataquès. Ca me semblerait beaucoup plus simple de reporter tout ça au niveau de inclure_page.
Committo,Ergo:Sum
Le 11 août 09 à 13:29, Matthieu Marcillaud a écrit :
En attendant ?exec=controle_forum ne me liste plus les messages en 2.1 et plugin forum depuis [14380] comme le fait remarquer Cédric.
En même temps, je trouve plus clair que la valeur de $contexte n'intervienne en rien dans le choix du fond.
Utiliser recuperer_fond('', $contexte) était tout de même pas très… affordant ^^.
Ceci règle le pb:
Committo,Ergo:Sum
Soit, mais de nouveau il faut généraliser le problème.
fais donc
Est-ce qu'on dit
qu'en cas d'inclusion les notes se trouve en bas du morceau inclus, ou bien
est-ce qu'on dit ça dépend du type d'inclusion, et c'est d'autant plus
galère que les modèles sont aussi appelable par la balise #MODELE, bonjour
le pataquès.
je ne crois pas qu'on puisse fonctionner rationnellement avec des
notes qui sortiraient des #INCLURE pour se reporter dans les #NOTES
appelantes ; de plus ça serait contradictoire avec le fait que
#INCLURE est équivalent à <INCLURE>
Ca me semblerait beaucoup plus simple de reporter tout ça au
niveau de inclure_page.
tant que ça marche ça me va
-- Fil
Le 11 août 09 à 14:48, Fil a écrit :
Soit, mais de nouveau il faut généraliser le problème.
fais donc
Est-ce qu'on dit
qu'en cas d'inclusion les notes se trouve en bas du morceau inclus, ou bien
est-ce qu'on dit ça dépend du type d'inclusion, et c'est d'autant plus
galère que les modèles sont aussi appelable par la balise #MODELE, bonjour
le pataquès.tant que ça marche ça me va
Examen fait, j'ai eu la surprise de découvrir cet enchaînement
dans inclure_modele: $GLOBALS['marqueur_notes'] = substr(md5(serialize($contexte)),0,8);
dans calculer_notes (appelée in fine par la précedente): $GLOBALS["marqueur_notes"] ++;
Un bug ? Mais PHP a plus d'une bourde dans ses specs:
si ça valait b3166435 ++ donne b3166436.
L'opérateur ++ marcherait sur les hexa ? Bah non:
si ça valait ffffffff ++ donne fffffffg
et le plus beau:
si ca valait zzzzzzzz ++ donne aaaaaaaa
Je n'invente rien.
Committo,Ergo:Sum