Comment récupérer le contenu d'une URL distante sans être ingénieur à la NASA?

Comme je suis en train de développer un module, j’ai regardé ce que font les autres pour avoir un exemple de personnes expérimentées avec Dolibarr.

Voici la fonction que j’ai vue :

public function getContents($url){
	$this->data = false;
	$res = @file_get_contents($url);
	$this->http_response_header = $http_response_header;
	$this->TResponseHeader = self::parseHeaders($http_response_header);
	if($res !== false){
		$this->data = $res;
	}
	return $this->data;
}

Je pense qu’il y a un problème ici.

Je regarde et je suis la logique de la variable : $this->data et ce qui est retourné.

Pourquoi s’embêter à tester $res pour tenter de retourner une valeur non nulle.

Idée de correction numéro 1 :

Voici donc la même fonction en plus simple ($this->data et la gestion des codes de retour ne sert à rien dans le reste du code de toute façon) :

public function getContents($url){
	return @file_get_contents($url);
}

Donc finalement, beaucoup de bruit pour rien avec une fonction inutile en fin de compte.

Idée de correction numéro 2 :

Ici, je montre un exemple simple qui peut gérer deux situations en fonction de la configuration de PHP. Je ne gère pas le code de retour, ça ne sert à rien, je n’en ai pas besoin.

public function getContents($url, $alt_url)
	{
		global $langs;

		// PHP method
		if (\filter_var(\ini_get('allow_url_fopen'), \FILTER_VALIDATE_BOOLEAN)) {
			$contents = @file_get_contents($url);
		} elseif (function_exists('curl_version')) {
			// CURL method
			$ch = curl_init();
			curl_setopt($ch, CURLOPT_URL, $url);
			curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
			$contents = curl_exec($ch);
			curl_close($ch);
		} else {
			// Default contents
			$contents = '<div class="error">'.$langs->trans('URLError', $alt_url).'</div>';
		}

		return $contents;
	}

Oui mais la fonction core/lib/geturl.lib.php :: getURLContent ?

Sans être ingénieur à la NASA pour comprendre le code on a dit :slight_smile:

Moi je dis qu’il manque une fonction

public function getContents($politesse)
}

:crazy_face:

1 « J'aime »
	public function getVersion($translated = 1)
	{
		global $langs, $conf;
		$currentversion = $this->version;
		if (!empty($conf->global->PATASMONKEY_SKIP_CHECKVERSION))
			return $currentversion;
		if ($this->disabled) {
			$newversion= $langs->trans("DolibarrMinVersionRequiered")." : ".$this->dolibarrminversion;
			$currentversion="<font color=red><b>".img_error($newversion).$currentversion."</b></font>";
			return $currentversion;
		}
		$context  = stream_context_create(array('http' => array('header' => 'Accept: application/xml')));
		$changelog = @file_get_contents(
				str_replace("www", "dlbdemo", $this->editor_url).'/htdocs/custom/'.$this->name.'/changelog.xml',
				false, $context
			);
		//$changelog = @file_get_contents($this->editor_url.$this->editor_version_folder.$this->name.'/');
		if ($changelog === false)	// not connected
			return $currentversion;
		else {
			$sxelast = simplexml_load_string(nl2br($changelog));
			if ($sxelast === false)
				return $currentversion;
			else
				$tblversionslast=$sxelast->Version;
			$lastversion = $tblversionslast[count($tblversionslast)-1]->attributes()->Number;
			if ($lastversion != (string) $this->version) {
				if ($lastversion > (string) $this->version) {
					$newversion= $langs->trans("NewVersionAviable")." : ".$lastversion;
					$currentversion="<font title='".$newversion."' color=#FF6600><b>".$currentversion."</b></font>";
				} else
					$currentversion="<font title='Version Pilote' color=red><b>".$currentversion."</b></font>";
			}
		}
		return $currentversion;
	}

	public function getLocalVersion()
	{
		global $langs;
		$context  = stream_context_create(array('http' => array('header' => 'Accept: application/xml')));
		$changelog = @file_get_contents(dol_buildpath($this->name, 0).'/changelog.xml', false, $context);
		$sxelast = simplexml_load_string(nl2br($changelog));
		if ($sxelast === false)
			return $langs->trans("ChangelogXMLError");
		else {
			$tblversionslast=$sxelast->Version;
			$currentversion = (string) $tblversionslast[count($tblversionslast)-1]->attributes()->Number;
			$tblDolibarr=$sxelast->Dolibarr;
			$minVersionDolibarr=$tblDolibarr->attributes()->minVersion;
			if ((int) DOL_VERSION < (int) $minVersionDolibarr) {
				$this->dolibarrminversion=$minVersionDolibarr;
				$this->disabled = true;
			}
		}
		return $currentversion;
	}

