You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ossie.apache.org by "ayushtkn (via GitHub)" <gi...@apache.org> on 2026/10/07 21:17:00 UTC
[PR] fix(validation): report unreadable input or schema, not a traceback [ossie]
ayushtkn opened a new pull request, #534:
URL: https://github.com/apache/ossie/pull/534
## Summary
The exists() checks pass for a path that cannot be read as text, so these inputs reached a traceback while every neighbouring bad input got a clean line — a directory, a file without read permission, a binary file, and a --schema that is any of those or not JSON.
Read each file first and parse the text after, so one guard covers both files and neither read nests inside a parse:
Error: Could not read validation: [Errno 21] Is a directory: 'validation'
Error: Invalid JSON in schema junk.json: Expecting value: line 1 column 1
Error: model.yaml is not valid UTF-8 text: 'utf-8' codec can't decode ...
Exit status was already 1 in each case and is unchanged, as are the messages for a missing file, a missing schema and malformed YAML.
## Related Issues
<!-- Link any related GitHub issues: Closes #123, Fixes #456 -->
## Checklist
### Specification
- [ ] Spec changes are included in `core-spec/` and follow the existing structure
- [ ] Spec changes have been discussed on the mailing list or in a linked issue
- [ ] Breaking changes to the spec are clearly called out in the summary
### Ontology
- [ ] Ontology changes in `ontology/` are consistent with spec changes
- [ ] New or modified terms are defined and documented
### Converters
- [ ] Converter logic in `converters/` is updated to reflect spec or ontology changes
- [ ] New converters include tests under the converter's test directory
- [ ] If adding a new converter, `.github/labeler.yml` is updated with the new path
### Validation
- [ ] Validation rules in `validation/` are updated if the spec changed
- [ ] New validation cases are covered by tests
### Documentation
- [ ] `docs/` is updated to reflect any user-facing changes
- [ ] New features or behaviors are documented with examples where appropriate
- [ ] `CONTRIBUTING.md` is updated if the contribution process changed
### Examples
- [ ] `examples/` are added or updated for any new spec constructs or converter support
### Tests
- [ ] All existing tests pass (`pytest` / CI green)
- [ ] New functionality is covered by tests
### Compliance
- [ ] ASF license headers are present on all new source files
- [ ] No third-party dependencies are added without PMC/IPMC approval
- [ ] If third-party source code is vendored/copied (not just declared as a dependency), `NOTICE` and/or `LICENSE` have been updated per [ASF policy](https://infra.apache.org/licensing-howto.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(validation): report unreadable input or schema, not a traceback [ossie]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #534:
URL: https://github.com/apache/ossie/pull/534#discussion_r4212098185
##########
validation/validate.py:
##########
@@ -583,20 +595,26 @@ def main():
print(f"Error: Schema not found: {schema_path}")
sys.exit(1)
- # Load files
- with open(schema_path) as f:
- schema = json.load(f)
+ # Read first, then parse. The exists() checks above pass for a path that cannot be
+ # read as text — a directory, one without read permission, or a binary file — so
+ # reading is guarded to report like every other bad input rather than raise.
+ schema_text = read_text_or_exit(schema_path, f"schema {schema_path}")
+ try:
+ schema = json.loads(schema_text)
+ except json.JSONDecodeError as e:
+ print(f"Error: Invalid JSON in schema {schema_path}: {e}")
+ sys.exit(1)
- with open(yaml_path) as f:
- try:
- data = yaml.load(f, Loader=UniqueKeyLoader)
- except yaml.YAMLError as e:
- print(f"Error: Invalid YAML: {e}")
- sys.exit(1)
- except RecursionError:
- # Deeply nested input surfaces as RecursionError, not YAMLError.
- print("Error: Invalid YAML: input is too deeply nested to parse")
- sys.exit(1)
+ model_text = read_text_or_exit(yaml_path, str(yaml_path))
+ try:
+ data = yaml.load(model_text, Loader=UniqueKeyLoader)
Review Comment:
Passing a plain string to PyYAML changes every parser mark's source from the model path to `<unicode string>`; previously the opened file's `.name` appeared in malformed-YAML and duplicate-key diagnostics. This contradicts the stated preservation of malformed-YAML messages and makes errors less actionable. Parse through a named in-memory text stream (for example, `StringIO` with `.name` set to `yaml_path`) to retain the path while keeping the guarded read.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(validation): report unreadable input or schema, not a traceback [ossie]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #534:
URL: https://github.com/apache/ossie/pull/534#discussion_r4212190748
##########
validation/test_validate.py:
##########
@@ -352,6 +362,106 @@ def test_duplicate_key_exits_nonzero(self):
self.assertIn("Error: Invalid YAML", result.stdout)
self.assertIn("found duplicate key 'name'", result.stdout)
+ def run_validator_args(self, *args) -> subprocess.CompletedProcess[str]:
+ """Run validate.py with arbitrary arguments, for bad-path cases."""
+ return subprocess.run(
+ [sys.executable, Path(__file__).with_name("validate.py"), *args],
+ check=False,
+ capture_output=True,
+ text=True,
+ )
+
+ def test_directory_as_input_is_reported(self):
+ with tempfile.TemporaryDirectory() as temp_dir:
+ result = self.run_validator_args(temp_dir)
+
+ self.assertEqual(result.returncode, 1)
+ self.assertIn("Error: Could not read", result.stdout)
+ self.assertNotIn("Traceback", result.stderr)
+
+ @unittest.skipIf(
+ hasattr(os, "geteuid") and os.geteuid() == 0,
+ "root can read a file regardless of its mode",
+ )
Review Comment:
On platforms without `os.geteuid` (notably Windows), this condition is false, so the test runs even though `chmod(0o000)` does not make a file unreadable there. The subprocess can read the model and this assertion then fails; skip the permission test when POSIX effective-user semantics are unavailable.
This issue also appears on line 419 of the same file.
##########
validation/validate.py:
##########
@@ -157,6 +157,23 @@ def _check_unique_keys(self, node: yaml.Node, visited: set) -> None:
self._check_unique_keys(child, visited)
+def load_yaml_named(text: str, name: str):
+ """Parse text with UniqueKeyLoader, naming the source in any parser mark.
+
+ yaml.load() names a str source "<unicode string>", which drops the path from
+ malformed-YAML and duplicate-key diagnostics. Naming the loader restores it
+ and keeps the offending line, which PyYAML only records for a str source:
+ Reader.get_mark() passes its buffer to the Mark only when the input was not
+ a stream, so a named StringIO would name the file but lose the snippet.
+ """
+ loader = UniqueKeyLoader(text)
+ loader.name = name
+ try:
+ return loader.get_single_data()
+ finally:
+ loader.dispose()
Review Comment:
`UniqueKeyLoader(text)` can itself raise `ReaderError` before `loader.name` is assigned. For example, valid UTF-8 containing a NUL/control character is rejected during `Reader.__init__`, so the new diagnostic still names `<unicode string>` instead of the model path. Catch that constructor-time error, attach `name`, and add this case to the diagnostic test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(validation): report unreadable input or schema, not a traceback [ossie]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on PR #534:
URL: https://github.com/apache/ossie/pull/534#issuecomment-6053520162
Thanks for the PR @ayushtkn . I will take a look later this weekend.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(validation): report unreadable input or schema, not a traceback [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre merged PR #534:
URL: https://github.com/apache/ossie/pull/534
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org