Closed
Conversation
Contributor
|
|
Xavier-Do
reviewed
Oct 10, 2024
be9141d to
e063209
Compare
Collaborator
|
@robodoo override=ci/security |
Motivation: Debian wants to kill pypdf2 and keep only pypdf (4.3) in trixie, so we need to be compatible or risk being dropped. - move all imports from PyPDF2 to odoo.tools.pdf - add two-ways compatibility shims and aliases
In 3.x the PDF header is the header *only*, as it's decoded to a string. However the mess is mostly unnecessary as since 1.27 pypdf2 always adds a binary comments line when writing out documents. So only add (or copy) our own garbage when using 1.x. Also this means our heuristic to detect pdf/a documents makes no sense, as any pdf document can have a binary sig, but hey... Also fix _ID handling: in pypdf 3.x it's a property which reads from the trailer *and* it's automatically copied over by `clone_reader_document_root`.
Try to override both old and new API, although for the new version it might actually be useless as pypdf seems to have added multiple attachments support circa 3.5? (py-pdf/pypdf#1611)
The test relied on reader and writer sharing data because `clone_reader_document_root` would just copy the reference to the root object, so the writer would immediately impact the reader. pypdf apparently now copies the document deeply (possibly since 3.2.0 / py-pdf/pypdf#1520) so this is broken, we need to serialize and reload the document every time.
Next up in the series "`clone_document_from_reader` does stuff now", `clone_document_from_reader` now properly clones over document metadata. This means any metadata set *before* the cloning are nuked. Update the branded writer to write its metadata at the last possible moment, to ensure it always applies (this might actually be undesirable as it means callers can't trivially override it anymore).
`test_download_with_encrypted_pdf` tried to corrupt the PDF by redirecting the /Encrypt indirect ref to an invalid entry, but it did so by replacing a hard-coded index. Except the encryption object is not stored at a fixed offset, where it's stored depends on what pypdf wants to do with it or what other objects it stores in the file. Update the replacement to not be completely hard-coded (and also not assume /Encrypt is at the end of the trailer), and point it back to a very early object (specifically the first one) which is *extremely* unlikely to be a valid encryption object (it's very likely to be either the catalog or the page tree root, though it could also be the metadata map). This leads pypdf to not find a `/Filter` entry, which triggers a parse error. Note: the indirect ref can't be pointed at a nonsensical entry (e.g. 0 or 5479) because in that case pypdf just ignores the ref entirely.
Partial cherry-pick of 59709ca to be able to build a debian package in Trixie.
d-fence
added a commit
to odoo-dev/odoo
that referenced
this pull request
Nov 20, 2024
In odoo#183165 the override of the PdfReader constructor was forcing the `strict` parameter to True [1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1](odoo/odoo@f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19)
robodoo
added a commit
that referenced
this pull request
Nov 22, 2024
In #183165 the override of the PdfReader constructor was forcing the `strict` parameter to True[1][1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1]: f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19 closes #187900 Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com> Co-authored-by: Christophe Monniez <moc@odoo.com> Co-authored-by: xmo-odoo <xmo@odoo.com>
fw-bot
pushed a commit
to odoo-dev/odoo
that referenced
this pull request
Nov 22, 2024
In odoo#183165 the override of the PdfReader constructor was forcing the `strict` parameter to True [1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1](odoo/odoo@f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19) X-original-commit: fd67b56
This was referenced Nov 22, 2024
d-fence
added a commit
to odoo-dev/odoo
that referenced
this pull request
Nov 22, 2024
In odoo#183165 the override of the PdfReader constructor was forcing the `strict` parameter to True [1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1](odoo/odoo@f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19)
robodoo
added a commit
that referenced
this pull request
Nov 22, 2024
In #183165 the override of the PdfReader constructor was forcing the `strict` parameter to True[1][1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1]: f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19 closes #188181 Forward-port-of: #187900 Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com> Signed-off-by: Christophe Monniez (moc) <moc@odoo.com> Co-authored-by: Christophe Monniez <moc@odoo.com> Co-authored-by: xmo-odoo <xmo@odoo.com>
robodoo
pushed a commit
that referenced
this pull request
Nov 25, 2024
In #183165 the override of the PdfReader constructor was forcing the `strict` parameter to True [1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1](f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19) closes #188195 Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
fw-bot
pushed a commit
to odoo-dev/odoo
that referenced
this pull request
Nov 25, 2024
In odoo#183165 the override of the PdfReader constructor was forcing the `strict` parameter to True [1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1](odoo/odoo@f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19) X-original-commit: d5473ce
fw-bot
pushed a commit
to odoo-dev/odoo
that referenced
this pull request
Nov 25, 2024
In odoo#183165 the override of the PdfReader constructor was forcing the `strict` parameter to True [1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1](odoo/odoo@f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19) X-original-commit: d5473ce
robodoo
pushed a commit
that referenced
this pull request
Nov 25, 2024
In #183165 the override of the PdfReader constructor was forcing the `strict` parameter to True [1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1](f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19) closes #188394 X-original-commit: d5473ce Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com> Signed-off-by: Christophe Monniez (moc) <moc@odoo.com>
robodoo
pushed a commit
that referenced
this pull request
Nov 25, 2024
In #183165 the override of the PdfReader constructor was forcing the `strict` parameter to True [1] in the pdf shim for PyPDF2 1.x. So, when a PdfReader is instanciated with strict set to False, with that PyPDF version installed, the strict parameter is not taken into account. In that case, it impossible to upload some PDF documents that were allowed before the shims. With this commit, the override of the constructor is removed in the 1.x shim and the PdfReader override is only made once at the upper level. The `strict` parameter is defaulting to True but not enforced as it was the default behavior in PyPDF2 1.x. It should be changed later to follow the behavior of PyPDF2 >= 2.x. Also, any other parameter of the constructor are discarded as they are useless in PyPDF 1.x and not compatible at all with PyPDF >= 2.x. On the other hand, the new `password` parameter of 2.x is not backward compatible. [1](f03a12c/odoo/tools/pdf/_pypdf2_1.py#L19) closes #188385 X-original-commit: d5473ce Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com> Signed-off-by: Christophe Monniez (moc) <moc@odoo.com>
DantePereyra
pushed a commit
to solvosci/edi
that referenced
this pull request
Jun 3, 2025
Since odoo/odoo#183165, it's failing. After a lot of time trying to get it working without sucess, let's disable it for greenifying the branch. FWIW, it seems Odoo wrapping class is calling `add_metadata` super method without first initializing empty metadata, provoking the assert.
simahawk
pushed a commit
to camptocamp/edi
that referenced
this pull request
Jun 10, 2025
Since odoo/odoo#183165, it's failing. After a lot of time trying to get it working without sucess, let's disable it for greenifying the branch. FWIW, it seems Odoo wrapping class is calling `add_metadata` super method without first initializing empty metadata, provoking the assert.
simahawk
pushed a commit
to camptocamp/reporting-engine
that referenced
this pull request
Jun 12, 2025
Since odoo/odoo#183165, it's failing. After a lot of time trying to get it working without sucess, let's disable it for greenifying the branch. FWIW, it seems Odoo wrapping class is calling `add_metadata` super method without first initializing empty metadata, provoking the assert.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
As Debian wants to remove pypdf2 and keep only pypdf (4.3) in
trixie, so we need to be compatible otherwise Odoo could not be released in the next Debian.