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

Configure: Check source and build dir equality a little more thoroughly #12337

Closed
wants to merge 3 commits into from

Conversation

@levitte
Copy link
Member

@levitte levitte commented Jul 1, 2020

'absolutedir' does a thorough job ensuring that we have a "real" path
to both source and build directory, unencumbered by symbolic links.
However, that isn't enough on case insensitive file systems on Unix
flavored platforms, where it's possible to stand in, for example,
/PATH/TO/Work/openssl, and then do this:

perl ../../work/openssl/Configure

... and thereby having it look like the source directory and the build
directory aren't the same.

We solve this by having a closer look at the computed source and build
directories, and making sure they are exactly the same strings if they
are in fact the same directory.

This is especially important when making symbolic links based on this
directories, but may have other ramifications as well.

Fixes #12323

'absolutedir' does a thorough job ensuring that we have a "real" path
to both source and build directory, unencumbered by symbolic links.
However, that isn't enough on case insensitive file systems on Unix
flavored platforms, where it's possible to stand in, for example,
/PATH/TO/Work/openssl, and then do this:

    perl ../../work/openssl/Configure

... and thereby having it look like the source directory and the build
directory aren't the same.

We solve this by having a closer look at the computed source and build
directories, and making sure they are exactly the same strings if they
are in fact the same directory.

This is especially important when making symbolic links based on this
directories, but may have other ramifications as well.

Fixes #12323
@levitte
Copy link
Member Author

@levitte levitte commented Jul 1, 2020

Do note that this is a problem in 1.1.1 as well.

@levitte
Copy link
Member Author

@levitte levitte commented Jul 1, 2020

@mouse07410, I would appreciate it if you tried this.

@levitte
Copy link
Member Author

@levitte levitte commented Jul 1, 2020

This is in WIP until I've conducted tests where I can (Cygwin & Mingw)

@levitte
Copy link
Member Author

@levitte levitte commented Jul 1, 2020

One might ask the question, "why would you call Configure like that???"... it's a valid question, but shouldn't really matter. However, from looking at details in #12323, it turns out that simple shell utilities like dirname could play tricks and canonicalise the path while it's doing its work, and thereby implicitly cause exactly the kind of perl command line that's shown in the description above.

@levitte levitte changed the title [WIP] Configure: Check source and build dir equality a little more thoroughly Configure: Check source and build dir equality a little more thoroughly Jul 1, 2020
@levitte
Copy link
Member Author

@levitte levitte commented Jul 1, 2020

Verified on Cygwin and Mingw, so taking this out of WIP

Configure Outdated Show resolved Hide resolved
Copy link
Member

@mattcaswell mattcaswell left a comment

Thanks - that looks better now.

@mattcaswell
Copy link
Member

@mattcaswell mattcaswell commented Jul 1, 2020

Hmm, appveyor failure looks relevant

@levitte
Copy link
Member Author

@levitte levitte commented Jul 1, 2020

Hmm, appveyor failure looks relevant

Ah, right, File::Spec uses File::Spec::Win32, which requires File::Spec::Unix, so File::Spec->isa('File::Spec::Unix') ends up true rather than false since it follows the chain of packages to the end. I've added a fixup that looks directly at @file::Spec::ISA instead.

@romen
Copy link
Member

@romen romen commented Jul 1, 2020

@levitte it might make sense if this is back ported to 1.1.1 to also look into #11940 !

@openssl-machine
Copy link

@openssl-machine openssl-machine commented Jul 2, 2020

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@levitte
Copy link
Member Author

@levitte levitte commented Jul 2, 2020

@levitte it might make sense if this is back ported to 1.1.1 to also look into #11940 !

I'll look at that separately

openssl-machine pushed a commit that referenced this pull request Jul 2, 2020
'absolutedir' does a thorough job ensuring that we have a "real" path
to both source and build directory, unencumbered by symbolic links.
However, that isn't enough on case insensitive file systems on Unix
flavored platforms, where it's possible to stand in, for example,
/PATH/TO/Work/openssl, and then do this:

    perl ../../work/openssl/Configure

... and thereby having it look like the source directory and the build
directory aren't the same.

We solve this by having a closer look at the computed source and build
directories, and making sure they are exactly the same strings if they
are in fact the same directory.

This is especially important when making symbolic links based on this
directories, but may have other ramifications as well.

Fixes #12323

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #12337)
openssl-machine pushed a commit that referenced this pull request Jul 2, 2020
'absolutedir' does a thorough job ensuring that we have a "real" path
to both source and build directory, unencumbered by symbolic links.
However, that isn't enough on case insensitive file systems on Unix
flavored platforms, where it's possible to stand in, for example,
/PATH/TO/Work/openssl, and then do this:

    perl ../../work/openssl/Configure

... and thereby having it look like the source directory and the build
directory aren't the same.

We solve this by having a closer look at the computed source and build
directories, and making sure they are exactly the same strings if they
are in fact the same directory.

This is especially important when making symbolic links based on this
directories, but may have other ramifications as well.

Fixes #12323

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #12337)

(cherry picked from commit 610e2b3)
@levitte
Copy link
Member Author

@levitte levitte commented Jul 2, 2020

Merged

master:
610e2b3 Configure: Check source and build dir equality a little more thoroughly

1.1.1:
a98fa84 Configure: Check source and build dir equality a little more thoroughly

@levitte levitte closed this Jul 2, 2020
@levitte levitte deleted the levitte:fix-12323 branch Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants