Skip to content
This repository was archived by the owner on Mar 20, 2024. It is now read-only.

Screenshot filename has invalid characters #253

Open
ollietb wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Screenshot filename has invalid characters #253

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Behat/MinkExtension/Context/RawMinkContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public function saveScreenshot($filename = null, $filepath = null)
{
// Under Cygwin, uniqid with more_entropy must be set to true.
// No effect in other environments.
$filename = $filename ?: sprintf('%s_%s_%s.%s', $this->getMinkParameter('browser_name'), date('c'), uniqid('', true), 'png');
$filename = $filename ?: sprintf('%s_%s_%s.%s', $this->getMinkParameter('browser_name'), date('Y-m-d_H.i.s'), uniqid('', true), 'png');
Copy link
Copy Markdown

@aik099 aik099 Jul 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe it is better to:

  1. use date('c') as before
  2. afterwards apply regular expression, that would replace all non filename safe characters

?

Copy link
Copy Markdown
Author

@ollietb ollietb Jul 21, 2016

Choose a reason for hiding this comment

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

@aik099 I'm not sure why that would be better. It would increase the amount of processing needed when we have full control over the filename anyway.

Copy link
Copy Markdown

@aik099 aik099 Jul 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

If you can guarantee, that other components in filename (e.g. browser_name and uniqid) won't ever contain forbidden symbols, then of course keep PR code as-as.

$filepath = $filepath ?: (ini_get('upload_tmp_dir') ? ini_get('upload_tmp_dir') : sys_get_temp_dir());
file_put_contents($filepath . '/' . $filename, $this->getSession()->getScreenshot());
}
Expand Down