[
https://issues.apache.org/jira/browse/PDFBOX-3971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16214231#comment-16214231
]
Tilman Hausherr edited comment on PDFBOX-3971 at 10/22/17 1:24 PM:
-------------------------------------------------------------------
Thanks... I'll review the code today or monday. I'll edit my thoughts here.
- setCertificate() has an NPE potential now (remove .getCOSObject()) (no need
to resubmit)
- line lengths (no need to resubmit)
- "public boolean remove..." it removes an item and returns a result, i.e. does
two things. I searched for "public boolean remove" (4 matches) and "public void
remove" (20 matches) in our code and it looks as we're mostly comfortable not
bothering about the result
- "setNeedToBeUpdated()" calls - they don't hurt, but I doubt that they are
needed. These are used in incremental save. But with signatures, the whole
signature is new, unless you're signing in an empty signature field. So setting
that is only needed if you think that the structure exists before signing. If
this is so, then keep it. If it isn't so, then remove these calls.
- line 341 dead code.
- why FLAG_RESERVED and related methods? My understanding is that this is
reserved for further use so it shouldn't be there at all.
- replace .getItem() with .getDictionaryObject() unless you expect it to be a
direct object
- the "if (flag)" code isn't needed where it is now, the code there can be put
at the place where flag is set to true, it is unlikely that the list element
would be there twice. (If the code is needed at all, see 4th point)
- line 364 you can use "base", and the result is never null
- javadoc of getKeyUsage / setKeyUsage: use HTML to have a list, see
AccessPermission class javadoc on how to do this
-
was (Author: tilman):
Thanks... I'll review the code today or monday. I'll edit my thoughts here.
- setCertificate() has an NPE potential now (remove .getCOSObject()) (no need
to resubmit)
- line lengths (no need to resubmit)
- "public boolean remove..." it removes an item and returns a result, i.e. does
two things. I searched for "public boolean remove" (4 matches) and "public void
remove" (20 matches) in our code and it looks as we're mostly comfortable not
bothering about the result
- "setNeedToBeUpdated()" calls - they don't hurt, but I doubt that they are
needed. These are used in incremental save. But with signatures, the whole
signature is new, unless you're signing in an empty signature field. So setting
that is only needed if you think that the structure exists before signing. If
this is so, then keep it. If it isn't so, then remove these calls.
- line 341 dead code.
- why FLAG_RESERVED and related methods? My understanding is that this is
reserved for further use so it shouldn't be there at all.
- replace .getItem() with .getDictionaryObject() unless you expect it to be a
direct object
- the "if (flag)" code isn't needed where it is now, the code there can be put
at the place where flag is set to true, it is unlikely that the list element
would be there twice. (If the code is needed at all, see 4th point)
- line 364 you can use "base", and the result is never null
-
> Add Certificate Dictionary to seed value in signature field
> -----------------------------------------------------------
>
> Key: PDFBOX-3971
> URL: https://issues.apache.org/jira/browse/PDFBOX-3971
> Project: PDFBox
> Issue Type: Improvement
> Components: Signing
> Reporter: Hossam Hazem
> Labels: documentation, features, newbie, patch, test
> Attachments: COSName.patch, PDSeedValue.patch,
> PDSeedValueCertificate.java
>
>
> This dictionary is important as it gives the ability to put certificate
> constraints on a signature field, like if you want signatures that are signed
> by a specific issuer or authority to only be used in a field.
> currently tested Issuer constraint and it worked, acrobat reader ignores
> other certificates and only allow the issuer given to sign the field.
> documentation is not complete waiting for the initial acceptance to complete.
> new class PDSeedValueCertificate is added which refers to this certificate.
> PDSeedValue is modified to add the new dictionary.
> COSName is modified to add the new pdf names that are included in the
> dictionary.
> reference for this dictionary can be found in PDF reference 1.7 section
> 12.7.4.5 table 235 page 457 in here
> http://www.adobe.com/content/dam/acom/en/devnet/pdf/PDF32000_2008.pdf
> or chapter 8 table 8.84 page 700 in here
> http://archimedespalimpsest.net/Documents/External/pdf_reference_1-7.pdf
> and in here
> https://www.adobe.com/devnet-docs/acrobatetk/tools/DigSig/Acrobat_DigitalSignatures_in_PDF.pdf
> this is my first contribution, hope everything goes well.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
