ansaurus

Question

PHP inserting Apostrophes where it shouldn't

Answer 1

+1 A:

Actually you tell to do put quotes around it by doing this:

 $tags  = $this->db->escape(set_value('tags'));

Your DB class does not know that you provide multiple values (how should it?). It treats the value getting from set_value('tags') as string and strings need to be escaped.

Later you explode that string:

 $pieces = explode(" ", $tags);

which should give you e.g.

$pieces[0] = "'test1";
$pieces[1] = "test2";
$pieces[2] = "test3'";

You are then concatenating the pieces again which in the end gives you: "'test1, test2, test3'".

The only thing that looks strange to me is the ending single quotation as you remove the last to characters from the string.

To solve the problem you can do this: Don't escape set_value('tags') beforehand. Escape the single values instead:

if ($pieces[0]!=NULL) {$ti1 .= "tag1, "; $ti2 .= $this->db->escape($pieces[0]) . ',';}

Another issue: I have no insight in your DB design but it looks like that tags should go into an own table (and then relating either via one-to-many or many-to-many relationship). Otherwise, you will be limited to only six tags. But that depends on the actual purpose.

Same for the browsers: What if a new browser gets on the market? You would have to extend your table. Better is to have a table with all browsers, e.g.

   id   |  browser
-------------------
   0    |    IE6
   1    |    IE7
   2    |    IE8
   3    |    FF2
etc.

and relate them via an intermediate table to the codes:

 --table code_browser

 code_id | browser_id
 --------------------
    0    |    0
    0    |    1
    0    |    3
    1    |    2
    2    |    1

 etc.

Felix Kling 2010-04-01 18:54:58

I presume second time you mean set_value('tags') but thank you for your answer. I'll investigate further :)

Jack Webb-Heller 2010-04-01 19:01:21

Answer 2

A:

A little confused by the question but from my understanding I think you have a couple of problems

a) For your ID column I would recommend using 0 and not '' (makes it a little more readable in my opinion.

b) You define your empty strings $ti1 = "";

Then you add to them

if ($pieces[0]!=NULL) {$ti1 .= "tag1, "; $ti2 .= "$pieces[0], ";} else {$ti1 .= "tag1, "; $ti2 .= "NULL, ";}

$ti1 is now equal to tag1,

  if ($pieces[1]!=NULL) {$ti1 .= "tag2, "; $ti2 .= "$pieces[1], ";} else {$ti1 .= "tag2, "; $ti2 .= "NULL, ";}

$ti1 is now equal to tag1, tag2,

What I think you should be doing is

  if ($pieces[0]!=NULL) {$ti1 .= "'tag1', "; $ti2 .= "$pieces[0], ";} else {$ti1 .= "'tag1', "; $ti2 .= "'NULL, ";}

(note the addition of the single quotes inside the double quotes, so instead of having $ti1 is now equal to tag1, tag2, you will have $ti1 is now equal to 'tag1', 'tag2', etc

c) I not entirely sure what these tags are meant to be representing but I would create a joining table with the id of the column and ONE tag. This means each id will match up to 5 rows in the joining table. This will allow you to construct better queries for counting etc on these tags.

bigstylee 2010-04-01 19:09:01

Answer 3

A:

A single variable is being given, so it's being treated as such.

You could use the if statements to build out your $sql query directly.

   $sql = "INSERT INTO code (id, title, author, summary, code, date";
        if ($pieces[0]!=NULL) {$sql .= ", tag1"; }
        if ($pieces[1]!=NULL) {$sql .= ", tag2"; }
    ...

etc.

micflan 2010-04-01 19:12:14

Answer 4

+2 A:

It's hard to be certain, but I believe this is the relevant line

$tags = $this->db->escape( set_value( 'tags' ) );

It's hard to say since I don't know what set_value() does

I'm going to guess that this turns the string test2 test2 test2 test2 test2 into 'test2 test2 test2 test2 test2' - those single-quotes stay in the string, even after you explode it and place it all together again.

But, I have to say, all of this code is really messy. In fact, the problem you're having is one you shouldn't even have to worry about (dynamically building an INSERT with variable columns). It shows me a weakness in your schema - tags should be in a N:M (many-to-many) relationship to the code table, not a hard limit of columns. The way you've done it here breaks the 2nd normal form of database normalization.

So, you can definitely go for the quick-fix and change how you're escaping these tag values, but I'd recommend updating your schema.

Peter Bailey 2010-04-01 19:14:18

Thank you Peter. set_value is basically CodeIgniter's shorthand for getting data from a $_POST[] variable - it would have been exactly the same to put $this->db->escape($_POST['tags']);

Jack Webb-Heller 2010-04-01 19:40:08

Also looking for an opinion: I am tempted to go with the quick fix. Yes, I'm sorry. And yes my code is messy and being a beginner I feel I should learn to fix it all up - but is there really any point getting a n-m database going when the web app needs users to be limited to 5 tags? I don't want any more than that. I want exactly 5 tags. Is it worth the hassle?

Jack Webb-Heller 2010-04-01 19:41:13

It's worth the hassle if you care about the performance of querying for a set of tags, or several tags. And like I mentioned, this breaks some pretty basic rules of schema design.

Peter Bailey 2010-04-01 22:33:31

It's worth the hassle, for all the reasons Peter Bailey mentioned, plus it will be vastly easier to write clean, short, non-redundant code with a more normalized schema.

Summer 2010-04-02 21:46:04

ansaurus

tags:

views:

answers:

PHP inserting Apostrophes where it shouldn't

related questions