[JingleDesigns]

Hi guys, been looking around DT for a few months now and finally decided to join up

I really started off in designing rather then coding, but soon learnt you need a bit of both really to get you anywhere! So for this reason I have started on the long and windy road to learning PHP and MySQL. For my first project I have decided to build my own CMS. All was going well till I decided to use a column out of one of my tables to populate the <h1> tag. I am using the below;

	function get_subject_by_id($subject_id) {
		global $connection;
		$query = "SELECT * ";
		$query .= "FROM subjects ";
		$query .= "WHERE id=" . $subject_id;
		$query .= " LIMIT 1";
		$result_set = mysql_query($query, $connection);
		confirm_query($result_set);
		// if no rows are returned fetch_arrey will return false
		if ($subject = mysql_fetch_array($result_set)) {
			return $subject;
		} else {
			return NULL;
		}
	}

And this to my actual page;

<?php $sel_subject = get_subject_by_id($sel_subj); ?>

<h1><?php echo $sel_subject['menu_name']; ?></h1>

This works fine if I access the page via

content.php?subj=5

However if I just try

content.php

I get the below error;

Database query failed: 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 'LIMIT 1' at line 1

Am I missing something really daft here, or is there an issue with the LIMIT function with version 5?

[karmedic]

Does 'subj' feed the sql query at all?

[JingleDesigns]

Originally Posted by karmedic

Does 'subj' feed the sql query at all?

I have called subj as below;

<?php if (isset($_GET['subj'])) {
		$sel_subj = $_GET['subj'];
		$sel_page = "";
	} elseif (isset($_GET['page'])) {
		$sel_page = $_GET['page'];
		$sel_subj = "";
	} else {
		$sel_subj = "";
		$sel_page = "";
	}
	$sel_subject = get_subject_by_id($sel_subj);
?>

Does that look about right to you?

[Turv]

It looks to me like your making a call to get_subject_by_id when there may not be a value assigned to $sel_subj.

So when you go to content.php (No subj value) the call is still made to your get_subject_by_id funciton, but as no value is passed, your Query looks like this.

PHP Code:

// "SELECT * FROM subjects WHERE id=  LIMIT 1"; 


Which is why you receive the 'LIMIT' error.

Ensure you only call get_subject_by_id f $sel_subj has a value (Make the call within your IF, not outside of it)

[esuntu]

And to avoid sql injections You should check the variables before passing them to MySql. For example Your code is vulnerable (changing manually the content.php?subj=xxx I can execute my own sql commands) because You are passing variables straight to the sql.

The simplest way to do that is to use mysql_real_escape_string( ) or mysql_escape_string( ) functions to escape special characters in any string. But in my opinion the good practice is to only accept data that you're looking for.

For example in Your code the variable ID should be a integer greater than zero and nothing else:

PHP Code:

if(ctype_digit($_GET['subj']) ){ // checks that the variable is set and is a valid integer
 // the subj variable is safe
} 


[haku]

If you look at this piece of code:

<?php if (isset($_GET['subj'])) {
		$sel_subj = $_GET['subj'];
		$sel_page = "";
	} elseif (isset($_GET['page'])) {
		$sel_page = $_GET['page'];
		$sel_subj = "";
	} else {
		$sel_subj = "";
		$sel_page = "";
	}
	$sel_subject = get_subject_by_id($sel_subj);
?>

There are three different possibilities here of things that can happen.

1) $_GET['subj'] exists, and so $sel_subj is equal to whatever $_GET['subj'] is.
2 and 3) In these choices, $sel_subj is set to be empty.

If you look at the call to the function:

<?php $sel_subject = get_subject_by_id($sel_subj); ?>

This passes $sel_subj to the function (look inside the brackets) and uses it in the function here:

$query .= "WHERE id=" . $subject_id;

If $sel_subj is empty when you pass it to the function, then the above code will make id equal to nothing, since $subject_id is equal to nothing.

This is what is causing your error.

So if $_GET['subj'] isn't set, then your function will fail. You will have to build in a way to set a default value for $sel_subj for situations 2 and 3, where it is currently being set to nothing.

I hope that makes sense.