-
Notifications
You must be signed in to change notification settings - Fork 48
ENH: meta command - Add Permissions + fix version read on encrypted PDFs #189
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
Open
iamrishu11
wants to merge
7
commits into
py-pdf:main
Choose a base branch
from
iamrishu11:feat/meta-permissions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
9e7fde4
meta: add Permissions + fix version read on encrypted; tidy Encrypted…
iamrishu11 07d2f15
TST: for the pr #189
iamrishu11 010496e
PI: removed extra table
iamrishu11 dbc29eb
DOC: for new feature and bug
iamrishu11 7d461f7
Update CHANGELOG.md
iamrishu11 b443e83
TST: made the unittest more robust
iamrishu11 26bc83c
TST: fixed file path issue
iamrishu11 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| """ | ||
| Unit tests for metadata module. | ||
| Tests the _format_permissions function and the meta CLI command. | ||
| """ | ||
|
|
||
| import json | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| from pdfly.metadata import _format_permissions | ||
| from .conftest import RESOURCES_ROOT, run_cli | ||
|
|
||
| SAMPLE_FILES = RESOURCES_ROOT / "sample-files" | ||
|
|
||
| # Optional: exercise the bitmask fallback with real pypdf flags if present | ||
| try: | ||
| from pypdf.constants import UserAccessPermissions as UAP | ||
| except Exception: # pragma: no cover | ||
| UAP = None | ||
|
|
||
|
|
||
| class TestFormatPermissions: | ||
| """Test the _format_permissions helper function.""" | ||
|
|
||
| def test_format_permissions_unencrypted(self): | ||
| assert _format_permissions(None) == "n/a (unencrypted)" | ||
|
|
||
| def test_format_permissions_encrypted_with_no_permissions(self, mocker): | ||
| mock_uap = mocker.Mock() | ||
| mock_uap.to_dict.return_value = { | ||
| "PRINT": False, | ||
| "PRINT_TO_REPRESENTATION": False, | ||
| "MODIFY": False, | ||
| "EXTRACT": False, | ||
| "ADD_OR_MODIFY": False, | ||
| "FILL_FORM_FIELDS": False, | ||
| "EXTRACT_TEXT_AND_GRAPHICS": False, | ||
| "ASSEMBLE_DOC": False, | ||
| } | ||
| assert _format_permissions(mock_uap) == "none (all denied)" | ||
|
|
||
| def test_format_permissions_some_allowed_via_dict(self, mocker): | ||
| mock_uap = mocker.Mock() | ||
| # Order here controls output order | ||
| mock_uap.to_dict.return_value = { | ||
| "PRINT": True, | ||
| "MODIFY": False, | ||
| "EXTRACT": True, | ||
| "ASSEMBLE_DOC": False, | ||
| } | ||
| formatted = _format_permissions(mock_uap) | ||
| # Lower-case labels from label_map | ||
| assert formatted == "print, extract" | ||
|
|
||
| @pytest.mark.skipif(UAP is None, reason="pypdf flags not available") | ||
| def test_format_permissions_some_allowed_bitmask_path(self): | ||
| # Exercises the IntFlag/bitmask fallback (no to_dict) | ||
| uap = UAP.PRINT | UAP.EXTRACT | ||
| formatted = _format_permissions(uap) | ||
| assert formatted == "print, extract" | ||
|
|
||
| def test_format_permissions_unknown_when_unhandled_obj(self): | ||
| class Weird: # no to_dict, not int-castable | ||
| pass | ||
| assert _format_permissions(Weird()) == "unknown" | ||
|
|
||
|
|
||
| class TestMetaCommand: | ||
| """End-to-end tests for the meta CLI command.""" | ||
|
|
||
| def test_meta_command_unencrypted_pdf(self, capsys): | ||
| rel = Path("002-trivial-libre-office-writer/002-trivial-libre-office-writer.pdf") | ||
| input_pdf = SAMPLE_FILES / rel | ||
| if not input_pdf.exists(): | ||
| pytest.skip(f"Unencrypted PDF file not found: {input_pdf}") | ||
|
|
||
| exit_code = run_cli(["meta", str(input_pdf), "--output", "json"]) | ||
| assert exit_code == 0 | ||
|
|
||
| captured = capsys.readouterr() | ||
| metadata = json.loads(captured.out) | ||
| assert metadata["permissions"] == "n/a (unencrypted)" | ||
| # header fix: should read raw PDF header bytes as text | ||
| assert metadata["pdf_file_version"].startswith("%PDF-") | ||
|
|
||
| def test_meta_command_encrypted_pdf(self, capsys): | ||
| rel = Path("005-libreoffice-writer-password/libreoffice-writer-password.pdf") | ||
| input_pdf = SAMPLE_FILES / rel | ||
| if not input_pdf.exists(): | ||
| pytest.skip(f"Encrypted PDF file not found: {input_pdf}") | ||
|
|
||
| exit_code = run_cli(["meta", str(input_pdf), "--output", "json"]) | ||
| assert exit_code == 0 | ||
|
|
||
| captured = capsys.readouterr() | ||
| metadata = json.loads(captured.out) | ||
|
|
||
| assert "permissions" in metadata | ||
| perms = metadata["permissions"] | ||
| assert perms not in {"n/a (unencrypted)", "unknown"} | ||
| # If not "all denied", check formatting invariants | ||
| if perms != "none (all denied)": | ||
| parts = [p.strip() for p in perms.split(",")] | ||
| # lower-case labels | ||
| assert all(p == p.lower() for p in parts) | ||
| # only known labels | ||
| allowed = { | ||
| "print", "print-high", "modify", "extract", | ||
| "annotate", "fill-forms", "accessibility-copy", "assemble", | ||
| } | ||
| assert set(parts).issubset(allowed) | ||
|
|
||
| # header fix also applies on encrypted files | ||
| assert metadata["pdf_file_version"].startswith("%PDF-") | ||
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be known, given that we test a specific PDF file, right?
Maybe it would be better to have 2 distinct test cases/methods:
none (all denied)is displayedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so instead of running that test on only those two file i can run that on all the files in the sample-files/, would that be cool?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be cool, yes.
However I tested your code, and the
assert act_set == exp_setassertion is never evaluated.Which means that we don't have any test covering this case...
With which PDF test file have you been able to test
pdfly metabehaviour, where it displays a list of permissions?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it seems like the test case isn't fully covered, and I don't have a PDF with the permissions metadata behavior to fully test it yet. I tried generating a sample PDF with permissions using PyPDF2, but I wasn't able to achieve the desired result. Honestly, I don't have a lot of experience with unittest and pytest yet, so I'm still getting familiar with some of the testing patterns. If you have any advice or pointers on how to create or detect PDFs with specific permissions metadata, it would be really helpful!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sample-files/005-libreoffice-writer-password/libreoffice-writer-password.pdfhas some permissions set:So there is probably an issue somewhere in the code...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the password need to be provided to read the permissions, as with
qpdf?