Bug 987155
| Summary: | Manifest#initialize should not be calling File.exists? on string contents of downloaded manifests | ||
|---|---|---|---|
| Product: | OpenShift Online | Reporter: | Clayton Coleman <ccoleman> |
| Component: | Containers | Assignee: | Paul Morie <pmorie> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | libra bugs <libra-bugs> |
| Severity: | medium | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 2.x | CC: | bmeng, dmcphers, pmorie, xtian |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2013-08-29 12:48:35 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
Commit pushed to master at https://github.com/openshift/origin-server https://github.com/openshift/origin-server/commit/0ee165a44d16cab7acb19ac5ca8d474c39bd0f55 Bug 987155 Checked on devenv_3632, the change has been merged. And create app form downloadable cartridge still works. Move the bug to verified. |
openshift-origin-common/models/manifest.rb#initialize is taking manifest as an argument, then calling File.exists? on that argument. If it returns false, it's assuming that the manifest is a downloaded manifest. This is bad code for a number of reasons (large manifests could raise spurious errors against some filesystems). Instead, #initialize should take a YAML document as "manifest", and the path as a second argument. If callers need to parse YAML, they should call a static method on Manifest that handles that behavior: def self.load_from_file(path) new(YAML.load_file(manifest), path, ...) end Also, the loading code that loads the manifest (@manifest = YAML.load_file(manifest)) is not using safe, which means that it's likely someone could use this incorrectly and expose a security issue. Medium because of the potential for problems.