OSSL_STORE and absolute devices #3907
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note: the tests should fail on Appveyor |
|
And now, with code to handle paths that start with a device name. Hopefully, Appveyor will be happier this time. (I'll have to try this with a device called "file" somewhere...) |
levitte
force-pushed
the
store2-and-absolute-devices
branch
2 times, most recently
from
7c93f99
to
6022a58
Compare
levitte
changed the title
|
This seems to do it, at least on Windows, so out of WIP it goes |
|
I've verified it on VMS as well. |
levitte
force-pushed
the
store2-and-absolute-devices
branch
3 times, most recently
from
602850f
to
a8e7165
Compare
dot-asm
reviewed
Jul 13, 2017
| #endif | ||
| } else { | ||
| path = uri; | ||
| if (stat(path_data[i].path, &st) < 0) { |
There was a problem hiding this comment.
It helps Windows if stat is redefined as _stat. See crypto/rand/randfile.c for an example. This kind of thing allows to remove dependency of the object code from compiler /M flag. I mean with stat redefined as _stat it's up to application which /M flag to use. And without it would occasionally cause link errors.
There was a problem hiding this comment.
Ah, good point, will do. Thanks for the reminder
|
On side note lack of empty lines between functions (and between functions and data) sting the eye. [I was under impression that it, empty lines between functions, was in coding style...] |
to_rel_file_uri really treated all files appropriately, absolute and relative alike, and really just constructs a URI, so gets renamed to to_file_uri to_file_uri, on the other hand, forces the path into an absolute one, so gets renamed to to_abs_file_uri
We haven't tested plain absolute paths without making them URIs...
… URIs To handle paths that contain devices (for example, C:/foo/bar.pem on Windows), try to "open" the URI using the file scheme loader first, and failing that, check if the device is really a scheme we know. The "file" scheme does the same kind of thing to pick out the path part of the URI. An exception to this special treatment is if the URI has an authority part (something that starts with "//" directly after what looks like a scheme). Such URIs will never be treated as plain file paths.
…h 'file:' These cases are performed on Linux only. They check that files with names starting with 'file:' can be processed as well.
If we have a local file with a name starting with 'file:', we don't want to check if the part after 'file:' is absolute. Instead, mark each possibility for absolute check if needed, and perform the absolute check later on, when checking each actual path.
levitte
force-pushed
the
store2-and-absolute-devices
branch
from
a8e7165
to
127914d
Compare
|
ping @dot-asm |
dot-asm
reviewed
Jul 14, 2017
| path_data[path_data_n].check_absolute = 1; | ||
| #ifdef _WIN32 | ||
| /* Windows file: URIs with a drive letter start with a / */ | ||
| if (p[0] == '/' && p[2] == ':' && p[3] == '/') { |
There was a problem hiding this comment.
Does it make sense to double-check that p[1] is 'a' to 'z'?
There was a problem hiding this comment.
Sure. Done, pushing in a moment
dot-asm
approved these changes
Jul 15, 2017
dot-asm
added
the
approval: done
levitte
added a commit
that referenced
this pull request
Jul 15, 2017
to_rel_file_uri really treated all files appropriately, absolute and relative alike, and really just constructs a URI, so gets renamed to to_file_uri to_file_uri, on the other hand, forces the path into an absolute one, so gets renamed to to_abs_file_uri Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #3907)
levitte
added a commit
that referenced
this pull request
Jul 15, 2017
We haven't tested plain absolute paths without making them URIs... Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #3907)
levitte
added a commit
that referenced
this pull request
Jul 15, 2017
Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #3907)
levitte
added a commit
that referenced
this pull request
Jul 15, 2017
… URIs To handle paths that contain devices (for example, C:/foo/bar.pem on Windows), try to "open" the URI using the file scheme loader first, and failing that, check if the device is really a scheme we know. The "file" scheme does the same kind of thing to pick out the path part of the URI. An exception to this special treatment is if the URI has an authority part (something that starts with "//" directly after what looks like a scheme). Such URIs will never be treated as plain file paths. Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #3907)
levitte
added a commit
that referenced
this pull request
Jul 15, 2017
…h 'file:' These cases are performed on Linux only. They check that files with names starting with 'file:' can be processed as well. Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #3907)
levitte
added a commit
that referenced
this pull request
Jul 15, 2017
If we have a local file with a name starting with 'file:', we don't want to check if the part after 'file:' is absolute. Instead, mark each possibility for absolute check if needed, and perform the absolute check later on, when checking each actual path. Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #3907)
levitte
added a commit
that referenced
this pull request
Jul 15, 2017
… named Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #3907)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is related to #3897
This is an attempt to solve the problem when OSSL_STORE is presented with a path that begins with a device name.