Skip to content

refactor(FILTER_SANITIZE_STRING): use new arg for SecurityController::filterInput and make the update work#1163

Merged
mrflos merged 3 commits intoYesWiki:doryphore-devfrom
J9rem:fix/const-sanitize-string
May 26, 2024
Merged

refactor(FILTER_SANITIZE_STRING): use new arg for SecurityController::filterInput and make the update work#1163
mrflos merged 3 commits intoYesWiki:doryphore-devfrom
J9rem:fix/const-sanitize-string

Conversation

@J9rem
Copy link
Contributor

@J9rem J9rem commented May 17, 2024

Contexte:

  • je me rends compte qu'avec php 8.3 et peut-être d'autres versions plus anciennes, l'utilisation de la constante : FILTER_SANITIZE_STRING crée un warning intempestif.

Propositionn:

  • définition d'une nouvelle constante FILTER_SANITIZE_STRING_BKUP de même valeur
  • ajout d'un argument à la méthode SecurityController::filterInput pour émuler le comportement de cette option
  • corrections de 2 bugs qui étaient présents dans la définition des noms de classes

@mrflos
Copy link
Contributor

mrflos commented May 17, 2024

Hello je ne suis pas chaud pour supporter une constante que personne d'autre dans le monde utilise...
Si j'en croies la doc :
image
C'est plutot htmlspecialchars qui ferai office de remplacement

Copy link
Contributor

@mrflos mrflos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plutot passer par htmlspecialchars que de passer par la methode qui appelle cette constante, ou changer le comportement de filterInput pour lui dire quoi faire avec les strings.
Related https://stackoverflow.com/questions/69207368/constant-filter-sanitize-string-is-deprecated

@J9rem J9rem force-pushed the fix/const-sanitize-string branch from a0ac773 to c116a74 Compare May 22, 2024 18:19
@J9rem
Copy link
Contributor Author

J9rem commented May 22, 2024

@mrflos je pense que je me suis mal expliqué.

Effectivement, il faut utiliser quelque chose comme https://stackoverflow.com/questions/69207368/constant-filter-sanitize-string-is-deprecated et c'est l'objectif de la nouvelle méthode de SecurityController (qui permet de factoriser le code).

Ma première proposition porte effectivement à confusion avec la constante dépréciée. Je viens de faire une nouvelle proposition qui ajoute un argument à la méthode SecurityController::input pour déclencher l'émulation de FILTER_SANITIZE_STRING sans utiliser la valeur de cette constante dépréciée.

@J9rem J9rem changed the title refactor(FILTER_SANITIZE_STRING): use FILTER_SANITIZE_STRING_BKUP refactor(FILTER_SANITIZE_STRING): use new arg for SecurityController::filterInput May 22, 2024
@J9rem
Copy link
Contributor Author

J9rem commented May 23, 2024

J'ai testé ceci:

  • copier de https://repository.yeswiki.net/doryphore/yeswiki-doryphore-4.4.4.zip
  • installation du wiki (rewrite_mode off, pas de robot)
  • paramètres:
  • 'archive' => ['preupdate_backup_activated' => false]
  • 'debug' => 'yes'
  • mise à jour du thème margot
  • mise à jour (réinstallation) de YesWiki doryphore 4.4.4
  • modification de la page GererMisesAJour => {{update version="doryphore-dev"}}
  • je clique sur Changer de version
  • => la mise à jour ne se termine pas et des messages d'erreurs apparaissent
  • avec les correctifs apportés par cette PR, tout fonctionne et il n'y a plus d'avertissement

@J9rem J9rem requested a review from mrflos May 23, 2024 09:07
@J9rem J9rem changed the title refactor(FILTER_SANITIZE_STRING): use new arg for SecurityController::filterInput refactor(FILTER_SANITIZE_STRING): use new arg for SecurityController::filterInput and make the update work May 23, 2024
Copy link
Contributor

@mrflos mrflos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bon j'approuve a contre cœur cette PR faute de temps, mais il y a plusieurs problèmes:

  • le fix sur la migration, fort utile, n'a rien a voir avec cette PR et aurait du faire l'objet d'une autre PR
  • rajouter des attributs booleen et faire des opérateurs ternaires en profusion rendent le code pas très lisible
  • revoir tous les appels de la méthode, c'est difficile a relire et il peut facilement y avoir des oublis
  • il aurait été plus judicieux de filtrer avec le composant Request de Symfony, plutôt que de passer par des méthodes maison (mais j'ai conscience que c'est plus de temps de travail)

@mrflos mrflos merged commit 14673c6 into YesWiki:doryphore-dev May 26, 2024
@J9rem J9rem deleted the fix/const-sanitize-string branch May 27, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants