Probleme d'affichage des pages de redirection au lieu de rediriger (erreur 301) #4878

Open
opened 2 months ago by Pierretux · 14 comments

Bonjour,

Sur un gros site en multi serveur, nous avons des redirections d'url courte géré aussi bien dans le BO de SPIP que sur APAChe (redirection historique) mais cela conduit dans certain cas en erreur 301.

Voici une réponse de l'hébergeur :

Bonjour,
Suite aux récentes remontées (en plus des ancienne qui avait été corrigé via des redirection au niveau apache) nous nous sommes penchée en profondeur sur le problème.

Pour être clair sur le problème :
La mise en cache des réponse de redirection changeait la réponse http en 200 au lieu de 301

Nous avons ouverts des ticket chez notre fournisseur en essayant de trouver pourquoi ce comportement qui ressemble a un bug. Nous n’avons pas trouvé de réponse, car impossible a reproduire hors environnement SPIP.

Après différents test il se trouve que le mécanisme de cache est sensible a la version de http spécifié dans la premiere ligne de la réponse qui est le code retour.

Une réponse commençant par HTTP/1.0 301 Moved Permanently se fera caché avec un code 200
Une réponse commençant par HTTP/1.1 301 Moved Permanently se fera caché avec un code 301

J’imagine que l’Appliance a certain problème avec un protocole de 25 ans qui définit vaguement le système de cache HTTP.

Pour corriger le comportement en préprod (uniquement possible de tester en enlevant l’authentification car les requêtes authentifié ne sont pas cachées (CF RFC 1945))

j’ai mis à jour le fichier https://git.spip.net/spip/spip/src/branch/master/ecrire/inc/headers.php#L225 dans la méthode http_status()
la ligne
header("HTTP/1.0 " . $status_string[$status]);
a été modifiée en :
http_response_code($status);

cette méthode php est présente depuis la version 5.4.0 et ne présente pas de problème particulier a part celui de répondre avec une version du protocole http un poil plus récente ( RFC 2616/7235 (1999/2014)) mais supporté par l’ensemble des navigateurs connus.

Avec @marcimat sur irc on peut simplifier le code :

function http_status($status} { return http-response-code($status); }

Quand penssez vous ?

Bonjour, Sur un gros site en multi serveur, nous avons des redirections d'url courte géré aussi bien dans le BO de SPIP que sur APAChe (redirection historique) mais cela conduit dans certain cas en erreur 301. Voici une réponse de l'hébergeur : > Bonjour, > Suite aux récentes remontées (en plus des ancienne qui avait été corrigé via des redirection au niveau apache) nous nous sommes penchée en profondeur sur le problème. > > Pour être clair sur le problème : > La mise en cache des réponse de redirection changeait la réponse http en 200 au lieu de 301 > > Nous avons ouverts des ticket chez notre fournisseur en essayant de trouver pourquoi ce comportement qui ressemble a un bug. Nous n’avons pas trouvé de réponse, car impossible a reproduire hors environnement SPIP. > > Après différents test il se trouve que le mécanisme de cache est sensible a la version de http spécifié dans la premiere ligne de la réponse qui est le code retour. > > Une réponse commençant par HTTP/1.0 301 Moved Permanently se fera caché avec un code 200 > Une réponse commençant par HTTP/1.1 301 Moved Permanently se fera caché avec un code 301 > > J’imagine que l’Appliance a certain problème avec un protocole de 25 ans qui définit vaguement le système de cache HTTP. > > > Pour corriger le comportement en préprod (uniquement possible de tester en enlevant l’authentification car les requêtes authentifié ne sont pas cachées (CF RFC 1945)) > > j’ai mis à jour le fichier https://git.spip.net/spip/spip/src/branch/master/ecrire/inc/headers.php#L225 dans la méthode http_status() > la ligne header("HTTP/1.0 " . $status_string[$status]); > a été modifiée en : > http_response_code($status); > > cette méthode php est présente depuis la version 5.4.0 et ne présente pas de problème particulier a part celui de répondre avec une version du protocole http un poil plus récente ( RFC 2616/7235 (1999/2014)) mais supporté par l’ensemble des navigateurs connus. Avec @marcimat sur irc on peut simplifier le code : ``` function http_status($status} { return http-response-code($status); } ``` Quand penssez vous ?
Owner

Quelques précisions donc :

Tout ça me fait dire qu’on devrait pouvoir remplacer cette fonction http_status par quelque chose comme :

/**
 * Renvoie au client le header HTTP avec le message correspondant au code indiqué.
 *
 * Ainsi `http_status(301)` enverra le message `301 Moved Permanently`.
 *
 * @link https://www.php.net/manual/fr/function.http-response-code.php
 * @uses http_response_code()
 * @deprecated 4.1 Utiliser http_response_code()
 *
 * @param int $status
 *     Code d'erreur
 **/
function http_status($status) {
	http_response_code($status);
}
Quelques précisions donc : - https://www.php.net/manual/fr/function.http-response-code.php (qui fait ce taf maintenant) - https://stackoverflow.com/questions/3258634/php-how-to-send-http-response-code (où on voit un peu l’historique, notamment avec cgi) - Il y a une `$GLOBALS['REDIRECT_STATUS']` de testée dans la fonction SPIP aussi ; mais aucune occurrence ailleurs. PHP parle lui de `$_SERVER['REDIRECT_STATUS']` https://stackoverflow.com/questions/24378472/what-is-php-serverredirect-status/31870773 . Cette globale est arrivée là https://git-mirror.spip.net/spip/spip/-/commit/ee23cb735cfee6e0e231efcff0b912a1f2f48b78 il y a 17 ans dans un fichier .php3 ; mais je ne trouve pas d’indication nulle part comme quoi ce `REDIRECT_STATUS` aurait été une globale un jour : cependant `$_SERVER` est apparu en PHP 4 (https://www.php.net/manual/fr/reserved.variables.server.php) et donc peut être avant (php 3) c’était accessible uniquement avec `global x`. Dans tous les cas donc actuellemennt ce test… ne fait rien du tout, et donc probablement ne sert plus aussi. Tout ça me fait dire qu’on devrait pouvoir remplacer cette fonction `http_status` par quelque chose comme : ```php /** * Renvoie au client le header HTTP avec le message correspondant au code indiqué. * * Ainsi `http_status(301)` enverra le message `301 Moved Permanently`. * * @link https://www.php.net/manual/fr/function.http-response-code.php * @uses http_response_code() * @deprecated 4.1 Utiliser http_response_code() * * @param int $status * Code d'erreur **/ function http_status($status) { http_response_code($status); } ```

Plus on peut déporter de choses sur des fonctions déjà existantes, mieux c'est à priori.

Si notre fonction ne fait réellement rien de plus, on peut effectivement la garder en mapping pour pas casser les appels des autres gens ailleurs, mais dans le même temps modifier partout notre code pour ne plus l'appeler nous-mêmes déjà. @deprecated quoi

Plus on peut déporter de choses sur des fonctions déjà existantes, mieux c'est à priori. Si notre fonction ne fait réellement rien de plus, on peut effectivement la garder en mapping pour pas casser les appels des autres gens ailleurs, mais dans le même temps modifier partout notre code pour ne plus l'appeler nous-mêmes déjà. `@deprecated` quoi
Owner

gogogo

gogogo
b_b added this to the 4.0 milestone 2 months ago
Poster

commit dans #4879

on peut l'avoir en SPIP 3.2 du coup ?

commit dans #4879 on peut l'avoir en SPIP 3.2 du coup ?
Collaborator

on peut l'avoir en SPIP 3.2 du coup ?

3.2 est en security-fix et a priori, ce n'est pas une vulnérabilité, donc a priori, non.

> on peut l'avoir en SPIP 3.2 du coup ? 3.2 est en security-fix et a priori, ce n'est pas une vulnérabilité, donc a priori, non.
Owner

on peut l'avoir en SPIP 3.2 du coup ?

3.2 est en security-fix et a priori, ce n'est pas une vulnérabilité, donc a priori, non.

