[Resolvido] Query_string sugestões

[Thiago Dias_132983 3]

fiz meu primeiro script de query_string e gostaria de saber se ele alguma falha ou vulnerabilidade nele , enfim , gostaria de ver sugestões de como posso melhora-lo !

 

:D

 

Aqui :

<?php

class pagnav {

private $pagina;

public function getPagina($pasta, $paginaerror404) { 

	if (isset($_GET['pg'])) : // VERIFICA SE EXISTE O GET PG

		$this -> pagina = trim(strip_tags($_GET['pg'])) . '.php'; // TIRA OS ESPAÇOS E TAGS 

        if (file_exists($pasta . '/' . $this -> pagina)) : // SE O ARQUIVO EXISTIR 

	include ($pasta . '/' . $this -> pagina); // INCLUI O ARQUIVO 
		else :

	$this -> pagina = 'error.php'; // PAGINA DE ERRO 

	include ($pasta . '/' . $this -> pagina); // SE NAO EXISTE INCLUI A PAGINA DE ERRO	

		endif;

	endif;

	if (@$_GET['pg'] == NULL) : // SE FOR NULO (' ')

		$this -> pagina = 'home.php'; // PAGINA PADRAO	

		include ($pasta . '/' . $this -> pagina); // INCLUI A PAGINA PADRAO

	endif;

}//FIM DA FUNCTION

}//FIM DA CLASS
?>

[Bruno Augusto 417]

Pra que colocar numa classe? Você sabe REALMENTE pra que servem classes?

 

E porque suprimir os erros com arroba? Além de ser uma péssima prática de programação, prejudica a performance.

[Thiago Dias_132983 3]

1º Autoload

 

2º pra tirar as mensagens de erro

 

se você leu oque eu postei la em cima "falha ou vulnerabilidade nele , enfim , gostaria de ver sugestões de como posso melhora-lo"

 

você só criticou é não ajudou em nada !

[Wesley David 20]

você só criticou é não ajudou em nada !

 

Arrogância não leva a nada, o cara te ajudou e te deu conselho, o principal foi o de ocultar erros, se existe erro ele não deve ser ocultado e sim resolvido.

[Thiago Dias_132983 3]

Tem certeza que eu fui o arrogante ?

[Bruno Augusto 417]

1º Autoload

Eu endosso meu ponto de vista. Pensar no autoload antes de pensar no que realmente é Orientação a Objetos é um grande erro.

 

2º pra tirar as mensagens de erro

Você não está tirando mensagens de erro, está varrendo-as para baixo do tapete.

 

O certo é você verificar se dado parâmetro está presente e, se estiver e for conhecido, inclui o arquivo. Se não estiver presente ou for desconhecido, inclui o arquivo padrão ou disara um erro próprio.

 

se você leu oque eu postei la em cima "falha ou vulnerabilidade nele , enfim , gostaria de ver sugestões de como posso melhora-lo"

Só esse último ponto já é uma grande vulnerabilidade.

 

você só criticou é não ajudou em nada !

Então tá, não está mais aqui quem falou. Boa sorte :thumbsup:

[William Bruno 1501]

Oi @Thiago Dias_132983,

 

Minhas sugestões:

-> Não faça o include dentro da classe. Isso é muito muito estranho.

O melhor seria que a tua class retornasse, e você usasse esse retorno em outro lugar, ai sim fazendo o include.

 

-> Não suprima erros com @

 

-> Percebeu que você recorre direto ao $_GET... ? crie um apelido para isso. Uma variavel temporária, interna da classe, já com o tratamento necessario, para evitar injections.

 

-> Use is_file no lugar de file_exists()

 

-> A página 404, não é "padrão" ?

[Thiago Dias_132983 3]

Cara Vlw , só nao "captei" a parte do include , retorna onde? pode me dar um exemplo ?

[William Bruno 1501]

Aqui:

 include ($pasta . '/' . $this -> pagina); // INCLUI O ARQUIVO 

 

em vez de você fazer o include, você deveria retornar:

 

 return $pasta . '/' . $this -> pagina; // INCLUI O ARQUIVO 

 

e então, lá onde você instancia essa classe, você pega esse retorno, e faz o include.

