Problem/Motivation

There is a bug in StageBase::stageDirectoryExists(). The problem is that it calls getStageDirectory(), which gives you a path that may not exist if we're in the PreCreate phase of the stage life cycle.

Proposed resolution

The method should return the result of is_dir($this->getStageDirectory()). It should look, overall, like this:

    try {
      return is_dir($this->getStageDirectory());
    }
    catch (\LogicException) {
      return FALSE;
    }

A reasonable way to test this is to add a simple PreCreateEvent listener (using addTestEventListener()) which confirms that stageDirectoryExists() returns TRUE, and that the stage directory actually exists ($this->assertDirectoryExists()).

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

Issue summary: View changes
wim leers’s picture

@phenaproxima How did you find this? How did this bug manifest?

(Tagging Needs issue summary update for this, but based on the current summary, this is already actionable, so keeping Active.)

wim leers’s picture

I happened to stumble upon the place where @phenaproxima discovered this.

This is a hard blocker for #3316617: Add a validator to check that PHP-TUF's Composer integration is present and configured correctly, so matching priority and tags.

phenaproxima’s picture

Assigned: Unassigned » phenaproxima

Self-assigning to fix.

phenaproxima’s picture

Assigned: phenaproxima » wim leers
Status: Active » Needs review

How did you find this? How did this bug manifest?

This manifested while writing a test for the PHP-TUF validator -- I need to get the allow-plugins config value for the stage directory, if it exists. So used stageDirectoryExists() -- only to discover that it is lying to me during pre-create, telling me that the stage directory exists when it doesn't.

I've now fixed this with a test, and confirmed that the test fails if I revert the fix.

tedbow’s picture

Status: Needs review » Reviewed & tested by the community

I confirmed that test fails without the fix. Looks good.

  • phenaproxima committed bb204bd0 on 3.0.x
    Issue #3358012 by phenaproxima, Wim Leers: StageBase::...
phenaproxima’s picture

Status: Reviewed & tested by the community » Fixed
wim leers’s picture

Assigned: wim leers » Unassigned
Issue tags: -Needs issue summary update

👍 Thanks for the context in #7! 😊 That addresses Needs issue summary update, and the issue is so short anyway, so untagging.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.