Je te rejoins sur le principe, mais on a aussi utilisé le terme de LTS pour la 3.2, et si le report peut-être fait facilement sans "avoir trop d'impact" je pense que ça vaut le coup de le faire :)

> > on peut l'avoir en SPIP 3.2 du coup ? > > 3.2 est en security-fix et a priori, ce n'est pas une vulnérabilité, donc a priori, non. Je te rejoins sur le principe, mais on a aussi utilisé le terme de LTS pour la 3.2, et si le report peut-être fait facilement sans "avoir trop d'impact" je pense que ça vaut le coup de le faire :)
Owner

Je suis assez d’accord avec @b_b : si on peut corriger des petits bugs sans que ça ait trop d’impact, autant le faire, tant qu’on en a la motivation.

Je suis assez d’accord avec @b_b : si on peut corriger des petits bugs sans que ça ait trop d’impact, autant le faire, tant qu’on en a la motivation.
Owner

Appliqué en 4.1

Appliqué en 4.1
Owner

Reporté en 4.0 et 3.2 (modification de http_status() uniquement sur ces branches)

Reporté en 4.0 et 3.2 (modification de http_status() uniquement sur ces branches)
Owner
Et donc en 4.1 - dae4b3bdc709ebc649a96386848a7be535395021 - e7cf7237825ed131a01f94ad967bf55c1b06a285 - ec6233ac6d0d3c7c818329feddcc04bfebc23458
Owner

Il reste une occurrence de flag_sapi_name en globale dans le formulaire de login (ainsi que sa définition dans inc/utils je crois). Je pense que ça peut virer car introduit il y a 14 ans, à une époque où certainement php_sapi_name() n’était peut être pas tout le temps présent ?

https://git-mirror.spip.net/spip/spip/-/commit/7ce2c047f7c8

Il reste une occurrence de `flag_sapi_name` en globale dans le formulaire de login (ainsi que sa définition dans inc/utils je crois). Je pense que ça peut virer car introduit il y a 14 ans, à une époque où certainement php_sapi_name() n’était peut être pas tout le temps présent ? https://git-mirror.spip.net/spip/spip/-/commit/7ce2c047f7c8
Owner

Top, merci pour la refonte et les reports :)

Il reste une occurrence de flag_sapi_name en globale dans le formulaire de login (ainsi que ça définition dans inc/utils je crois).

Oui ici https://git-mirror.spip.net/spip/spip/-/blob/master/ecrire/inc/utils.php#L2595 et là https://git-mirror.spip.net/spip/spip/-/blob/master/prive/formulaires/login.php#L186

Je pense que ça peut virer car introduit il y a 14 ans, à une époque où certainement php_sapi_name() n’était peut être pas tout le temps présent ?

https://git-mirror.spip.net/spip/spip/-/commit/7ce2c047f7c8

Amha oui, ça ne doit plus être bien utile maintenant.

Top, merci pour la refonte et les reports :) > Il reste une occurrence de `flag_sapi_name` en globale dans le formulaire de login (ainsi que ça définition dans inc/utils je crois). Oui ici https://git-mirror.spip.net/spip/spip/-/blob/master/ecrire/inc/utils.php#L2595 et là https://git-mirror.spip.net/spip/spip/-/blob/master/prive/formulaires/login.php#L186 > Je pense que ça peut virer car introduit il y a 14 ans, à une époque où certainement php_sapi_name() n’était peut être pas tout le temps présent ? > > https://git-mirror.spip.net/spip/spip/-/commit/7ce2c047f7c8 Amha oui, ça ne doit plus être bien utile maintenant.
Owner

j’ai ouvert #4881 ; je ferme celui-là donc.

j’ai ouvert #4881 ; je ferme celui-là donc.
b_b added the
amélioration
label 1 month ago
b_b commented 1 month ago
Owner

heu, on peut fermer ce ticket non ? @marcimat c'est pas ce que tu souhaitais faire ?

heu, on peut fermer ce ticket non ? @marcimat c'est pas ce que tu souhaitais faire ?
Sign in to join this conversation.
No Milestone
No project
No Assignees
6 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.