0

First link goes to a question I've asked concerning an issue related to the use of Heredoc syntax I am unable to fix an issue concerning an HEREDOC syntax error

Second link goes to a question very similar to the above link Issue with heredoc and PHP

I'm facing a problem concerning the retrieving of data from a database in a php page. The problem is not concerned with Heredoc, the Heredoc is in the code but the problem is caused because of a sql syntax error.

<?php
     // riceve l'identifictivo di un regista e ne restituisce il nome completo
    function get_director($director_id) {

        global $db;

        $query = 'SELECT
                people_fullname
            FROM
                people
            WHERE
                people_id = ' . $director_id;
        $result = mysql_query($query, $db) or die(mysql_error($db));

        $row = mysql_fetch_assoc($result);
        extract($row);

        return $people_fullname;
    }

    // riceve l'identificativo di un attore principale e ne restituisce il nome 
    // completo
    function get_leadactor($leadactor_id) {

        global $db;

        $query = 'SELECT
                people_fullname
            FROM
                people
            WHERE
               people_id = ' . $leadactor_id;
        $result = mysql_query($query, $db) or die(mysql_error($db));

        $row = mysql_fetch_assoc($result);
        extract($row);

        return $people_fullname;                 
    }

    // riceve l'identificativo di un tipo di film e ne restituisce la 
    //descrizione
    function get_movie($type_id) {

        global $db;

        $query = 'SELECT
                movietype_label
            FROM
                movietype
            WHERE
                movietype_id = ' . $type_id;
        $result = mysql_query($query, $db) or die(mysql_error($db));

        $row = mysql_fetch_assoc($result);
        extract($row);

        return $movietype_label;
    }

    // funziona per calcolare se un film ha generato un profitto, una perdita o è
    // in pareggio
    function calculate_differences($takings, $cost) {

        $difference = $takings - $cost;

        if ($difference < 0) {
            $color = 'red';
            $difference = '$' . abs($difference) . ' million';
        } elseif ($difference > 0) {
            $color ='green';
            $difference = '$' . $difference . ' million';
        } else {
            $color = 'blue';
            $difference = 'broke even';
        }

        return '<span style="color:' . $color . ';">' . $difference . '</span>';        
    }

    // collegamento a MYSQL
    $db = mysql_connect('localhost', 'pippo', 'pluto') or 
        die ('Unable to connect. Check your connection parameters. ');
    mysql_select_db('moviesite', $db) or die(mysql_error($db));

    // recupera le informazioni
    $query = 'SELECT
            movie_name, movie_year, movie_director, movie_leadactor,
            movie_type, movie_running_time, movie_cost, movie_takings
        FROM
            movie
        WHERE
            movie_id = ' . $_GET['movie_id'];
    $result = mysql_query($query, $db) or die(mysql_error($db));

    $row = mysql_fetch_assoc($result);
    $movie_name        = $row['movie_name'];
    $movie_director    = get_director($row['movie_director']);
    $movie_leadactor   = get_leadactor($row['movie_leadactor']);
    $movie_year        = $row['movie_year'];
    $movie_running_time = $row['movie_running_time'] .' mins';
    $movie_takings     = $row['movie_takings'] . ' million';
    $movie_cost        = $row['movie_cost'] . ' million';
    $movie_health      = $calculate_differences($row['movie_takings'],
                              $row['movie_cost']);

    // mostra le informazioni
    echo <<<ENDHTML
    <html>
        <head>
             <title>Details and Reviews for: $movie_name</title>
        </head>
        <body>
        <div style="text-align: center;">
        <h2>$movie_name</h2>
        <h3><em>Details</em></h3>
        <table cellpadding="2" cellspacing="2"
         style="width: 70%; margin-left: auto; margin-right: auto;">
        <tr>
         <td><strong>Title</strong></strong></td>
         <td>$movie_name</td>
         <td><strong>Release Year</strong></strong></td>
         <td>$movie_year</td>
         </tr><tr>
         <td><strong>Movie Director</strong></td>
         <td>$movie_director</td>
         <td><strong>Cost</strong></td>
         <td>$$movie_cost<td/>
         </tr><tr>
         <td><strong>Lead Actor</strong></td>
         <td>$movie_leadactor</td>
         <td><strong>Takings</strong></td>
         <td>$$movie_takings<td/>
         </tr><tr>
         <td><strong>Running Time</strong></td>
         <td>$movie_running_time</td>
         <td><strong>Health</strong></td>
         <td>$movie_health<td/>
        </tr>
      </table></div>
      </body>
    </html>
ENDHTML;
?>

and this is the syntax error I'm dealing with:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 7

Community
  • 1
  • 1
