Paths

Table of Contentst

Differential D7059

[lib] Introduce PKCS#7 padding
ClosedPublic
Actions

Authored by bartek on Mar 13 2023, 7:40 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Wed, Oct 2, 10:31 PM

	Unknown Object (File)
	Wed, Oct 2, 10:31 PM

	Unknown Object (File)
	Wed, Oct 2, 10:31 PM

	Unknown Object (File)
	Wed, Oct 2, 10:31 PM

	Unknown Object (File)
	Wed, Oct 2, 10:29 PM

	Unknown Object (File)
	Sep 27 2024, 10:29 AM

	Unknown Object (File)
	Sep 14 2024, 1:13 PM

	Unknown Object (File)
	Aug 31 2024, 6:49 AM

View All 75 Files

Subscribers

Details

Reviewers

atul
marcin
tomek

Commits

rCOMMb8bc1c09f374: [lib] Introduce PKCS#7 padding

Summary

Part of ENG-3099

This diff implements standard PKCS#7 padding using JS typed arrays.

Test Plan

Added unit tests. Also tested using my CLI tool and hex editor / viewer (e.g. xxd tool)

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Mar 13 2023, 7:40 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 13 2023, 7:40 AM

bartek added a child revision: D7060: [lib] Introduce superblock padding.Mar 13 2023, 7:44 AM

Harbormaster completed remote builds in B17292: Diff 23691.Mar 13 2023, 7:56 AM

bartek published this revision for review.Mar 13 2023, 8:09 AM

bartek removed a reviewer: anunay.

bartek added a subscriber: anunay.

ashoat added inline comments.Mar 13 2023, 12:03 PM

lib/utils/pkcs7-padding.js
12	Nit: we don't use inline exports in the codebase except for Flow types and for files in `lib/types` (and other "types" files like that) Can you move these to be a single `export` declaration at the bottom of the file?

Specifically adding @tomek as reviewer since he seemed to have some thoughts during the Encryption Sync (that might've been discussed offline?)

In D7059#210217, @atul wrote:

Specifically adding @tomek as reviewer since he seemed to have some thoughts during the Encryption Sync (that might've been discussed offline?)

Yes, I've met with @bartek and he clarified my concern. The thing which I was missing was the fact that the block padding is always added. Now I can't find any example that won't work.

Thanks for including unit tests!

This revision is now accepted and ready to land.Mar 17 2023, 1:01 PM

atul added inline comments.Mar 17 2023, 1:08 PM

lib/utils/pkcs7-padding.js
13–14	Would it maybe make more sense to throw exceptions instead of using `invariant`? CC @ashoat

ashoat added inline comments.Mar 17 2023, 2:01 PM

lib/utils/pkcs7-padding.js
13–14	I think `invariant` is good for "programmer error" type scenarios like this but I don't feel strongly. Ultimately `invariant` just throws an exception so it doesn't really matter

bartek added inline comments.Mar 21 2023, 5:36 AM

lib/utils/pkcs7-padding.js
12	Sure, will do
13–14	I'd stick to `invariant` because it looks clean.

Rebase, fix exports

Harbormaster completed remote builds in B17481: Diff 23946.Mar 21 2023, 9:42 AM

Closed by commit rCOMMb8bc1c09f374: [lib] Introduce PKCS#7 padding (authored by bartek). · Explain WhyMar 21 2023, 10:21 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMMb8bc1c09f374: [lib] Introduce PKCS#7 padding.

Revision Contents
Changeset List

Path

Size

lib/

utils/

pkcs7-padding.js

40 lines

pkcs7-padding.test.js

90 lines

Diff 23691

lib/utils/pkcs7-padding.js

Loading...

lib/utils/pkcs7-padding.test.js

Loading...