Skip to content

Protection récursive, fix #45, lignes inutiles#46

Open
blackheaven wants to merge 4 commits into
IFAEDI:fix-old-protection-stylefrom
blackheaven:fix-old-style
Open

Protection récursive, fix #45, lignes inutiles#46
blackheaven wants to merge 4 commits into
IFAEDI:fix-old-protection-stylefrom
blackheaven:fix-old-style

Conversation

@blackheaven
Copy link
Copy Markdown
Contributor

  • Correction du $A = $A #45, suppression des $A = $A
  • Correction bug encodage
  • un bug dans la protection XSS qui est récursive maintenant.

@bnjbvr
Copy link
Copy Markdown
Member

bnjbvr commented Feb 14, 2013

Est-ce que c'est testé localement ?

@blackheaven
Copy link
Copy Markdown
Contributor Author

Rapidement, malheureusement.

@bnjbvr
Copy link
Copy Markdown
Member

bnjbvr commented Feb 14, 2013

cf le mail de Seb pour les règles de merge, mais l'idée, c'est quand même de tester avant de seulement proposer la Pull Request ;)

@benjaminplanche
Copy link
Copy Markdown
Member

C'est moi, ou cette PR non mergée est déjà en production ?
Rassurez-moi ...

@blackheaven
Copy link
Copy Markdown
Contributor Author

la production est sur master

@benjaminplanche
Copy link
Copy Markdown
Member

Cool, bon réflexe ! Mais le comportement est étrange, l'erreur #24 de master ne semble plus présente ...

@blackheaven
Copy link
Copy Markdown
Contributor Author

C'est pour cette raison qu'on a des branches (profitons du système propre de Git).
Pour le #24 : je ne suis pas arrivé à le voir donc je ne peux pas dire grand chose :/

@bnjbvr
Copy link
Copy Markdown
Member

bnjbvr commented Feb 14, 2013

J'ai édité la PR pour donner un bon exemple. Voir le wiki pour les suivantes et le workflow ;)

@blackheaven
Copy link
Copy Markdown
Contributor Author

merci

@bnjbvr
Copy link
Copy Markdown
Member

bnjbvr commented Feb 14, 2013

Testé localement de mon côté, ça marche. Il y a toujours cette histoire d'accents et d'apostrophes évoquée dans #24 . Est-il envisageable de la corriger aussi dans cette PR?

Edit: par ailleurs, je n'ai pas le problème d'accents sur une copie de AEDI/SI fraîche. C'est donc un des commits introduits ici qui rajoute cette erreur. Encodage des chaînes en BDD?

@blackheaven
Copy link
Copy Markdown
Contributor Author

tu as tenté avec une nouvelle entrée ? (je pense que ça vient des données).

@bnjbvr
Copy link
Copy Markdown
Member

bnjbvr commented Feb 19, 2013

@blackheaven, merci de tester localement avant de proposer la PR la prochaine fois
C'était une bonne idée d'ajouter des données, vu qu'effectivement les chaînes apparaissent comme étant url encodées désormais...

Annuaire des entreprises

Ajouter l'entreprise suivante:

Nom: Rhéolia
Description: Ce sont les inventeurs de l'eau chaude. Ils aiment les accents aigus (comme dans é), les accents graves (comme dans è), les accents circonflexes (comme dans ê), les esperluettes (comme &) et les balises <script>alert('Hello world!');</script>

Ajouter l'employé

remplir les champs nécessaires
Poste: Rédacteur

Résultats

bug

@blackheaven
Copy link
Copy Markdown
Contributor Author

je vois ça

@blackheaven
Copy link
Copy Markdown
Contributor Author

welcome1

Chez moi ça marche

@bnjbvr
Copy link
Copy Markdown
Member

bnjbvr commented Feb 21, 2013

Même en rechargeant la page?
Recherche et d& eacute ;veloppement, c'est normal?

Merci de revert le commit SERVER[REMOTE_ADDR], ça a déjà été corrigé par une PR de Seb.

@blackheaven
Copy link
Copy Markdown
Contributor Author

en rechargeant ça plante en effet
non
ok

@bnjbvr
Copy link
Copy Markdown
Member

bnjbvr commented Feb 21, 2013

Autant pour moi j'avais omis de préciser qu'effectivement j'avais rechargé la page après insertion.

@blackheaven
Copy link
Copy Markdown
Contributor Author

pas de soucis, tant que je peux le reproduire, je peux le corriger

@blackheaven
Copy link
Copy Markdown
Contributor Author

c'est encodeURIComponent qui m'embête, je corrige ça

@blackheaven
Copy link
Copy Markdown
Contributor Author

normalement les accents passent bien (j'ai testé, mais il faut le tester chez vous aussi).
j'ai virer mon commit de REMOTE_ADDR.

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.

3 participants