Maybe bug in post form and comment

Frank Vèssia · February 2, 2011

I'm using in my site a form that generated pages with form post fields.

This worked good until i setup a comment system in all my pages. Now when i use my form the system get me error like there are some empty fileds in my form but i guess this error is referred to the comment form that is not set in this kind of page.

ryan · February 2, 2011

I think we need to get a look at your whole template file. Chances are that your code to look for a form is just checking if there are posted values, rather than checking for posted values from a specific form.

If I'm correct about that, you would want to change your form posting code to save only if it detects the name of the submit button. So here is what I'm guessing it looks like now:

if($page->editable() && count($input->post)) {
    // save the form
}

What'd you want to do is change it to look like this:

if($page->editable() && $input->post->submit_form) {
    // save the form
}

Replace "submit_form" the the name assigned to your submit button. If it's just "submit", then you may want to change it to something more specific in this case.

If this isn't it, please post your template or email it to me and I can get a better look.

Frank Vèssia · February 2, 2011

Mmmm, i tried to change the submit name with no luck.. This is my template, divided in two files, the code that save the page and the form included.

insert-ad.php

<?php

$code = md5(uniqid(rand(), true));
$code = substr($code, 0, 10);

require_once('functions.php');

if($input->post->submit_annuncio) {

    // create a new Page instance
    $p = new Page();

    // set the template and parent (required)
    $p->template = $templates->get("annuncio");
    $categoria = $input->post->categoria;
    $p->parent = $pages->get("/".$categoria."/"); 
   
    // populate the page's fields with sanitized data
    // the page will sanitize it's own data, but this way no assumptions are made
    $p->hidden_path = $sanitizer->text($input->post->hidden_path); 
    
    $p->title = $sanitizer->text($input->post->title); 
    $p->nome = $sanitizer->text($input->post->nome); 
    $p->Email = $sanitizer->email($input->post->Email); 
    $p->citta = $sanitizer->text($input->post->citta); 
    $provincia = $input->post->provincia;
    $p->provincia = $pages->get("/annunci/".$provincia."/"); 
    $p->categoria = $pages->get("/".$categoria."/");
    $p->testo = $sanitizer->textarea($input->post->testo); 

    // PW2 requires a unique name field for pages with the same parent
    // make the name unique by combining the current timestamp with title
    $p->name = $sanitizer->pageName(rand(0,900)."-".tagliaUrl($p->title,50));
    
   /* $p2 = new page();
    $p2->template = $templates->get("citta");
    $p2->parent = $input->post->provincia;
    $p2->title = $sanitizer->text($input->post->citta); 
    $p2->name = $sanitizer->pageName(time() . $input->post->citta);*/

    if($p->title && $p->email && $p->citta) {
        // our required fields are populated, so save the page
        // you might want to email it here too
        $p->save(); 
        //$p2->save(); 
        echo "<h2>Grazie, il tuo annunci é stato inserito.</h2>";

    } else {
        // they missed a required field
        echo "<p class='error'>Errori nel form. Compila tutti i campi.</p>";  
    }

} else { ?>
<!DOCTYPE html>
<html lang="it">
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title><?php echo $page->get("headline|title"); ?></title>
<meta name="description" content="<?php echo $page->summary; ?>" />
<meta name="robots" content="noindex, nofollow" />
<link rel="stylesheet" type="text/css" href="<?php echo $config->urls->templates?>styles/main.css" />
<!--[if IE]>
<link rel="stylesheet" type="text/css" href="<?php echo $config->urls->templates?>styles/ie.css" />
<![endif]-->	
<script type="text/javascript" src="<?php echo $config->urls->templates?>scripts/jquery-1.4.2.min.js"></script>
<script type="text/javascript" src="<?php echo $config->urls->templates?>scripts/main.js"></script>
<script type="text/javascript" src="<?php echo $config->urls->templates?>scripts/jquery.validate.min.js"></script>
<script type="text/javascript" src="<?php echo $config->urls->templates?>scripts/jquery.metadata.js"></script>
<script type="text/javascript">
	function gotopage(page){
		window.location = page;
	}
</script>
<link href="/uploadify/uploadify.css" type="text/css" rel="stylesheet" />
<script type="text/javascript" src="/uploadify/swfobject.js"></script>
<script type="text/javascript" src="/uploadify/jquery.uploadify.v2.1.4.min.js"></script>
<script type="text/javascript">
$(document).ready(function() {
  $('#file_upload').uploadify({
    'uploader'  : '/uploadify/uploadify.swf',
    'script'    : '/uploadify/uploadify.php',
    'cancelImg' : '/uploadify/cancel.png',
    'folder'    : '/photos/',
    'buttonImg'   : '/uploadify/button.jpg',
    'width'       : 135,
    'fileExt'     : '*.jpg;*.jpeg',
    'multi'       : true,
    'queueSizeLimit' : 3,
    'sizeLimit'   : 307200,
    'auto'      : false,
    'scriptData'  : {'title':'<?=$code?>'},
    'onQueueFull'    : function (event,queueSizeLimit) {
      alert("Non puoi inserire altre immagini");
      return false;
    },
    'onAllComplete' : function(event,data) {
      alert('Immagini caricate. Ora clicca su Inserisci Annuncio');
    }
  });
  
  $("#form_annuncio").validate({
	rules: {
		email: {
			required: true,
			email: true
		}
	},
	messages: {
		email: "Inserisci una email valida"
	}
});

  $.metadata.setType("attr", "validate");
  
});
</script>

</head>
<body>
<div id="container">
<div id="header"><a href="/"><img src="/site/templates/styles/images/logo.png" /></a></div>
<? include('sidebar.inc'); ?>
<div id="main">
	<h2>Inserisci un annuncio gratuito</h2>
	<?php  include("./nuovo-annuncio.inc"); ?>
</div>
<div id="clear"></div>
</div><!-- container -->
<div id="footer">
<div id="inside">© 2010-2011 MF Network s.r.o.</div>
</div>
</body>
</html>
<? } ?>

