Displaying text extracted from XML using PHP simplexml

Question

I have this xml file and i am parsing it with php simplexml.

My XML file looks like this:

<?xml version="1.0" encoding="UTF-8"?>
<noticias>
 <noticia url="noticia-1">
 <titulo>título da notícia 1</titulo>
 <desc>plain text.</desc>
 <texto>plain text.</texto>
 <img></img>
 <in>Publico</in>
 </noticia>
 ...
</noticias>

In my php page i parse it like this:

$file = 'xml/noticias.xml';
if(file_exists($file)) {
 $xml = simplexml_load_file($file);
 foreach($items as $item) {
 $titulo = htmlentities($item->titulo, ENT_QUOTES, 'utf-8');
 $desc = htmlentities($item->desc, ENT_QUOTES, 'utf-8');
 $url = htmlentities($item['url'], ENT_QUOTES, 'utf-8');
 if(strlen($titulo) < 3) {$titulo = 'T&iacute;tulo em falta';} else {$titulo = $titulo;}
 if(strlen($desc) < 3) {$desc = 'Descri&ccedil;&atilde;o em falta';} else {$desc = $desc;}
 if(strlen($url) < 3) {$h3 = '<h3>'.$titulo.'</h3>';} else {$h3 = '<a href="noticia/'.$url.'"><h3>'.$titulo.'</h3></a>';}
 ?>
 <div class="col-lg-3 col-md-6">
 <div class="item">
 <div class="content">
 <?php echo $h3; ?>
 <p><?php echo $desc; ?></p>
 </div>
 </div>
 </div>
 <?php
 }
}
else {
 // do something... throw error message
}

Is this ok? i mean, i escape values when i get from xml. Is it ok to do it like this or should i escape values on echo? is there any danger to leave as i have?

other thing... i have the xml files protected with htaccess. right now, they can only be edited directly. no scripts to edit them.

Answer 1

My preference is to leave the html escaping until the last minute.

Here are some changes you could make to simplify your code.

Code should work but there is no link between $xml and $items, so unable to test.

<?php
$file = 'xml/noticias.xml';
// guard clause exit early, instead of last
if (!file_exists($file)) {
 // do something... throw error message
 throw new \Exception('File does not exist');
}
$xml = simplexml_load_file($file);
foreach ($items as $item) {
 $titulo = $item->titulo;
 $desc = $item->desc;
 $url = $item['url'];
 // use mb_ function as you are dealing with unicode text, strlen will report incorrect length
 if (mb_strlen($titulo) < 3) {
 $titulo = 'Título em falta';
 }
 // no need to reassign value to itself
 //else {
 // $titulo = $titulo;
 //}
 if (mb_strlen($desc) < 3) {
 $desc = 'Descrição em falta';
 }
// else {
// $desc = $desc;
// }
 // not expecting url to be unicode
 if (strlen($url) >= 3) {
 $url = "noticia/{$url}";
 } else {
 $url = null;
 }
 $h3 = $titulo;
// $h3 = '<h3>' . $titulo . '</h3>';
// } else {
// $h3 = '<a href="noticia/' . $url . '"><h3>' . $titulo . '</h3></a>';
// }
 ?>
 <div class="col-lg-3 col-md-6">
 <div class="item">
 <div class="content">
 <?php if ($url): ?>
 <a href="<?php echo $url; ?>">
 <?php endif; ?>
 <h3><?php echo htmlentities($h3); ?></h3>
 <?php if ($url): ?>
 </a>
 <?php endif; ?>
 <p><?php echo htmlentities($desc); ?></p>
 </div>
 </div>
 </div>
 <?php
}

• Your Common Sense: you are making avery common mistake here, doing a job that PHP can do, and making it much worse than PHP. When a file is not found, PHP will throw an error already and it will be much more informative than a blunt "file not found". If you aren't going to handle the case but want just to throw an error then just refrain from using file_exists() at all
• bumperbox: Interesting point, I tried removing file_exists() and I get a warning. How can I catch a warning? PHP Warning: simplexml_load_file(): I/O warning : failed to load external entity "xml/noticias.xml"
• Your Common Sense: you can catch a warning by implementing an error handler that converts all errors to exceptions. but the point here is the question, what are you going to do with the caught exception? if nothing particular, then there is no point in throwing it, as it will end up as a fatal error all the same. if you're going to handle it somehow, then there is no point in throwing again, as file_exists would serve for the purpose as well. So my previous point was rather: using file_exists() with throw makes little sense. either write a handling code, or, if you want an error, let PHP to throw it
• bumperbox: Fair point, I will let the OP decide, to be honest, I just added throw() where they put the comment. "// do something... throw error message" assuming they knew what they were going to do with it.