Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plantage de seenthis sur un message #174

Closed
Fil opened this issue Dec 17, 2016 · 34 comments
Closed

plantage de seenthis sur un message #174

Fil opened this issue Dec 17, 2016 · 34 comments

Comments

@Fil
Copy link
Member

Fil commented Dec 17, 2016

le message https://seenthis.net/messages/552380 faisait planter seenthis, je ne sais pas pour quelle raison ; je l'ai effacé et je note son contenu ici pour qu'on puisse faire des tests avant de le remettre.

Avant l'arriv de l'ectricit les ouvriers outaient de la techno au travail

http://www.core77.com/posts/58982/How-Did-Factories-Get-Power-to-Their-Machines-Before-Electricity

https://youtu.be/Nc8LXlNuh8s

http://s3files.core77.com/blog/images/549455_81_58982_VVxCIx9m5.jpg

http://s3files.core77.com/blog/images/549466_81_58982_URtPCkWCG.jpg

(j'imagine que l'absence de caractères accentués est due à la manière dont j'ai copié/collé en vitesse)

@Fil
Copy link
Member Author

Fil commented Dec 17, 2016

quesion subsidiaire : peut-on détecter un plantage, le noter quelque part, et éviter le message à l'avenir.

@brunob
Copy link
Member

brunob commented Dec 18, 2016

Je viens de tester sur mes instances locales en 2.1 et 3.1 sans reproduire le bug, peut-être que cela venait des accents du message ?

@Fil
Copy link
Member Author

Fil commented Dec 18, 2016

Je viens de réessayer et ce sont les fichiers jpg qui font planter.

update spip_me_texte set texte="Avant l'arrivee de l'electricite les ouvriers ecoutaient de la techno au travail\n\nhttp://www.core77.com/posts/58982/How-Did-Factories-Get-Power-to-Their-Machines-Before-Electricity\n\nhttps://youtu.be/Nc8LXlNuh8s\n\n\nhttp://s3files.core77.com/blog/images/549455_81_58982_VVxCIx9m5.jpg\n\nhttp://s3files.core77.com/blog/images/549466_81_58982_URtPCkWCG.jpg" where id_me=552380;

=> crash

update spip_me_texte set texte="Avant l'arrivee de l'electricite les ouvriers ecoutaient de la techno au travail\n\nhttp://www.core77.com/posts/58982/How-Did-Factories-Get-Power-to-Their-Machines-Before-Electricity\n\nhttps://youtu.be/Nc8LXlNuh8s" where id_me=552380;

=> OK

@brunob
Copy link
Member

brunob commented Dec 18, 2016

lol ça n'est pas un jpg mais un png, du coup image_reduire tombe dans le panneau :p

http://s3files.core77.com/blog/images/549455_81_58982_VVxCIx9m5.jpg

@brunob
Copy link
Member

brunob commented Dec 18, 2016

Je ne crois pas qu'on y puisse grand chose car le serveur en face envoie l'image en indiquant que c'est un jpg, `copie_locale' est bien faite, mais là tout est en place pour la tromper :p

bb@tybook:~$ curl -I http://s3files.core77.com/blog/images/549455_81_58982_VVxCIx9m5.jpg
HTTP/1.1 200 OK
x-amz-id-2: ZCesJ9l14UGYBDRVK1fXxQ9bWIiBF3w6wwpvw8vBlukxPw4RTJSnTtX+A5YIQza4Ig29l4b+tEw=
x-amz-request-id: 49B09DD150D49FA8
Date: Sun, 18 Dec 2016 17:11:07 GMT
Last-Modified: Thu, 15 Dec 2016 16:28:06 GMT
ETag: "053936a55b995a3ee730dc3d986982e6"
Expires: Tue, 15 Dec 2026 16:28:15 GMT
Accept-Ranges: bytes
Content-Type: image/jpg
Content-Length: 1192999
Server: AmazonS3

Du coup, on ferme le bug ?

@salvatore-spip
Copy link
Member

salvatore-spip commented Dec 18, 2016 via email

@brunob
Copy link
Member

brunob commented Dec 18, 2016

@salvatore-spip oui pour le ticket, perso je n'ai pas d'idée sur le fix à proposer.

@kent1D
Copy link
Contributor

kent1D commented Dec 18, 2016 via email

@Fil
Copy link
Member Author

Fil commented Dec 18, 2016

en lien avec #173 alors donc

@0gust1
Copy link

0gust1 commented Dec 19, 2016

Ok, même constat que @brunob sur ce que renvoie le serveur, je venais le poster ici.

@salvatore-spip
Copy link
Member

J'ai testé avec le mécanisme de base de SPIP (attacher un doc distant).

Avec GD2, ça plante la génération de l'image en mettant :

Erreur lecture imagecreatefromjpeg IMG/distant/jpg/549455_81_581516.jpg

Avec convert, pas de problème du tout ni avec php-imagick

Cependant pas de plantage au point de péter une page...

@salvatore-spip
Copy link
Member

Donc du coup, je pense que l'on pourrait faire un petit plugin qui gèrerait cela si on dispose de php-imagick ou convert (même si pas forcément choisi comme méthode de traitement des images) qui renommerait l'image au sortir de copie_local s'il sait l'identifier via Imagick::identifyImage ou identify

Qu'en pensez vous

@kent1D
Copy link
Contributor

kent1D commented Dec 19, 2016

Désolé, j'ai un pb de double identité :D

@brunob
Copy link
Member

brunob commented Dec 19, 2016

@kent1D oui pourquoi pas dans un plugin pour tester, et intégrer le bouzin dans le core par la suite (ou alors directement dans le core, au choix).

@kent1D
Copy link
Contributor

kent1D commented Dec 19, 2016

Tu saurais me dire si le serveur de seenthis actuel dispose de l'un ou l'autre (php-imagick ou convert?)

@kent1D
Copy link
Contributor

kent1D commented Dec 19, 2016

@brunob je suis dessus là

@brunob
Copy link
Member

brunob commented Dec 19, 2016

@kent1D super, merci :) S'il manque des trucs sur le serveur on peut les ajouter sans pb.