nuovo-annuncio.inc

<form action='/inserisci-annuncio/' method='post' id="form_annuncio" name="form_annuncio" enctype="multipart/form-data">
    <input type="hidden" name="hidden_path" value="<?=$code?>" />
    <p>
    <label id="etichetta">Nome</label>
    <input type='text' name='nome' class="required" />
    </p>

    <p>
    <label id="etichetta">E-Mail</label>
    <input type='email' name='Email' id='Email' class="required" />
    </p>
    
     <p>
    <label id="etichetta">Provincia</label>
    <?php
		$province = $pages->get("/annunci/")->children("sort=provincia_regione"); 
		echo "<select name='provincia'><option value=''>Scegli la tua Provincia</option>";
		$regione = "";
		foreach($province as $provincia) {
			if ($regione == $provincia->provincia_regione){
				echo "<option value='{$provincia->name}'>  {$provincia->title}</option>";
			}else{
				echo "<option value='' disabled style='background-color:#dcdcc3;text-transform:uppercase'>{$provincia->provincia_regione}</option>";
				echo "<option value='{$provincia->name}'>  {$provincia->title}</option>";
			}
			$regione = $provincia->provincia_regione;
		}
		echo "</select>";
	?>
     </p>
    
     <p>
    <label id="etichetta">Cittá</label>
     <input type='text' name='citta' class="required" />
    </p>
    
 <p>
    <label id="etichetta">Categoria</label>
    <select name="categoria">
    <? foreach ($pages->find("template=categoria") as $categoria)
    echo "<option value='{$categoria->name}'>{$categoria->title}</option>"; ?>
    </select>
    </p>

    <p>
    <label id="etichetta">Titolo</label>
    <input type="text" name='title' id="title" style="width:300px" class="required" />
    </p>
    
    <p>
    <label id="etichetta">Annuncio</label>
    <textarea name='testo' cols="60" rows="6" class="required"></textarea>
    </p>
<p>
<label id="etichetta">Immagini</label>
	<div style="width:390px;float:left"><strong>Puoi inserire fino a 3 immagini .jpg di massimo 300 kb l'una.<br />Clicca sul bottone arancione per scegliere le tue immagini, dopo clicca su Carica Immagini.<br /></strong>
	<input type="file" id="file_upload" name="file_upload" />
	<a class="carica" href="javascript:$('#file_upload').uploadifyUpload();">Carica Immagini</a>
	</div><div style="clear:both"></div>
