php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #58399 Resource creation overcomplicated and allowing to make silly mistakes
Submitted: 2008-10-29 15:15 UTC Modified: -
From: anrdaemon at freemail dot ru Assigned:
Status: Open Package: mailparse (PECL)
PHP Version: Irrelevant OS: Irrelevant
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: anrdaemon at freemail dot ru
New email:
PHP Version: OS:

 

 [2008-10-29 15:15 UTC] anrdaemon at freemail dot ru
Description:
------------
I've discovered the mailparse package and it looks like it ready to serve my intentions the smartest possible way, but I've found some nasty inconsistences.

See this code:

<?php

if($msg = mailparse_msg_parse_file('a.eml'))
{
  $rc = mailparse_msg_get_structure($msg);
  $_rc = var_export($rc, true);
  echo "- message structure\n{$_rc}\n";

  foreach($rc as $section)
  {
    $part = mailparse_msg_get_part($msg, $section);
    $data = mailparse_msg_get_part_data($part);
    $_d = var_export($data, true);
    echo "- section {$section} details\n{$_d}\n";

    $body = mailparse_msg_extract_part_file($part, 'b.eml', NULL);
    echo "- section {$section} body\n{$body}\n- section {$section} end\n";
  }
}

?>

Interesting? Another example? See:

<?php

$buffer = "To: my@localhost\r\n\r\nHello myself!\r\n";
$trash = "This is crap";
$msg = mailparse_msg_create();

if(mailparse_msg_parse($msg, $buffer))
{

  $rc = mailparse_msg_get_structure($msg);
  $_rc = var_export($rc, true);
  echo "- message structure\n{$_rc}\n";

  foreach($rc as $section)
  {
    $part = mailparse_msg_get_part($msg, $section);
    $data = mailparse_msg_get_part_data($part);
    $_d = var_export($data, true);
    echo "- section {$section} details\n{$_d}\n";

    $body = mailparse_msg_extract_part($part, $trash, NULL);
    echo "- section {$section} body\n{$body}\n- section {$section} end\n";
  }
}

?>

And even worse, mixing things together:

<?php

$buffer = "To: my@localhost\r\n\r\nHello myself!\r\n";
$trash = "This is crap";
$msg = mailparse_msg_create();

if(mailparse_msg_parse($msg, $buffer))
{

  $rc = mailparse_msg_get_structure($msg);
  $_rc = var_export($rc, true);
  echo "- message structure\n{$_rc}\n";

  foreach($rc as $section)
  {
    $part = mailparse_msg_get_part($msg, $section);
    $data = mailparse_msg_get_part_data($part);
    $_d = var_export($data, true);
    echo "- section {$section} details\n{$_d}\n";

    $body = mailparse_msg_extract_part_file($part, $buffer, NULL);
    echo "- section {$section} body\n{$body}\n- section {$section} end\n";
  }
}

?>

It is no problem to discover such issue in test example, but in real project it is not that easy.
Now, what I want to ask, is "why it is ever possible to write such code"?

Wouldn't it be more understandable to eliminate the need of second argument in msg_extract_part function?
You're creating a file handler internally anyway. Keep it opened for the time script working with it, destroy when script is finished or when resource are freed.

In the end, it would looks like this:
Working with existing file:

$mimeResource = mailparse_msg_parse_file(filename);

As it is read-only handler, operations like mailparse_msg_parse($mimeResource)
will fail with "Warning: read-only resource".

Creating new entity:

$mimeResource = mailparse_msg_create([filename]);
while($chunk = getDataFromSomewhere())
{
  if(mailparse_msg_parse($mimeResource, $chunk))
  {
    continue;
  }
  else
  {
    throw new Exception('Boom!');
  }
}

If filename supplied, new file should be created at that location and stored when script exits. If no name is given, it will create a temporary file (using
standard mechanism for creating temporary files) and destroy it at exit.

mimeResource should contain internal file handler to created/opened filename, thus no need to supply any additional info when working with it, and as a result - lower possibility to make such mistakes, as in examples above.

If this scheme will be implemented, the mailparse_msg_extract_*part_file commands becomes redundant, and mailparse_msg_extract_*part get it's number of arguments reduced to two - the mimemail resource and callback.

mailparse_msg_get_part would duplicate the file handler from the given mimemail resource, increasing number of "locks" on it.

mailparse_msg_free will decrease number of locks or release file if mimemail no longer using it.


Patches

Add a Patch

Pull Requests

Add a Pull Request

 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Nov 19 01:31:42 2017 UTC