@kent1D
Copy link
Contributor

kent1D commented Dec 19, 2016

Ah mais c'est une 2.1.x non, tu as de quoi tester sur une 3.x déjà? avant que je me lance dans de la rétrocompat?

@brunob
Copy link
Member

brunob commented Dec 19, 2016

Ouep, j'ai une instance locale en 3.1 et une autre 3.1 en ligne.

@kent1D
Copy link
Contributor

kent1D commented Dec 19, 2016

Du coup, le problème vient du choix de gd2 comme méthode de réduction d'image qui plante quand on lui donne à bouffer un jpg qui est un png et j'imagine inversement également.

Ce problème ne se produit ni avec imagick ni convert, donc ce serait pas mal de switcher si possible sur convert qui est bien plus performant normalement.

Cependant j'ai commis le plugin medias_identifier qui du coup devrait éviter les problèmes rencontrés.

[(#VAL{http://s3files.core77.com/blog/images/549455_81_58982_VVxCIx9m5.jpg}|copie_locale|image_reduire{300})] passe du coup sans problème pour moi avec ce plugin activé

@kent1D
Copy link
Contributor

kent1D commented Dec 19, 2016

Ok, avec le dernier commit, on peut même faire le |image_reduire directement sur une URL un peu vérolée sans copie_locale préalable :

[(#VAL{http://s3files.core77.com/blog/images/549455_81_58982_VVxCIx9m5.jpg}|image_reduire{300})]

@brunob
Copy link
Member

brunob commented Dec 19, 2016

Testé et approuvé sur mon instance locale en 3.1 après avoir installé php-imagick bien sûr ;)

@brunob
Copy link
Member

brunob commented Dec 20, 2016

@Fil pas d'objection à ce que je bascule de gd2 vers convert sur seenthis ?

@Fil
Copy link
Member Author

Fil commented Dec 20, 2016 via email

@brunob
Copy link
Member

brunob commented Dec 20, 2016

Cool, c'est fait :)

@brunob
Copy link
Member

brunob commented Dec 21, 2016

Hum, on dirait bien que convert ne fait pas bien le job sur seenthis, j'observe des différence dans les avatars suivant la taille de l'image générée, exemples :

https://seenthis.net/local/cache-gd2/f3b1a45e06c587349b6b5270fc43566c.jpg
https://seenthis.net/local/cache-gd2/36e4909ad8d29b8ada3323e74c7afb0e.jpg
https://seenthis.net/local/cache-gd2/8ae99ca6a4c31bb69a22df7ac5eb7e38.jpg

@brunob
Copy link
Member

brunob commented Dec 21, 2016

Dites, ça serait pas ça l'origine du pb ?

root@seenthis:/var/www/seenthis.net# convert
bash: convert : commande introuvable
root@seenthis:/var/www/seenthis.net# apt-cache policy imagemagick
imagemagick:
  Installé : (aucun)
  Candidat : 8:6.8.9.9-5+deb8u6

@Fil
Copy link
Member Author

Fil commented Dec 21, 2016

et à propos d'images voir aussi https://seenthis.net/messages/372622

@brunob
Copy link
Member

brunob commented Dec 21, 2016

Et donc seenthis/hebergement#10 ;)

@brunob
Copy link
Member

brunob commented Dec 21, 2016

2016-12-21 14:49:40 84.159.203.218 (pid 30188) convert -quality 85 -resize 300x83! IMG/distant/jpg/banner_icela0f28.jpg local/cache-vignettes/L300xH83/banner_icela0f28-c46ae.jpg
2016-12-21 14:49:40 84.159.203.218 (pid 30188) echec convert sur local/cache-vignettes/L300xH83/banner_icela0f28-c46ae.jpg

https://core.spip.net/projects/spip/repository/entry/branches/spip-2.1/ecrire/inc/filtres_images_lib_mini.php#L541

Il semble donc que si on veut jouer avec convert, il faut disposer de sa commande.

@brunob
Copy link
Member

brunob commented Dec 21, 2016

Je viens d'installer les paquets imagemagick et php5-imagick sur le serveur, ça règle bien le problème. On doit pouvoir fermer le ticket maintenant non ?

@brunob
Copy link
Member

brunob commented Dec 21, 2016

On ferme donc, @kent1D ce que tu as envoyé dans le plugin medias_identifier aurait peut-être sa place dans le core en 3.1 ?

@salvatore-spip
Copy link
Member

salvatore-spip commented Dec 21, 2016 via email

@brunob
Copy link
Member

brunob commented Dec 22, 2016

@salvatore-spip ou @kent1D ^^ Non car seenthis tourne encore en 2.1, mais comme je le disais j'ai pu le tester avec succès sur mon instance en 3.1.

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

No branches or pull requests

5 participants