Erreur json avec la dernière version de Bigup/SPIP 3.2 #4851

Bonjour,

En tant d'uploader un PDf de 2125ko, j'ai systématiquement cette erreur avec https://git.spip.net/spip/bigup/src/branch/1.0 à jour sous SPIP 3.2 (à jour git aussi):

Erreur de transfert. : SyntaxError: Unexpected end of JSON input

De ce que je vois, sur un fichier de 4.7Mo, le JS envoie 4 paquets : 1M, 1M, 1M, 1.7M… au lieu de 5 tel que 1M, 1M, 1M, 1M, 1M, .7M

Du coup le php a un peu de mal à suivre.

Y a peut être un report de bug sur la lib JS qui n’a pas suivi ?

Ceci expliquerait pourquoi de la différence du signe < vs <= que j’ai modifié récemment et qui plantait…

Je viens de tester sur la 4.0 avec un up git de bigup : même message d'erreur.

Par contre, sur une 4.0 installée avec SPIP Loader, l'upload se fait bien.

Après avoir fait pas mal de tests en essayant de revenir au code précédent de BigUp, j'ai découvert qu'il doit y avoir un autre cache au niveau du navigateur qui fait que si ça a planté avec un fichier, même avec la version précédente de BigUp, ça continue à planter.

⇒ Donc, pour tester, il faut en plus changer le nom du fichier sur le disque dur avant upload.

Et donc, en :