isso vai te dar mais flexibilidade, e maior reaproveitamento.

 

 

É pelo mesmo motivo, que os métodos nunca dão echo.(sempre retornam)

Faça as alterações, e poste a tua classe, para prosseguirmos com a análise.

[Thiago Dias_132983 3]

Cara, praticamente não mudei quase nada .

 

<?php

class pagnav {

  private $pagina;

   public function getPagina($pasta, $paginaerror404) {	

   $pg = strip_tags( trim ( $_GET['pg'] ) ); 

if ( isset( $pg ) ) : // VERIFICA SE EXISTE

  $this -> pagina = $pg . '.php';


if (is_file($pasta . '/' . $this -> pagina)) : // SE O ARQUIVO EXISTIR 

 return $pasta . '/' . $this -> pagina; // RETORNA O CAMINHO 

   else : // SE NAO

           $this -> pagina = 'error.php'; // A VARIAVEL PAGINA = ERROR 404  

 return $pasta . '/' . $this -> pagina; // SE NAO EXISTIR , RETORNA O CAMINHO DO ERRO

endif;

    endif;

 if(!isset ($pg)):

   $this->pagina = 'home.php';

return ($pasta . '/' . $this -> pagina);

endif;
	}//FIM DA FUNCTION

}//FIM DA CLASS
?>

 

agora como posso resgatar o valor do return ? :S

[William Bruno 1501]

ok, olha lá, você usa 4 vezes a mesma concatenação:

$pasta . '/' . $this -> pagina

 

isso é lento. Se você já tem o resultado, você também deveria ter uma variavel temporária, em que armazenasse este valor, e pudesse usar sem perca de desempenho.

 

Eu pessoalmente não gosto da sintaxe:

if:

endif;

 

 

O nome da tua class, deveria (por boa prática), ser iniciada por uma letra em maiusculo, e seguir então o estilo UpperCamelCase

 

 

 

Agora, olhe a tua cadeia de ifs.

Este teu último if:

if(!isset ($pg)):

 

não é negação de tudo acima ?

então deveria ser um ELSE

 

 

 

return é uma estrutura da linguagem, então você não precisa dos parênteses.

Corrija e poste novamente.

[Thiago Dias_132983 3]

<?php
class PagNav {

private $pagina;

public function GetPagina($pasta, $error) {

	$pg = strip_tags(trim($_GET['pg']));

	if (isset($pg)) :
	// VERIFICA SE EXISTE

		$this -> pagina = "$pg.php";

		if (is_file($pasta . $this -> pagina)) :
		// SE O ARQUIVO EXISTIR

			return $pasta . $this -> pagina;
		// RETORNA O CAMINHO

		else :// SE NAO

			$this -> pagina = "$error.php";
			// A VARIAVEL PAGINA = ERROR 404

			return $pasta . $this -> pagina;
		// SE NAO EXISTIR , RETORNA O CAMINHO DO ERRO

		endif;

	endif;

}//FIM DA FUNCTION

}//FIM DA CLASS
?>

 

e quando vou instancia-lo :

 

<?php
$pg = new PagNav(); 
$inc = $pg->GetPagina('paginas/','error');
include($inc);

?>

melhorou algo ? :S

[Vadio 13]

ClasseXyz

metodoXyz

 

chamando

$ClasseXyz = new ClasseXyz();
$pq = $ClasseXyz->metodoXyz

dessa maneira você saberá identificar qual var é uma classe por isso começar os nome de classe com letras maiusculas

contudo você não deve fazer isso com os métodos

 

otra coisa tua query string é básicamente uma função e segundo wiki só tem memso uma função pelo q intendi, então pq não criar esse teu método no construtor da sua classe. dessa maneira

 

classs ClasseXyz{
private $pg;
function __construct($file= null, $error = null){
//seu cod
}
}

 

dessa maneira você irá utilizar sua classe de maneira mto mais agil

 

assim

$file = 'abc/';
$QueryString = new QueryString($file);
include($QueryString);

 

 

uma boa maneira de criar classes é utilizando MVC não posso falar mto sobre pq tbm to aprendendo mas ajuda bastante! Só tem q escrever bastante xD