</p>
    <p>
    <input type='submit' name='submit_annuncio' value='Inserisci Annuncio' />
    </p>
</form>

Frank Vèssia · February 2, 2011

mmm, my last post with whole template has disappeared...but if a try to reply i can see it in replies queue...

Adam Kiss · February 2, 2011

akismet checked it as possible spam: we have to wait for ryan to uncheck it, because he still didn't give me enough rights to do it myself

ryan · February 2, 2011

$categoria = $input->post->categoria;
$p->parent = $pages->get("/".$categoria."/");

That code snippet above is not safe because $categoria is not validated/sanitized. Someone could really tear up your site using this current code, since you are using that to find the parent. For instance, what if they manipulated the form so that it was '/processwire/' or some prominent page on your site. Since you are using this to save pages, they could then go in and add a million pages to your navigation. I suggest that you do this instead:

$valid = array('categoria1', 'categoria2', 'categoria3'); 
$categoria = $sanitizer->pageName($input->post->categoria);
if(!in_array($categoria, $valid)) die('abort'); 
$p->parent = $pages->get("/$categoria/");

...and likewise for the other places where you are doing this. Or better yet, only deal with page IDs in your form, rather than page names. That's better because integers are very easy to validate and may be preferable here:

$valid = array(10,11,12); 
$categoria_id = (int) $input->post->categoria; 
if(!in_array($categoria_id, $valid)) die('abort'); 
$p->parent = $pages->get($categoria_id);

This is also not safe:

require_once('functions.php');

The problem with that is that PHP is searching all the include paths for 'functions.php'. It may very well find the wrong one. Instead, you want to ensure it just operates in the current path, so prefix it with a "./":

require_once('./functions.php');

I'm also a little concerned about what hidden_path is, and if there any security implications with that, but I just want to reiterate that you should never trust any user-submitted data. Assume that it is tainted. People will eventually exploit these issues. Don't use anything they submit in selectors, in page data, or echo anything back to the screen unless you've sanitized and validated it first.

As for why it's generating the error message... I can't immediately tell what the issue is or why you are getting something from comments (there doesn't appear to be any comments code here). It is possibly the include path issue I mentioned above, though I'm not counting on it. Can you paste in the exact error message that you are receiving?

Thanks,

Ryan

Frank Vèssia · February 2, 2011

Thanks for the suggests...

hidden_path is the name of the folder where i store images. I save this parameters as a field in my page and when i displays it i know where the images are.

Regarding the error i've found right now. I'm feel and idiot....it's the uppercase field Email....when the code checks mandatory fields before saving the page i wrote email and not Email... Sorry for this post.

Adam Kiss · February 2, 2011

re: categories: You can simplify the array generation with:

 $validC = array();
 foreach($pages->find('template=categoria') as $c) $validC[] = $c->name; //or id, title, whatever...

Regarding errors: I once somehow replaced:

  $var = ('this: $foo='.$foo);

with

  $var = ('this'); $foo='foo';

Because it seemed right (I was debugging and I had some wrong values, and somehow, I thought that that echo should have been actually something else. Took me 3 hours to figure that out.)

Also, listen to Ryan V he sometimes sounds like madman ;D when it comes to safety, but his [even large ones] pages are safe.

ryan · February 2, 2011

I make similar errors every day. It just goes with the territory. Glad you found the solution on that one.

Regarding the hidden_path. Assuming that is used to store something automatically at some point, I would suggest not making that part of your posted fields if at all possible. The reason is that someone could manipulate that value and have you overwriting stuff you shouldn't.

Frank Vèssia · February 2, 2011

re: categories: You can simplify the array generation with:
  $validC = array();
  foreach($pages->find('template=categoria') as $c) $validC[] = $c->name; //or id, title, whatever...

That's great because i have lots of categories..thx

Sign In

Maybe bug in post form and comment

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Create an account or sign in to comment

Create an account

Sign in

Recently Browsing 0 members