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

Routes vides dans index.php #135

Open
bugeaud opened this issue Jan 2, 2023 · 6 comments
Open

Routes vides dans index.php #135

bugeaud opened this issue Jan 2, 2023 · 6 comments

Comments

@bugeaud
Copy link

bugeaud commented Jan 2, 2023

Bonjour,

De nombreuses traces d'avertissements de tentative d'accès à un table sur un booleen sont présentes dans les logs :

[php:warn] [pid 453] [client 192.168.1.58:39492] PHP Warning: Trying to access array offset on value of type bool in /app/MedShakeEHR-base/public_html/index.php on line 122, referer: http://host/patient/2746/

J'ai rajouté avant cette ligne à l'emplacement https://github.com/MedShake/MedShakeEHR-base/blob/master/public_html/index.php#L122 (induisant un décallage du n° de ligne de +6 ensuite dans les traces)

if(empty($match)){
    error_log("Variable match vide, routes login:" . var_export( msSystem::getRoutes(['login']),1) . " trace=" . print_r(debug_backtrace(), true));
}else{
    error_log("Variable match =". var_export($match,1) );
}

pour permettre de tracer ce qui se passe comme différence de condition entre le cas ou un tableau de route configurées correspondant à la demande est retournée et quand une valeur retournée est vide (bool ?).

Exemple de trace obtenues

[Mon Jan 02 16:27:20.978169 2023] [php:notice] [pid 1200] [client 192.168.27.70:49558] Variable match =array (\n  'target' => 'login/logIn',\n  'params' => \n  array (\n  ),\n  'name' => NULL,\n), referer: http://host/patients/
[Mon Jan 02 16:27:25.943673 2023] [php:notice] [pid 1200] [client 192.168.27.70:49558] Variable match vide, routes login:false trace=Array\n(\n)\n, referer: http://host/patients/
[Mon Jan 02 16:27:25.943776 2023] [php:warn] [pid 1200] [client 192.168.27.70:49558] PHP Warning:  Trying to access array offset on value of type bool in /app/MedShakeEHR-base/public_html/index.php on line 128, referer: http://host/patients/
[Mon Jan 02 16:27:25.943807 2023] [php:warn] [pid 1200] [client 192.168.27.70:49558] PHP Warning:  Trying to access array offset on value of type bool in /app/MedShakeEHR-base/public_html/index.php on line 128, referer: http://host/patients/
[Mon Jan 02 16:27:25.943831 2023] [php:warn] [pid 1200] [client 192.168.27.70:49558] PHP Warning:  Trying to access array offset on value of type bool in /app/MedShakeEHR-base/public_html/index.php on line 128, referer: http://host/patients/
[Mon Jan 02 16:27:25.979956 2023] [php:notice] [pid 1200] [client 192.168.27.70:49558] Variable match =array (\n  'target' => 'login/logIn',\n  'params' => \n  array (\n  ),\n  'name' => NULL,\n), referer: http://host/patients/

La 1e ligne est une trace qui semble correcte, la seconde un exemple de trace erronée car il n'y a pas de trace de routage pour login dans un contexte d'appel direct.

Il semble bien que dans certains cas un simple msSystem::getRoutes(['login']) varie dans sa réponse, ce qui produit que

    if(msConfiguration::getDefaultParameterValue('optionGeActiverApiRest') == 'true') {
      $match = msSystem::getRoutes(['login', 'apiRest']);
    } else {
      $match = msSystem::getRoutes(['login']);
    }

peut retourner 3 cas : les routes pour login, les routes pour login&apiReste ou rien (bool?).

Je ne vois pas dans le code ce qui peut induire cette différence.
Les fichiers yaml de route sont statiques (aucun changements entre deux appels). il n'y a pas de routes ajoutées par un module installées.

Une idée ?

@MedShake
Copy link
Owner

MedShake commented Jan 7, 2023

msSystem::getRoutes() ne fait rien d'autre que de renvoyer le résultat de la méthode match() d'Altorouter qui elle-même ne renvoie théoriquement qu'un array ou false :
https://github.com/dannyvankooten/AltoRouter/blob/20674b89537080c2257fa69fe766ea7d5d1721cb/AltoRouter.php#L254

Donc effectivement il y a le coup où le retour est false à gérer, car sinon en 122 sur index.php, ça coince.

Par contre en 122, on est a priori dans un cas où l'identification n'est pas présente. Il n'est donc pas logique que les logs produits donne /patients/ en referer ?

On peut donc, il faut donc, gérer le cas ou match est false en 122, mais il faudrait savoir si cela se produit par accident (entrée de l'url sans identification active) ou si ces appels viennent d'une situation d'utilisation normale.

B.

@bugeaud
Copy link
Author

bugeaud commented Jan 7, 2023

J'ai au moins deux cas:

  • Production lorsqu'on refresh la page patient (pour tester un changement, vérifier l'ajout d'un document, etc)
  • Production spontanée alors qu'il n'y a pas d'utilisateur actif (j'anticipe un http refresh quelque part qui déclenche le GET /patient/, mais je n'ai pas encore confirmé cette hypothèse).

@MedShake
Copy link
Owner

MedShake commented Jan 7, 2023

Je propose en 122 d'ajouter préalablement :

   	if($match === false) {
		msTools::redirection('/login/');
	}

Ca ne répond pas à l'origine mais ca traite déjà la réponse à donner.

@MedShake
Copy link
Owner

MedShake commented Jan 7, 2023

Sinon il n'y a logiquement pas 36 scripts qui attaquent en arrière-plan. De mémoire, il n'y en a qu'un seul même : le rafraîchissement de la liste des patients en salle d'attente. Il peut tourner alors même que le menu n'est pas activé.
Sinon le plus simple est d'inspecter la page et les appels réseaux. post chargement initial.

MedShake added a commit that referenced this issue Jan 7, 2023
@bugeaud
Copy link
Author

bugeaud commented Jan 7, 2023

C'est étrange. Pourquoi un refresh qui dispose du même contexte d'auth dans le user agent tomberait sur ce cas s'il n'y a pas de problème sur le côté serveur aussi ?
Je vais tester cet aprem le contournement qui me semble de toute façon nécessaire pour éviter un chemin indésirable mais celà risque de produire des effets de bords si les appels bloqués étaient légitimes.

@MedShake
Copy link
Owner

MedShake commented Jan 7, 2023

C'est bien toute la question effectivement :-)
J'ai testé en local ici et pas d'effet de bord visible.
Appliqué ce contrôle permettra peut-être de faire sortir la chose plus ouvertement. Toute façon il s'avère nécessaire à terme dans une prochaine version consolidée.

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

2 participants