Riccardo
  • 66
  • 2
  • 16
  • 1
    Please, [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). They are no longer maintained and are [officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). Learn about [prepared statements](http://en.wikipedia.org/wiki/Prepared_statement) instead, and consider using PDO, [it's not as hard as you think](http://jayblanchard.net/demystifying_php_pdo.html). *What is line 7?* – Jay Blanchard May 12 '15 at 15:59
  • I know that mysql_ +functions are deprecated but I'm following examples from a book anyway line "7" is the line where the error occurs in the snippet of code – Riccardo May 12 '15 at 16:05
  • 2
    This has NOTHING to do with the heredoc. That is a mysql query error you are getting from running a query that is not valid. That is the output from the `or die(mysql_error($db))` part. I would assume it is the third query because that is the only one that is 7 lines long. You are likely not passing a `$_GET['movie_id']` in the url. – Jonathan Kuhn May 12 '15 at 16:05
  • 1
    There is no nice way to put this, but this code is *completely* insecure and should be avoided at all costs. If you are learning this from a book, I would burn the book as it is more useful to start a fire than it is at teaching php. – Jonathan Kuhn May 12 '15 at 16:08
  • so what is your advice to solve the problem? @JonathanKuhn where do I have to correct the code to make the whole page work? thanks in advance for your help – Riccardo May 12 '15 at 16:09
  • I know that line 7 is the line where the error occirred but we would have no idea what line 7 is here. Please tell us what it is. – Jay Blanchard May 12 '15 at 16:10
  • as @JonathanKuhn has stated line 7 is the length of the query and not the position where the error occurs, I'm sorry for the mistake – Riccardo May 12 '15 at 16:13
  • Also, my above comment is in no way a criticism of *your* code or abilities. I'm not trying to discourage. You can't learn from a bad source and expect to be good at what you do. You only know what you have been taught. – Jonathan Kuhn May 12 '15 at 16:14
  • To fix it though, you would likely need to test if the get variable has been set (`isset($_GET['var']`) and add that parameter to your where clause in the query only if it has been set. Or at the very least, set a default value if no value was passed. – Jonathan Kuhn May 12 '15 at 16:16
  • @JonathanKuhn under no circumstances am I mistaking your advice for a critic I'm here to learn from my mistakes otherwise I would never have subscribed to stack overflow, and I am aware to be a php beginner thus I'm trying to be helped by you guys surely a way better then me – Riccardo May 12 '15 at 16:17
  • @JonathanKuhn could you please provide me an answer to my question? thus not only would I upvote it but I will be able to try out what you mean – Riccardo May 12 '15 at 16:19

1 Answers1

1

OK, this is a quick fix. It goes very little into making the code secure or safe, but it will help this error and at least prevent one injection issue.

At the top of your code you need to check if $_GET['movie_id'] isset. It would also be prudent to put a simple is_numeric check which will prevent the value in the url from being something other than a number. If the id passed is not a number (as an id should be), then it is as good as not being passed. I would likely use something more concise like a ternary statement, but this is more readable and understandable to someone new. Put something like this at the top of your script before you build and run that first query (can be before or after declaring the functions):

//if the movie id was passed in the url and is a number
if(isset($_GET['movie_id']) && is_numeric($_GET['movie_id'])){
    //get it
    $movie_id = $_GET['movie_id'];
} else {
    //stop the code and display error.
    die("Movie id not found.");
}

Then down below in the query, replace $_GET['movie_id'] with the variable $movie_id.

Another issue I saw is you have and will run into, $calculate_differences is a function name, not a variable. Remove the $ from before it.

Also, you should likely check if the result from your first query there has any rows with mysql_num_rows. It is entirely possible that someone passes a movie id that doesn't exist.

Jonathan Kuhn
  • 15,279
  • 3
  • 32
  • 43
  • where exactly in the top of my code? can I write it anywhere? – Riccardo May 12 '15 at 16:53
  • hey it return movie id not found, as a matter of fact it seems that movie_id does not exist anywhere – Riccardo May 12 '15 at 16:56
  • If it returned "movie id not found", then you are not passing a movie id in the url. You need to load the page like: `page.php?movie_id=1234` where `1234` is a movie id from your database. Typically this page is linked to from another page that lists all the movies. – Jonathan Kuhn May 12 '15 at 17:27
  • hey the fact is that I do have a main page from which i can access the table i will show you – Riccardo May 13 '15 at 08:14
  • it's fine. I understand how this is supposed to work as it is pretty much the standard for websites. If you are still getting "movie id not found" though, just double check the spelling of `movie_id` in the url as well as in the code above. If that doesn't help, do a `print_r` of `$_GET` and double check that the query string is populating the `$_GET` array. If you don't see it, you likely have something like an htaccess file that is stripping off the query string. If so, easy fix, add the `[QSA]` flag to the end of your rule. If you don't understand any of this, do you best and let me know. – Jonathan Kuhn May 13 '15 at 15:18