Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Base64 encode binary data before logging #177

Merged
merged 3 commits into from Mar 18, 2013
Merged

Base64 encode binary data before logging #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e69509
Base64 encode MongoBinData objects before logging.
Mar 14, 2013
951b133
Swapped type and bin data when logging MongoBinData in the profiler.
Mar 15, 2013
e3c9482
Fix tests using MongoBinData to pass BYTE_ARRAY constant.
Mar 15, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion DataCollector/PrettyDataCollector.php
View file
Open in desktop
Expand Up @@ -216,7 +216,7 @@ private function bsonEncode($query, $array = true)
} elseif ($value instanceof \MongoMaxKey) {
$formatted = 'new MaxKey()';
} elseif ($value instanceof \MongoBinData) {
$formatted = 'new BinData("'.$value->bin.'", "'.$value->type.'")';
$formatted = 'new BinData('.$value->type.', "'.base64_encode($value->bin).'")';
} elseif ($value instanceof \MongoGridFSFile || $value instanceof GridFSFile) {
$formatted = 'new MongoGridFSFile("'.$value->getFilename().'")';
} elseif ($value instanceof \stdClass) {
Expand Down
6 changes: 6 additions & 0 deletions Logger/Logger.php
View file
Open in desktop
Expand Up @@ -48,6 +48,12 @@ public function logQuery(array $query)
$query['data'] = '**'.$query['num'].' item(s)**';
}

array_walk_recursive($query, function(&$value, $key) {
if ($value instanceof \MongoBinData) {
$value = base64_encode($value->bin);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this also be logged using BinData format above? Otherwise, the base64 string will be indistinguishable from a regular string.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't disagree. I was trying to remain consistent as no other types are currently treated special in this logger. I'll change it to use the same format as above.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While changing this, I realized the output will end up looking a little strange since it will be wrapped in quotes since it's just being json_encoded.

It'll end up looking like {"foo":{"bar":"new BinData(\"anVuayBkYXRh\", \"2\")"}}.

If that's fine, then I'll leave it as is. Otherwise, maybe just a tag like "BINARY: anVuayBkYXRh, TYPE: 2"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, do other objects that get logged simply get casted to strings?

It's been some time since I saw the logger output for this bundle, so your example helped jog my memory.

I suppose the most preferable option would be to serialize the driver classes in the "strict JSON" format defined in: http://docs.mongodb.org/manual/reference/mongodb-extended-json/#data_binary (the mongoexport commands use that notation), but that would be outside the scope of this PR. Let's leave it as-is for now, then.

}
});

$this->logger->info($this->prefix.json_encode($query));
}
}
10 changes: 6 additions & 4 deletions Tests/DataCollector/PrettyDataCollectorTest.php
View file
Open in desktop
Expand Up @@ -36,16 +36,18 @@ public function testCollect($query, $formatted)
public function getQueries()
{
return array(
// batchInsert
array(
'batch insert' => array(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove these array keys? This isn't used by PHPUnit and is redundant given the inline comments above.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The array keys are used by PHPUnit to name the data set being used. In debug output or when a test fails, instead of having "data set #1" failed, it'll put out "data set "batch insert" failed".

However, if you don't want named datasets, I can remove the keys.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I had no idea. That sounds great, then 👍. If you like, we can drop the comments instead.

array('db' => 'foo', 'collection' => 'bar', 'batchInsert' => true, 'num' => 1, 'data' => array('foo' => 'bar'), 'options' => array()),
array('use foo;', 'db.bar.insert({ "foo": "bar" });'),
),
// find
array(
'find' => array(
array('db' => 'foo', 'collection' => 'bar', 'find' => true, 'query' => array('foo' => null), 'fields' => array()),
array('use foo;', 'db.bar.find({ "foo": null });'),
),
'bin data' => array(
array('db' => 'foo', 'collection' => 'bar', 'update' => true, 'query' => array('_id' => 'foo'), 'newObj' => array('foo' => new \MongoBinData('junk data', \MongoBinData::BYTE_ARRAY))),
arraY('use foo;', 'db.bar.update({ "_id": "foo" }, { "foo": new BinData(2, "' . base64_encode('junk data') . '") });'),
)
);
}
}
28 changes: 28 additions & 0 deletions Tests/Logger/LoggerTest.php
View file
Open in desktop
Expand Up @@ -42,4 +42,32 @@ public function testLogQuery()
$logger = new Logger($this->logger);
$logger->logQuery(array('foo' => 'bar'));
}

public function testMongoBinDataBase64Encoded()
{
$binData = new \MongoBinData('junk data', \MongoBinData::BYTE_ARRAY);
$query = array('foo' => base64_encode($binData->bin));
$log = json_encode($query);

$this->logger->expects($this->once())
->method('info')
->with('MongoDB query: '.$log);

$logger = new Logger($this->logger);
$logger->logQuery(array('foo' => new \MongoBinData('junk data', \MongoBinData::BYTE_ARRAY)));
}

public function testMongoBinDataBase64EncodedRecursively()
{
$binData = new \MongoBinData('junk data', \MongoBinData::BYTE_ARRAY);
$query = array('foo' => array('bar' => base64_encode($binData->bin)));
$log = json_encode($query);

$this->logger->expects($this->once())
->method('info')
->with('MongoDB query: '.$log);

$logger = new Logger($this->logger);
$logger->logQuery(array('foo' => array('bar' => new \MongoBinData('junk data', \MongoBinData::BYTE_ARRAY))));
}
}