Skip to main content
se añadieron 1325 caracteres en el cuerpo
Origen Enlace
A. Cedano
  • 95.2k
  • 31
  • 145
  • 257

Sin embargo, en otros aspectos tu código es farragoso, repetitivo y no controla datos esenciales. Conviene por ejemplo que si alguna de las columnas tiene una restricción NOT NULL, verifiques que los valores para esas columnas contengan datos y evites la inserción, respetando así el contrato de NOT NULL en la base de datos y escribiendo un código coherente.

Conviene igualmente que verifiques cada dato que vendría en el $_POST, el usuario pudo haber mandando algunos datos vacíos, y te estarías exponiendo al menos a un casouno o más casos de Undefined index. La verificación del action también se puede simplificar. Y la coherencia del código en sí misma no se aprecia, por ejemplo, no veo si retornas algo desde el contexto en que insertas.

Dado que dices que haces un return $status te propondré una forma controlada de retornar lo que haya ocurrido en el método, usaré un array, porque te permite trabajar con pares clave => valor que puedes luego verificar y mostrar mensajes. Esto es más fácil de adaptar a una política de manejo de errores, o a respuestas de una API, etc.

include_once 'class/Estudiante.php';

#Declarar cada variable sólo cuando estés seguro de que la usarás
$json = array();

#¿Para qué es esta variable?    
$studentInfo = array();
/*
   Si usas PHP anterior a la v. 7
   cambia esto por:
   $action = !empty($_POST['action']) ? $_POST['action'] : null;
*/
$action=$_POST['action'] ?? null;
if ($action && $action == 'inscribir') {
    /*
       Aquí vamos a verificar cada dato para evitar
       casod de Undefined index
       Si tienes PHP v. anterior a 7 usa lo que
       ya se indicó más arriba para el action en todas
       las asignaciones que siguen
       Aquí convendría un if para verificar todas aquellas
       columnas que tiene restriccion NOT NULL
       Usaré una convención de nombre en inglés,
       entre otras ventajas, te evitarías 
       nombre de variables kilométricos, por ejemplo
       dob = date of birth (fecha de nacimiento)
       */
    
    $id=$_POST['estudiante_id'] ?? null;
    $age=$_POST['Edad'] ?? null;
    $dob=$_POST['Fecha_Nacimiento'] ?? null;
    $code=$_POST['Codigo'] ?? null;

    /*
       Aquí verificarías con un if TODOS
       los NOT NULL, por ejemplo
       if ($id && $code) {  // insertar } else { //faltan datos obligatorios }
       */

    $studentObj = new Estudiante();
    $studentObj->setStudentID($id);   
    # ¿Por qué la edad no es un campo calculado?    
    $studentObj->setEdad($age);
    $studentObj->setNacimiento($dob);        
    $studentObj->setPassword($code); 

    $status/*
 = $studentObj->inscribir();     OJO A ESTO:
       En la nueva versión del código
    # Abreviamos  inscribir devolverá un array con dos posibilidades:
       1. Siempre una clave status que será true o false
       2. Una clave msg con un ternariomensaje
    $json['msg'] = !empty($status) ?De 'success'este :modo, 'failed';la respuesta del método
       queda acoplada a la lógica que ya estabas siguiendo
       aunque quizá convendría refinarla un poco
       adpotando lo de la clave true o false           
       */ 
    $json = $studentObj->inscribir();


} else {
    $json['msg'] = 'No se ha enviado la acción inscribir';
}