remettant la ligne 293 à for ($i = 1; $i < $numOfChunks; $i++) {
renommant mon fichier PDF pour tester de nouveau

Ça marche™ !

Certes… mais on a justement changé cette ligne pour corriger un autre bug…
Du coup je suis tout perdu là…

bon il semble qu’un des soucis soit que les calculs du nombre ne soient pas identiques entre le JS et le PHP…

var chunks = Math.max(
  round(this.size / this.chunkSize), 1
);

$numOfChunks = 
	intval($totalSize / $chunkSize) 
    + ($totalSize % $chunkSize == 0 ? 0 : 1)
;

il semblerait qu’il faille changer le php là dessus… tss.

Bon, donc j’ai a peu près toutes les explications je pense.

Le commit 509ef131ff était insuffisant.

Côté JS

De ce que j’ai compris, flow.js découpe le fichier en N chunks (morceaux) de taille T (disons 1Mo).

Pour le dernier morceau il a 2 choix de stratégie avec une option forceChunkSize (par défaut à false) :

false : le dernier chunk fait au minimum T, et au maximum 2 * T
true : le dernier chunk fait entre 0 et T.

Autrement dit, le dernier chunk est soit >= T (false), soit <= T (true).
Cela change le nombre total N de chunks.

Pour un fichier de 4.7Mo on aurait donc

false : 4 chunks : 1 - 1 - 1 - 1.7
true : 5 chunks : 1 - 1 - 1 - 1 - .7

Si le fichier fait 4Mo tout rond, les deux options découpent en 4 chunks.

On peut supposer déjà que Bigup utilise toujours la même option (mais après tout ce n’est pas forcément obligatoire).

La bonne nouvelle c’est que Flow transmets le nombre de chunks qu’il utilise lors de ses requêtes.

Côté PHP

Côté PHP j’utilisais une formule qui semble t’il ne calculait pas correctement le nombre de chunks.

En l’occurrence il y avait avant 509ef131ff :

$numOfChunks = intval($totalSize / $chunkSize) + ($totalSize % $chunkSize == 0 ? 0 : 1);
for ($i = 1; $i < $numOfChunks; $i++) {
	if (!$this->isChunkUploaded($identifier, $filename, $i)) {
			return false;
	}
}

Je n’ai aucun souvenir du pourquoi ce calcul là présentement (peut être repris d’un code d’exemple quelque part à un moment ?). Toujours est-il que

la formule est différente de Flow par défaut, qui ferait : max(floor($totalSize / $chunkSize), 1)
on ne testait que n-1 chunks dans ce code (sous entendu qu’on calculait «comme si» le dernier chunk était toujours cumulé à l’avant dernier, ce qui est souvent juste… mais pas tout le temps.

Récacapitulons

TotalSize	ChunkSize	N Js (false)	N PHP
5000	1000	5	5
5001	1000	5	6
4999	1000	4	5
500	1000	1	1
0	1000	1	0

Il y a une différence entre PHP et JS particulièrement sur les fichiers dont la taille est une division entière des chunks. Je pense que c’est ce problème qui a été rencontré par @g0oz alors qu’il créait des fichiers avec truncate -s 5M 5M.png

Bref, ceci permet d’y voir un peu plus clair.

Corrections possibles.

Plein de choix !

corriger le calcul théorique du nombre de chunks par PHP.

$numOfChunks = max(floor($totalSize / $chunkSize), 1);
for ($i = 1; $i <= $numOfChunks; $i++) { }

utiliser… le nombre de chunks envoyé par Flow

Ça a l’avantage de fonctionner quelque soit la stratégie de flow.js du dernier chunk (contrairement au calcul 1) précédent).

$numOfChunks = $this->_request('totalChunks');
for ($i = 1; $i <= $numOfChunks; $i++) { }

calculer soi-même si la taille du fichier est suffisante

Mais ça ajoute des accès disques plus nombreux (filesize)

	/**
	 * Indique si tous les morceaux d'un fichier ont été reçus
	 *
	 * @param string $filename
	 * @param string $identifier
	 * @param int $chunkSize
	 * @param int $totalSize
	 * @return bool
	**/
	public function isFileUploadComplete($filename, $identifier, $chunkSize, $totalSize) {
		$chunksDir = $this->cache->parts->fichiers->dir_fichier($identifier, $filename);
		$chunks = $this->getChunkFiles($chunksDir);
		$size = 0;
		foreach ($chunks as $chunk) {
			$size += filesize($chunk);
		}
		if ($size >= $totalSize) {
			return true;
		}
		return false;
	}

Bon, donc j’ai a peu près toutes les explications je pense. Le commit 509ef131ffff6 était insuffisant. ### Côté JS De ce que j’ai compris, flow.js découpe le fichier en N chunks (morceaux) de taille T (disons 1Mo). Pour le dernier morceau il a 2 choix de stratégie avec une option `forceChunkSize` (par défaut à false) : - false : le dernier chunk fait au minimum T, et au maximum 2 * T - true : le dernier chunk fait entre 0 et T. Autrement dit, le dernier chunk est soit >= T (false), soit <= T (true). Cela change le nombre total N de chunks. Pour un fichier de 4.7Mo on aurait donc - false : 4 chunks : 1 - 1 - 1 - 1.7 - true : 5 chunks : 1 - 1 - 1 - 1 - .7 Si le fichier fait 4Mo tout rond, les deux options découpent en 4 chunks. On peut supposer déjà que Bigup utilise toujours la même option (mais après tout ce n’est pas forcément obligatoire). La bonne nouvelle c’est que Flow transmets le nombre de chunks qu’il utilise lors de ses requêtes. ### Côté PHP Côté PHP j’utilisais une formule qui semble t’il ne calculait pas correctement le nombre de chunks. En l’occurrence il y avait avant 509ef131ffff6 : ```php $numOfChunks = intval($totalSize / $chunkSize) + ($totalSize % $chunkSize == 0 ? 0 : 1); for ($i = 1; $i < $numOfChunks; $i++) { if (!$this->isChunkUploaded($identifier, $filename, $i)) { return false; } } ``` Je n’ai aucun souvenir du pourquoi ce calcul là présentement (peut être repris d’un code d’exemple quelque part à un moment ?). Toujours est-il que - la formule est différente de Flow par défaut, qui ferait : `max(floor($totalSize / $chunkSize), 1)` - on ne testait que n-1 chunks dans ce code (sous entendu qu’on calculait «comme si» le dernier chunk était toujours cumulé à l’avant dernier, ce qui est souvent juste… mais pas tout le temps. Récacapitulons | TotalSize | ChunkSize | N Js (false) | N PHP | | --------- | --------- | ------------ | ----- | | 5000 | 1000 | 5 | 5 | | 5001 | 1000 | 5 | 6 | | 4999 | 1000 | 4 | 5 | | 500 | 1000 | 1 | 1 | | 0 | 1000 | 1 | 0 | Il y a une différence entre PHP et JS particulièrement sur les fichiers dont la taille est une division entière des chunks. Je pense que c’est ce problème qui a été rencontré par @g0oz alors qu’il créait des fichiers avec `truncate -s 5M 5M.png` Bref, ceci permet d’y voir un peu plus clair. ### Corrections possibles. Plein de choix ! 1) corriger le calcul théorique du nombre de chunks par PHP. ```php $numOfChunks = max(floor($totalSize / $chunkSize), 1); for ($i = 1; $i <= $numOfChunks; $i++) { } ``` 2) utiliser… le nombre de chunks envoyé par Flow Ça a l’avantage de fonctionner quelque soit la stratégie de flow.js du dernier chunk (contrairement au calcul 1) précédent). ```php $numOfChunks = $this->_request('totalChunks'); for ($i = 1; $i <= $numOfChunks; $i++) { } ``` 3) calculer soi-même si la taille du fichier est suffisante Mais ça ajoute des accès disques plus nombreux (filesize) ```php /** * Indique si tous les morceaux d'un fichier ont été reçus * * @param string $filename * @param string $identifier * @param int $chunkSize * @param int $totalSize * @return bool **/ public function isFileUploadComplete($filename, $identifier, $chunkSize, $totalSize) { $chunksDir = $this->cache->parts->fichiers->dir_fichier($identifier, $filename); $chunks = $this->getChunkFiles($chunksDir); $size = 0; foreach ($chunks as $chunk) { $size += filesize($chunk); } if ($size >= $totalSize) { return true; } return false; } ```

Merci pour cette explication argumentée !

Il me semble que la proposition 3 est la plus robuste, on n'est moins sensible à une modification du calcul du nombre de chunck par flow. Si nous branchons une librairie pour redimmensionner les photos coté client, nous serons moins impactés également.

Sans hésiter la 1) car c'est la seule manière """correcte/logique""" de faire sans dégrader la sécurité et/ou les performances.

Mais quand je lis le tableau que tu donnes ; c'est plutôt JS qui ne compte "pas droit" et qu'il faudrait corriger en fixant simplement "forceChunkSize" a true non ?

La 2) puisque tu n'envisages pas de modifier le js de flow et qu'il va bien falloir faire avec comme il fait lui :-)

Je propose un mix entre 2 et 3 : On ne vérifie la taille que si on pense avoir obtenu tous les morceaux. Ça limite donc cette vérification de taille à la réception du dernier morceau.

Ca semble résoudre le problème chez moi.

To be verified.
Tu confirmes que c’est bon @RealET ?

To be verified.
Tu confirmes que c’est bon @RealET ?

Je confirme que c'est bon avec le fichier qui posait problème.
Tu peux passer le paquet en 1.0.10 et poser un tag ;-)