Et sinon dans le coeur de dolibarr vous avez la fonction getURLContent qui fait tout ce qu’il faut proprement « dolibarr way of life »

voir le fichier core/lib/geturl.lib.php

issu du code de dolibarr-15.0 par exemple:

/**
 * Function to get a content from an URL (use proxy if proxy defined).
 * Support Dolibarr setup for timeout and proxy.
 * Enhancement of CURL to add an anti SSRF protection:
 * - you can set MAIN_SECURITY_ANTI_SSRF_SERVER_IP to set static ip of server
 * - common local lookup ips like 127.*.*.* are automatically added
 *
 * @param	string	  $url 				    URL to call.
 * @param	string    $postorget		    'POST', 'GET', 'HEAD', 'PUT', 'PUTALREADYFORMATED', 'POSTALREADYFORMATED', 'DELETE'
 * @param	string    $param			    Parameters of URL (x=value1&y=value2) or may be a formated content with $postorget='PUTALREADYFORMATED'
 * @param	integer   $followlocation		0=Do not follow, 1=Follow location.
 * @param	string[]  $addheaders			Array of string to add into header. Example: ('Accept: application/xrds+xml', ....)
 * @param	string[]  $allowedschemes		List of schemes that are allowed ('http' + 'https' only by default)
 * @param	int		  $localurl				0=Only external URL are possible, 1=Only local URL, 2=Both external and local URL are allowed.
 * @return	array						    Returns an associative array containing the response from the server array('content'=>response, 'curl_error_no'=>errno, 'curl_error_msg'=>errmsg...)
 */

Ce n’est pas très propre, du code dans tous les sens.
Utilisable ? Non, regarde bien, il y a des trucs bizarres sur les vérifications d’IP, ça n’a rien à faire là.

Ça c’est votre point de vue, toute amélioration est sans aucun doute bienvenue → github de dolibarr

Mon point de vue est que lorsque chaque développeur implémente « sa meilleure solution dans son coin » on se retrouve avec un millefeuille dont l’utilisateur final finit toujours par payer la note.

Dolibarr propose en solution « core » des outils, autant les utiliser et apprendre à les utiliser et les améliorer lorsque c’est nécessaire…

1 « J'aime »

Non ce n’est pas mon point de vue car malheureusement l’utilisation de la fonction ne fonctionne pas avec mes tests car il faudrait enlever des lignes.

Et je ne peux pas corriger cette fonction car elle est illisible dans son état actuel.

Je suis désolé que tu le prennes mal mais Dolibarr est loin d’être un exemple en matière de qualité de code.

Pour éviter de perdre du temps, pour l’instant, à corriger la fonction qui est complètement à revoir, j’ai fait une simple.

Donc voici ma méthode pour corriger Dolibarr :

  • je commence par proposer des petits patchs pour tester le processus et les humains qui valident le code.
  • après, je vais corriger un peu plus gros tant que ça ne remet pas en cause les façons de faire dans le code.
  • et ensuite je vais tenter de faire corriger le cœur en proposant des améliorations qui vont impacter le reste du code dans le but d’accélérer Dolibarr et de diminuer le nombre de ligne de code.
1 « J'aime »

yaka…

a) je ne le prends pas mal
et
b) je n’ai jamais dit que dolibarr était un exemple en matière de code

mais

c) dolibarr a de l’expérience … sans doute beaucoup plus que moi
d) il y a déjà eu des contributions sur cette fonction, signe que son contenu a été analysé et fait face à de nombreuses situations dont on ne peut pas forcément soupçonner l’existence
e) quand je vois par exemple une annotation par rapport à yogosha je ne peux que me dire qu’il doit y avoir des choses intéressantes à creuser et qu’elle a du faire face à quelques tests

(un exemple de situation à laquelle on ne pense pas forcément toujours dès le début, que se passe-t-il avec « votre » fonction si l’adresse de destination a un certificat auto signé par exemple dans un cas de réseau local ?)

etc. donc encore une fois j’hésite entre LOL et pédagogie … et merci pour tous les correctifs de dolibarr à venir, dans tous les cas nous oeuvrons tous dans le même but: rendre dolibarr meilleur et c’est la seule chose importante dans le fond.

1 « J'aime »

Bonsoir, en attendant de m’attaquer au gros poisson, j’ai simplifié (avec les conseils d’eldy) la fonction juste en dessous dans geturl.lib.php :

Dolibarr est rempli de code simplifiable ou même du code qui ne fait rien et ce serait bien de nettoyer un peu car on s’en sort plus.

Exemple de code qui ne faisait rien et que j’ai enlevé :

https://github.com/Dolibarr/dolibarr/commit/ba499631861fa1b29af707fb82277660cb342e7e

super merci pour vos contributions tout le monde en bénéficiera c’est top !