6

While browsing Moodle's source code I stumbled across this:

repository/recent/lib.php

public function get_listing($encodedpath = '', $page = '') {
    global $OUTPUT;
    $ret = array();
    $ret['dynload'] = true;
    $ret['nosearch'] = true;
    $ret['nologin'] = true;
    $list = array();
    $files = $this->get_recent_files(0, $this->number);

    try {
        foreach ($files as $file) {
            $params = base64_encode(serialize($file));
            // Check that file exists and accessible
            $filesize = $this->get_file_size($params);
            if ($file['filename'] == 'image.png') {
              var_dump($filesize);
            }
            if (!empty($filesize)) {
                $node = array(
                    'title' => $file['filename'],
                    'size' => $filesize,
                    'date' => '',
                    'source'=> $params,
                    'thumbnail' => $OUTPUT->pix_url(file_extension_icon($file['filename'], 32))->out(false),
                );
                $list[] = $node;
            }
        }
    } catch (Exception $e) {
        throw new repository_exception('emptyfilelist', 'repository_recent');
    }
    $ret['list'] = array_filter($list, array($this, 'filter'));
    return $ret;
}

repository/lib.php

public function get_file_size($source) {
    $browser    = get_file_browser();
    $params     = unserialize(base64_decode($source));
    $contextid  = clean_param($params['contextid'], PARAM_INT);
    $fileitemid = clean_param($params['itemid'], PARAM_INT);
    $filename   = clean_param($params['filename'], PARAM_FILE);
    $filepath   = clean_param($params['filepath'], PARAM_PATH);
    $filearea   = clean_param($params['filearea'], PARAM_AREA);
    $component  = clean_param($params['component'], PARAM_COMPONENT);
    $context    = get_context_instance_by_id($contextid);
    $file_info  = $browser->get_file_info($context, $component, $filearea, $fileitemid, $filepath, $filename);
    if (!empty($file_info)) {
        $filesize = $file_info->get_filesize();
    } else {
        $filesize = null;
    }
    return $filesize;
}

My question is what is the purpose of base64 encoding and serializing this when it's immediately undone once inside the function? Is there a valid reason for doing this or is this just over engineered?

j0k
  • 22,600
  • 28
  • 79
  • 90
Kyle Decot
  • 20,715
  • 39
  • 142
  • 263
  • No valid reason for doing this, unless this is a really big exception and the most common usage of `get_file_size` by far is to get the file size of a `serialize`d, `base64_encode`d object. – Ry- Oct 08 '12 at 17:30
  • [`function get_file_size`](http://xref.schoolsict.net/moodle/2.2/nav.html?repository/lib.php.source.html#l1367) in moodle source – air4x Oct 08 '12 at 17:32
  • Can you show the code that follows the call to `$this->get_file_size()`? – NullUserException Oct 08 '12 at 17:43
  • @NullUserException I'm not sure why the return value of `get_file_size` matters as the serialization/deserialization has already occurred but in any case I've updated my question to include both methods that are being called. – Kyle Decot Oct 08 '12 at 18:14
  • 1
    I see that in `get_listing()`, `$params` is used further down (`$node['source']`), which leads me to believe it is serialized and base64 encoded for storage somewhere. So why not just pass `$file` to `$this->get_file_size()`? Well, I suppose this `get_file_size()` is used by other methods in moodle; methods that pass a serialized and base64 encoded string. – NullUserException Oct 08 '12 at 18:20
  • http://davidwalsh.name/php-serialize-unserialize-issues – Flukey Oct 10 '12 at 10:45

1 Answers1

2

A quick click around in their PHPXref reveals that this function is also called in repository_ajax.php

Passing serialised data around in a query string / form field simply isn't going to work (potentially), but with base64 encoding this is not an issue.

So the function was designed to handle requests delivered via ajax, base64 decode, deserialise, and then return the file size. (This is probably linked to the source element of the list node too)

In essence, the call is verifying that encoded string is valid, for when it is later used in the ajax request.

Leigh
  • 12,859
  • 3
  • 39
  • 60