header('Content-Type: application/json');   
echo json_encode($json); 
try {
        $cargo = "Ninguno";
        $fechaActual = date('Y-m-d');
        $salida = date('Y-m-d',strtotime($fecha_actual.'+ 1 year'));
        $RAMA= $this->_Rama;
        $integer = $this->_edad;
        $edad = (int)$integer;
        $sql = "INSERT INTO student(edad, password, fecha_nacimiento, fecha_creacion,fecha_salida)  VALUES (:edad, :pass ,:nacimiento, :inscripcion,:salida)";

         /*
             Vamos a poner los dos puntos a los marcadores
             de todos modos PDO lo haría por nosotros
             pero si se lo damos hecho mejor,
             así lo hacemos trabajar menos
         */
         $data = [
                ':edad' => $edad,               
                ':pass' => $this->_Password,               
                ':nacimiento' => $this->_Fecha_Nac,               
                ':inscripcion' => $fechaActual,                
                ':salida' => $salida                               
            ];

          $stmt = $this->db->prepare($sql);
          $stmt->execute($data);
          /*
             NoComo quedadije claromás cómoarriba, seaquí acoplavamos estoa devolver
             un array con eluna códigoclave anterior
status que será true
          Por otra parte, siy eluna códigoclave entramsg enque elserá catchun mensaje
             No 
 estarías controlando adecuadamente en el llamador
       con la cantidad de filas afectadas
 Convendría quizá responder con un array       */
             con$status clave= successarray(
 aquí y con clave fail en el catch
             Así verificarías posibles errores aquí      'status'=>true,
             No deberías resolver con un simple die
            'msg' el=> cualsprintf("Se rompeinsertaron la%s COHERENCIAfilas",$stmt->rowCount())
             */
             $status = $stmt->rowCount( );            
        } catch (Exception $err) {          
            /*
               LoAquí, dichopara enseguir con el comentariocontrato anteriorde nuestro código
               estovamos rompea ladevolver COHERENCIAun delarray códigocon una clave status que es false
               ¿Cómoy controlarássiempre estouna desdeclave msg que será el llamador?mensaje de error
               ¿OEn quizáun soncontexto sólode suposicionesPolítica mías?
de manejo de errores más avanzada
          Sin ver todo   podrías recoger el contextocódigo node puedoerror saberlopor ejemplo
               e incorporarlo a un sistema de errores personalizados o cosas así ...
               PorLo otraque partees EVITAimportante mostraraquí erroreses internosevitar
               Unque gravísimose fallomuestren mensajes de PDOerror porinterno
               los cuales son MUY jugosos para los hackers
               Por ejemplo, un gravísimo fallo de PDO es que
               ante errores de conexión ¡revela la contraseña!
               en el mensaje de error!
               */
            die
            $status = array("Error!: 
 "                              'status'=>false,
                               'msg' => 'Error ejecutando la consulta. $errSi quieres captura el código de error e incorpóralo en tu PME'
                             );        
            return $status; 

        }

Sin embargo, en otros aspectos tu código es farragoso, repetitivo y no controla datos esenciales. Conviene por ejemplo que si alguna de las columnas tiene una restricción NOT NULL, verifiques que los valores para esas columnas contengan datos y evites la inserción.

Conviene igualmente que verifiques cada dato que vendría en el $_POST, el usuario pudo haber mandando algunos datos vacíos, y te estarías exponiendo al menos a un caso de Undefined index. La verificación del action también se puede simplificar. Y la coherencia del código en sí misma no se aprecia, por ejemplo, no veo si retornas algo desde el contexto en que insertas.

include_once 'class/Estudiante.php';

#Declarar cada variable sólo cuando estés seguro de que la usarás
$json = array();

#¿Para qué es esta variable?    
$studentInfo = array();
/*
   Si usas PHP anterior a la v. 7
   cambia esto por:
   $action = !empty($_POST['action']) ? $_POST['action'] : null;
*/
$action=$_POST['action'] ?? null;
if ($action && $action == 'inscribir') {
    /*
       Aquí vamos a verificar cada dato para evitar
       casod de Undefined index
       Si tienes PHP v. anterior a 7 usa lo que
       ya se indicó más arriba para el action en todas
       las asignaciones que siguen
       Aquí convendría un if para verificar todas aquellas
       columnas que tiene restriccion NOT NULL
       Usaré una convención de nombre en inglés,
       entre otras ventajas, te evitarías 
       nombre de variables kilométricos, por ejemplo
       dob = date of birth (fecha de nacimiento)
       */
    
    $id=$_POST['estudiante_id'] ?? null;
    $age=$_POST['Edad'] ?? null;
    $dob=$_POST['Fecha_Nacimiento'] ?? null;
    $code=$_POST['Codigo'] ?? null;

    /*
       Aquí verificarías con un if TODOS
       los NOT NULL, por ejemplo
       if ($id && $code) {  // insertar } else { //faltan datos obligatorios }
       */

    $studentObj = new Estudiante();
    $studentObj->setStudentID($id);   
    # ¿Por qué la edad no es un campo calculado?    
    $studentObj->setEdad($age);
    $studentObj->setNacimiento($dob);        
    $studentObj->setPassword($code);
    $status = $studentObj->inscribir();

    # Abreviamos con un ternario
    $json['msg'] = !empty($status) ? 'success' : 'failed';

} else {
    $json['msg'] = 'No se ha enviado la acción inscribir';
}

header('Content-Type: application/json');   
echo json_encode($json); 
try {
        $cargo = "Ninguno";
        $fechaActual = date('Y-m-d');
        $salida = date('Y-m-d',strtotime($fecha_actual.'+ 1 year'));
        $RAMA= $this->_Rama;
        $integer = $this->_edad;
        $edad = (int)$integer;
        $sql = "INSERT INTO student(edad, password, fecha_nacimiento, fecha_creacion,fecha_salida)  VALUES (:edad, :pass ,:nacimiento, :inscripcion,:salida)";

         /*
             Vamos a poner los dos puntos a los marcadores
             de todos modos PDO lo haría por nosotros
             pero si se lo damos hecho mejor,
             así lo hacemos trabajar menos
         */
         $data = [
                ':edad' => $edad,               
                ':pass' => $this->_Password,               
                ':nacimiento' => $this->_Fecha_Nac,               
                ':inscripcion' => $fechaActual,                
                ':salida' => $salida                               
            ];

          $stmt = $this->db->prepare($sql);
          $stmt->execute($data);
          /*
             No queda claro cómo se acopla esto
             con el código anterior
             Por otra parte, si el código entra en el catch
             No estarías controlando adecuadamente en el llamador
             Convendría quizá responder con un array
             con clave success aquí y con clave fail en el catch
             Así verificarías posibles errores aquí
             No deberías resolver con un simple die
             el cual rompe la COHERENCIA
             */
             $status = $stmt->rowCount();            
        } catch (Exception $err) {          
            /*
               Lo dicho en el comentario anterior
               esto rompe la COHERENCIA del código
               ¿Cómo controlarás esto desde el llamador?
               ¿O quizá son sólo suposiciones mías?
               Sin ver todo el contexto no puedo saberlo...
               Por otra parte EVITA mostrar errores internos
               Un gravísimo fallo de PDO por ejemplo es que
               ante errores de conexión ¡revela la contraseña!
               en el mensaje de error!
               */
            die("Error!: " . $err);
        }

Sin embargo, en otros aspectos tu código es farragoso, repetitivo y no controla datos esenciales. Conviene por ejemplo que si alguna de las columnas tiene una restricción NOT NULL, verifiques que los valores para esas columnas contengan datos y evites la inserción, respetando así el contrato de NOT NULL en la base de datos y escribiendo un código coherente.

Conviene igualmente que verifiques cada dato que vendría en el $_POST, el usuario pudo haber mandando algunos datos vacíos, y te estarías exponiendo al menos a uno o más casos de Undefined index. La verificación del action también se puede simplificar. Y la coherencia del código en sí misma no se aprecia, por ejemplo, no veo si retornas algo desde el contexto en que insertas.

Dado que dices que haces un return $status te propondré una forma controlada de retornar lo que haya ocurrido en el método, usaré un array, porque te permite trabajar con pares clave => valor que puedes luego verificar y mostrar mensajes. Esto es más fácil de adaptar a una política de manejo de errores, o a respuestas de una API, etc.

include_once 'class/Estudiante.php';

#Declarar cada variable sólo cuando estés seguro de que la usarás
$json = array();

#¿Para qué es esta variable?    
$studentInfo = array();
/*
   Si usas PHP anterior a la v. 7
   cambia esto por:
   $action = !empty($_POST['action']) ? $_POST['action'] : null;
*/
$action=$_POST['action'] ?? null;
if ($action && $action == 'inscribir') {
    /*
       Aquí vamos a verificar cada dato para evitar
       casod de Undefined index
       Si tienes PHP v. anterior a 7 usa lo que
       ya se indicó más arriba para el action en todas
       las asignaciones que siguen
       Aquí convendría un if para verificar todas aquellas
       columnas que tiene restriccion NOT NULL
       Usaré una convención de nombre en inglés,
       entre otras ventajas, te evitarías 
       nombre de variables kilométricos, por ejemplo
       dob = date of birth (fecha de nacimiento)
       */
    
    $id=$_POST['estudiante_id'] ?? null;
    $age=$_POST['Edad'] ?? null;
    $dob=$_POST['Fecha_Nacimiento'] ?? null;
    $code=$_POST['Codigo'] ?? null;

    /*
       Aquí verificarías con un if TODOS
       los NOT NULL, por ejemplo
       if ($id && $code) {  // insertar } else { //faltan datos obligatorios }
       */

    $studentObj = new Estudiante();
    $studentObj->setStudentID($id);   
    # ¿Por qué la edad no es un campo calculado?    
    $studentObj->setEdad($age);
    $studentObj->setNacimiento($dob);        
    $studentObj->setPassword($code); 

    /*
       OJO A ESTO:
       En la nueva versión del código
       inscribir devolverá un array con dos posibilidades:
       1. Siempre una clave status que será true o false
       2. Una clave msg con un mensaje
       De este modo, la respuesta del método
       queda acoplada a la lógica que ya estabas siguiendo
       aunque quizá convendría refinarla un poco
       adpotando lo de la clave true o false           
       */ 
    $json = $studentObj->inscribir();


} else {
    $json['msg'] = 'No se ha enviado la acción inscribir';
}

header('Content-Type: application/json');   
echo json_encode($json); 
try {
        $cargo = "Ninguno";
        $fechaActual = date('Y-m-d');
        $salida = date('Y-m-d',strtotime($fecha_actual.'+ 1 year'));
        $RAMA= $this->_Rama;
        $integer = $this->_edad;
        $edad = (int)$integer;
        $sql = "INSERT INTO student(edad, password, fecha_nacimiento, fecha_creacion,fecha_salida)  VALUES (:edad, :pass ,:nacimiento, :inscripcion,:salida)";

         /*
             Vamos a poner los dos puntos a los marcadores
             de todos modos PDO lo haría por nosotros
             pero si se lo damos hecho mejor,
             así lo hacemos trabajar menos
         */
         $data = [
                ':edad' => $edad,               
                ':pass' => $this->_Password,               
                ':nacimiento' => $this->_Fecha_Nac,               
                ':inscripcion' => $fechaActual,                
                ':salida' => $salida                               
            ];

          $stmt = $this->db->prepare($sql);
          $stmt->execute($data);
          /*
             Como dije más arriba, aquí vamos a devolver
             un array con una clave status que será true
             y una clave msg que será un mensaje
              
             con la cantidad de filas afectadas
             */
             $status = array(
                               'status'=>true,
                               'msg' => sprintf("Se insertaron %s filas",$stmt->rowCount())
                             );            
        } catch (Exception $err) {          
            /*
               Aquí, para seguir con el contrato de nuestro código
               vamos a devolver un array con una clave status que es false
               y siempre una clave msg que será el mensaje de error
               En un contexto de Política de manejo de errores más avanzada
               podrías recoger el código de error por ejemplo
               e incorporarlo a un sistema de errores personalizados o cosas así ...
               Lo que es importante aquí es evitar
               que se muestren mensajes de error interno
               los cuales son MUY jugosos para los hackers
               Por ejemplo, un gravísimo fallo de PDO es que
               ante errores de conexión ¡revela la contraseña
               en el mensaje de error!
               */
            
            $status = array( 
                               'status'=>false,
                               'msg' => 'Error ejecutando la consulta. Si quieres captura el código de error e incorpóralo en tu PME'
                             );        
            return $status; 

        }
Origen Enlace
A. Cedano
  • 95.2k
  • 31
  • 145
  • 257

Tu código no presenta ningún riesgo de inyección SQL, pues estás usando correctamente las consultas preparadas y pasando correctamente los valores, los cuales se pueden pasar usando los métodos bindParam(), bindValue() o en el método execute(). La inyección SQL sólo es posible si tú pasas los valores directamente en la instrucción SQL que uses, sea mediante prepare() sea mediante query(). Al poner marcadores (de :nombre o de posición ?, sin mezclar), en la instrucción SQL evitas que un usuario mal intencionado inyecte código a través de lo que serían los valores de la consulta.

Sin embargo, en otros aspectos tu código es farragoso, repetitivo y no controla datos esenciales. Conviene por ejemplo que si alguna de las columnas tiene una restricción NOT NULL, verifiques que los valores para esas columnas contengan datos y evites la inserción.

Conviene igualmente que verifiques cada dato que vendría en el $_POST, el usuario pudo haber mandando algunos datos vacíos, y te estarías exponiendo al menos a un caso de Undefined index. La verificación del action también se puede simplificar. Y la coherencia del código en sí misma no se aprecia, por ejemplo, no veo si retornas algo desde el contexto en que insertas.

Aplicando un poco lo dicho podemos mejorar el código de este modo:

Donde recibes el $_POST

include_once 'class/Estudiante.php';

#Declarar cada variable sólo cuando estés seguro de que la usarás
$json = array();

#¿Para qué es esta variable?    
$studentInfo = array();
/*
   Si usas PHP anterior a la v. 7
   cambia esto por:
   $action = !empty($_POST['action']) ? $_POST['action'] : null;
*/
$action=$_POST['action'] ?? null;
if ($action && $action == 'inscribir') {
    /*
       Aquí vamos a verificar cada dato para evitar
       casod de Undefined index
       Si tienes PHP v. anterior a 7 usa lo que
       ya se indicó más arriba para el action en todas
       las asignaciones que siguen
       Aquí convendría un if para verificar todas aquellas
       columnas que tiene restriccion NOT NULL
       Usaré una convención de nombre en inglés,
       entre otras ventajas, te evitarías 
       nombre de variables kilométricos, por ejemplo
       dob = date of birth (fecha de nacimiento)
       */
    
    $id=$_POST['estudiante_id'] ?? null;
    $age=$_POST['Edad'] ?? null;
    $dob=$_POST['Fecha_Nacimiento'] ?? null;
    $code=$_POST['Codigo'] ?? null;

    /*
       Aquí verificarías con un if TODOS
       los NOT NULL, por ejemplo
       if ($id && $code) {  // insertar } else { //faltan datos obligatorios }
       */

    $studentObj = new Estudiante();
    $studentObj->setStudentID($id);   
    # ¿Por qué la edad no es un campo calculado?    
    $studentObj->setEdad($age);
    $studentObj->setNacimiento($dob);        
    $studentObj->setPassword($code);
    $status = $studentObj->inscribir();

    # Abreviamos con un ternario
    $json['msg'] = !empty($status) ? 'success' : 'failed';

} else {
    $json['msg'] = 'No se ha enviado la acción inscribir';
}

header('Content-Type: application/json');   
echo json_encode($json); 

Donde haces la inserción

try {
        $cargo = "Ninguno";
        $fechaActual = date('Y-m-d');
        $salida = date('Y-m-d',strtotime($fecha_actual.'+ 1 year'));
        $RAMA= $this->_Rama;
        $integer = $this->_edad;
        $edad = (int)$integer;
        $sql = "INSERT INTO student(edad, password, fecha_nacimiento, fecha_creacion,fecha_salida)  VALUES (:edad, :pass ,:nacimiento, :inscripcion,:salida)";

         /*
             Vamos a poner los dos puntos a los marcadores
             de todos modos PDO lo haría por nosotros
             pero si se lo damos hecho mejor,
             así lo hacemos trabajar menos
         */
         $data = [
                ':edad' => $edad,               
                ':pass' => $this->_Password,               
                ':nacimiento' => $this->_Fecha_Nac,               
                ':inscripcion' => $fechaActual,                
                ':salida' => $salida                               
            ];

          $stmt = $this->db->prepare($sql);
          $stmt->execute($data);
          /*
             No queda claro cómo se acopla esto
             con el código anterior
             Por otra parte, si el código entra en el catch
             No estarías controlando adecuadamente en el llamador
             Convendría quizá responder con un array
             con clave success aquí y con clave fail en el catch
             Así verificarías posibles errores aquí
             No deberías resolver con un simple die
             el cual rompe la COHERENCIA
             */
             $status = $stmt->rowCount();            
        } catch (Exception $err) {          
            /*
               Lo dicho en el comentario anterior
               esto rompe la COHERENCIA del código
               ¿Cómo controlarás esto desde el llamador?
               ¿O quizá son sólo suposiciones mías?
               Sin ver todo el contexto no puedo saberlo...
               Por otra parte EVITA mostrar errores internos
               Un gravísimo fallo de PDO por ejemplo es que
               ante errores de conexión ¡revela la contraseña!
               en el mensaje de error!
               */
            die("Error!: " . $err);
        }