Skip to content

[FIX] tools: fix PdfFileReader overwrite#187900

Closed
d-fence wants to merge 2 commits intoodoo:17.0from
odoo-dev:17.0-fix-pdf-compatibility-shims-moc
Closed

[FIX] tools: fix PdfFileReader overwrite#187900
d-fence wants to merge 2 commits intoodoo:17.0from
odoo-dev:17.0-fix-pdf-compatibility-shims-moc

Conversation

@d-fence
Copy link
Contributor

@d-fence d-fence commented Nov 20, 2024

In #183165 the override of the PdfReader constructor was forcing the strict parameter to True1 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.

@robodoo
Copy link
Contributor

robodoo commented Nov 20, 2024

Pull request status dashboard

@xmo-odoo
Copy link
Collaborator

@robodoo squash

@robodoo
Copy link
Contributor

robodoo commented Nov 20, 2024

Merge method set to squash.

Copy link
Contributor

@Demesmaeker Demesmaeker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @d-fence

Tested in 17.0, works perfectly, thank you, it was indeed more complex ^^

Have a nice day

@C3POdoo C3POdoo added the RD research & development, internal work label 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)
@d-fence d-fence force-pushed the 17.0-fix-pdf-compatibility-shims-moc branch from ce72b58 to 2c1d175 Compare November 20, 2024 15:16
@d-fence d-fence marked this pull request as ready for review November 21, 2024 12:34
@C3POdoo C3POdoo requested a review from a team November 21, 2024 12:37
@xmo-odoo
Copy link
Collaborator

@robodoo override=ci/style r+

@robodoo
Copy link
Contributor

robodoo commented Nov 21, 2024

@d-fence @xmo-odoo because this PR has multiple commits, I need to know how to merge it:

  • merge to merge directly, using the PR as merge commit message
  • rebase-merge to rebase and merge, using the PR as merge commit message
  • rebase-ff to rebase and fast-forward

@xmo-odoo
Copy link
Collaborator

@robodoo squash

@robodoo
Copy link
Contributor

robodoo commented Nov 21, 2024

Merge method set to squash.

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>
@robodoo robodoo closed this Nov 22, 2024
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>
@d-fence d-fence deleted the 17.0-fix-pdf-compatibility-shims-moc branch November 27, 2024 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RD research & development, internal work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants