Skip to content
Snippets Groups Projects
Commit dc7e54d4 authored by Adam G-H's avatar Adam G-H Committed by Ted Bowman
Browse files

Issue #3417905: Prevent random build test failures by preventing stage IDs from starting with -

parent b0222f84
No related branches found
No related tags found
1 merge request!1022ltrim dashes from the stage ID
Pipeline #90223 failed
...@@ -6,7 +6,7 @@ namespace Drupal\package_manager; ...@@ -6,7 +6,7 @@ namespace Drupal\package_manager;
use Composer\Semver\VersionParser; use Composer\Semver\VersionParser;
use Drupal\Component\Datetime\TimeInterface; use Drupal\Component\Datetime\TimeInterface;
use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Random;
use Drupal\Core\Queue\QueueFactory; use Drupal\Core\Queue\QueueFactory;
use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\Core\StringTranslation\TranslatableMarkup; use Drupal\Core\StringTranslation\TranslatableMarkup;
...@@ -344,7 +344,13 @@ abstract class StageBase implements LoggerAwareInterface { ...@@ -344,7 +344,13 @@ abstract class StageBase implements LoggerAwareInterface {
// to create a stage directory at around the same time. If an error occurs // to create a stage directory at around the same time. If an error occurs
// while the event is being processed, the stage is marked as available. // while the event is being processed, the stage is marked as available.
// @see ::dispatch() // @see ::dispatch()
$id = Crypt::randomBytesBase64(); // We specifically generate a random 32-character alphanumeric name in order
// to guarantee that the the stage ID won't start with -, which could cause
// it to be interpreted as an option if it's used as a command-line
// argument. (For example,
// \Drupal\Component\Utility\Crypt::randomBytesBase64() would be vulnerable
// to this; the stage ID needs to be unique, but not cryptographically so.)
$id = (new Random())->name(32);
// Re-acquire the tempstore to ensure that the lock is written by whoever is // Re-acquire the tempstore to ensure that the lock is written by whoever is
// actually logged in (or not) right now, since it's possible that the stage // actually logged in (or not) right now, since it's possible that the stage
// was instantiated (i.e., __construct() was called) by a different session, // was instantiated (i.e., __construct() was called) by a different session,
......
...@@ -280,11 +280,7 @@ class ConsoleUpdateStage extends UpdateStage { ...@@ -280,11 +280,7 @@ class ConsoleUpdateStage extends UpdateStage {
* The ID of the current stage. * The ID of the current stage.
*/ */
protected function triggerPostApply(string $stage_id): void { protected function triggerPostApply(string $stage_id): void {
// The stage ID needs to be quoted in order to prevent it from being parsed $arguments = sprintf('post-apply %s', $stage_id);
// as a command-line option if it begins with -, which is a possibility
// because we use \Drupal\Component\Utility\Crypt::randomBytesBase64() to
// generate the stage ID, and the string it returns might begin with -.
$arguments = sprintf('post-apply "%s"', $stage_id);
if ($this->isFromWeb) { if ($this->isFromWeb) {
$arguments .= ' --is-from-web'; $arguments .= ' --is-from-web';
} }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment